You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/04/27 11:39:40 UTC
[GitHub] [tvm] gayatripk1 opened a new pull request, #11142: Fix mixed precision output type to original type
gayatripk1 opened a new pull request, #11142:
URL: https://github.com/apache/tvm/pull/11142
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on a diff in pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
comaniac commented on code in PR #11142:
URL: https://github.com/apache/tvm/pull/11142#discussion_r862270681
##########
src/relay/transforms/to_mixed_precision.cc:
##########
@@ -36,6 +36,7 @@
namespace tvm {
namespace relay {
+TVM_REGISTER_PASS_CONFIG_OPTION("relay.ToMixedPrecision.enable_original_type", Bool);
Review Comment:
This name is a bit confusing. Maybe something like "keep_orig_output_dtype".
##########
src/relay/transforms/to_mixed_precision.cc:
##########
@@ -381,6 +400,11 @@ class MixedPrecisionPass : public MixedModeMutator {
if (accumulation_dtype != output_dtype) {
output = CastArg(output, GetType(output), output_dtype);
}
+ if (pre_call_node == static_cast<const CallNode*>(root_) && enable_original_type_) {
Review Comment:
Could we just use `.same_as`?
##########
tests/python/relay/test_to_mixed_precision.py:
##########
@@ -41,17 +41,31 @@ def verify_mixed_precision_output_close(
mixed_precision_dtype="float16",
rtol: float = 1e-3,
atol: float = 0,
+ enable_original_type=False,
) -> tvm.runtime.Module:
mod = InferType()(mod)
result_fp32 = run_module(mod, mod_params)
- fp16_mod = ToMixedPrecision(mixed_precision_dtype)(mod)
- result_fp16 = run_module(fp16_mod, mod_params)
+
+ if enable_original_type == False:
Review Comment:
```suggestion
if not enable_original_type:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac merged pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
comaniac merged PR #11142:
URL: https://github.com/apache/tvm/pull/11142
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gayatripk1 commented on a diff in pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
gayatripk1 commented on code in PR #11142:
URL: https://github.com/apache/tvm/pull/11142#discussion_r862980415
##########
src/relay/transforms/to_mixed_precision.cc:
##########
@@ -36,6 +36,7 @@
namespace tvm {
namespace relay {
+TVM_REGISTER_PASS_CONFIG_OPTION("relay.ToMixedPrecision.enable_original_type", Bool);
Review Comment:
Changed the name to keep_orig_output_dtype
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] AndrewZhaoLuo commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1112706241
Sorry ive been busy, ill take a look tomorrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
comaniac commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1111081510
I think this PR is for the model output dtype only so it shouldn't affect other cases.
In addition to that, I'd suggest 1) exposing this behavior to users by adding a configure to PassContext, and 2) adding a unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yangulei commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
yangulei commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1111649979
> I think this PR is for the model output dtype only so it shouldn't affect other cases.
OK, thanks for your clarification.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yangulei commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
yangulei commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1111013803
Hi, does this mean the output OP has different dtype for inputs, weights and dst after AMP? If so, I'm afraid that my bfloat16 support for DNNL BYOC(https://github.com/apache/tvm/pull/11111) might conflict with this. I implement bfloat16 support with the assumption that the dtype for the inputs, weights and dst are either all float32 or all bfloat16.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gayatripk1 commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
gayatripk1 commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1118655570
waiting for more reviews?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gayatripk1 commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
gayatripk1 commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1113619855
@AndrewZhaoLuo @comaniac, Modified this behavior to users by adding a configure to PassContext, and adding a unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
comaniac commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1118709163
Thanks @gayatripk1 @AndrewZhaoLuo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gayatripk1 commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
gayatripk1 commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1116856540
> Looks like CI is flaky, please push empty commit to restart CI. @gayatripk1
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gayatripk1 commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
gayatripk1 commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1110899859
cc @AndrewZhaoLuo @kparzysz-quic
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] AndrewZhaoLuo commented on pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on PR #11142:
URL: https://github.com/apache/tvm/pull/11142#issuecomment-1116700827
Looks like CI is flaky, please push empty commit to restart CI. @gayatripk1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] gayatripk1 commented on a diff in pull request #11142: Fix mixed precision output type to original type
Posted by GitBox <gi...@apache.org>.
gayatripk1 commented on code in PR #11142:
URL: https://github.com/apache/tvm/pull/11142#discussion_r862986680
##########
src/relay/transforms/to_mixed_precision.cc:
##########
@@ -381,6 +400,11 @@ class MixedPrecisionPass : public MixedModeMutator {
if (accumulation_dtype != output_dtype) {
output = CastArg(output, GetType(output), output_dtype);
}
+ if (pre_call_node == static_cast<const CallNode*>(root_) && enable_original_type_) {
Review Comment:
changing to same_as will require us to use "if (((GetRef<Call>(pre_call_node)).same_as(GetRef<RelayExpr>(root_))) && keep_orig_output_dtype_ )". So leaving the comparison as such
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org