You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "jinhongyii (via GitHub)" <gi...@apache.org> on 2023/03/21 02:08:42 UTC
[GitHub] [tvm] jinhongyii opened a new pull request, #14348: [Unity] Add More Ops For FX Translator
jinhongyii opened a new pull request, #14348:
URL: https://github.com/apache/tvm/pull/14348
This PR makes 2 changes:
1. Add Relax Op Maximum and Minimum
2. Add translation function for torch function/method silu, to, ones, full, masked_fill_, mean, rsqrt, neg, max in fx translator
cc: @tqchen @MasterJH5574
--
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] jinhongyii commented on pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on PR #14348:
URL: https://github.com/apache/tvm/pull/14348#issuecomment-1477270840
seems the CI failed because its pytorch version is not over 2.0
--
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] MasterJH5574 merged pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "MasterJH5574 (via GitHub)" <gi...@apache.org>.
MasterJH5574 merged PR #14348:
URL: https://github.com/apache/tvm/pull/14348
--
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] tvm-bot commented on pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #14348:
URL: https://github.com/apache/tvm/pull/14348#issuecomment-1477182333
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @quic-sanirudh <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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] jinhongyii commented on a diff in pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on code in PR #14348:
URL: https://github.com/apache/tvm/pull/14348#discussion_r1142853740
##########
tests/python/relax/test_frontend_from_fx.py:
##########
@@ -19,20 +19,51 @@
import tvm
from tvm import relax
import tvm.testing
-from tvm.script.parser import ir as I, relax as R, tir as T
+from tvm.script import ir as I
+from tvm.script import relax as R
+from tvm.script import tir as T
-def verify_model(torch_model, input_info, binding, expected):
+def verify_model(torch_model, input_info, binding, expected, use_dynamo=False):
+ import torch
+ import torch._dynamo as dynamo
from torch import fx
from tvm.relax.frontend.torch import from_fx
- graph_model = fx.symbolic_trace(torch_model)
- mod = from_fx(graph_model, input_info)
+ if use_dynamo:
+ args = []
+ for info in input_info:
+ args.append(torch.zeros(*info[0], dtype=_convert_data_type(info[1])))
+ graph_model = dynamo.export(torch_model, *args)[0]
+ else:
+ graph_model = fx.symbolic_trace(torch_model)
+ print(graph_model.code)
+ mod = from_fx(graph_model, input_info, unwrap_unit_return_tuple=True)
Review Comment:
maybe I can add a param to decide whether to turn on unwrap_unit_return_tuple for now
--
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] tqchen commented on pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #14348:
URL: https://github.com/apache/tvm/pull/14348#issuecomment-1478713013
we can use pytest.markskipif()
https://github.com/apache/tvm/blob/unity/tests/python/relax/test_frontend_dynamo.py#L19
if the tests involves dynamo, consider move them to test_frontend_dynamo instead
--
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] MasterJH5574 commented on a diff in pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "MasterJH5574 (via GitHub)" <gi...@apache.org>.
MasterJH5574 commented on code in PR #14348:
URL: https://github.com/apache/tvm/pull/14348#discussion_r1142847893
##########
tests/python/relax/test_frontend_from_fx.py:
##########
@@ -19,20 +19,51 @@
import tvm
from tvm import relax
import tvm.testing
-from tvm.script.parser import ir as I, relax as R, tir as T
+from tvm.script import ir as I
+from tvm.script import relax as R
+from tvm.script import tir as T
-def verify_model(torch_model, input_info, binding, expected):
+def verify_model(torch_model, input_info, binding, expected, use_dynamo=False):
+ import torch
+ import torch._dynamo as dynamo
from torch import fx
from tvm.relax.frontend.torch import from_fx
- graph_model = fx.symbolic_trace(torch_model)
- mod = from_fx(graph_model, input_info)
+ if use_dynamo:
+ args = []
+ for info in input_info:
+ args.append(torch.zeros(*info[0], dtype=_convert_data_type(info[1])))
+ graph_model = dynamo.export(torch_model, *args)[0]
+ else:
+ graph_model = fx.symbolic_trace(torch_model)
+ print(graph_model.code)
+ mod = from_fx(graph_model, input_info, unwrap_unit_return_tuple=True)
Review Comment:
For `unwrap_unit_return_tuple=True` I think it depends. Usually for graph from `dynamo.export`, you want to use `unwrap_unit_return_tuple=True`. But for graphs from `fx.symbolic_trace`, it is not necessary to use True. We also want to test the case of `unwrap_unit_return_tuple=False` in this file. Seems that with this change it is untested.
##########
tests/python/relax/test_frontend_from_fx.py:
##########
@@ -19,20 +19,51 @@
import tvm
from tvm import relax
import tvm.testing
-from tvm.script.parser import ir as I, relax as R, tir as T
+from tvm.script import ir as I
+from tvm.script import relax as R
+from tvm.script import tir as T
-def verify_model(torch_model, input_info, binding, expected):
+def verify_model(torch_model, input_info, binding, expected, use_dynamo=False):
+ import torch
+ import torch._dynamo as dynamo
from torch import fx
from tvm.relax.frontend.torch import from_fx
- graph_model = fx.symbolic_trace(torch_model)
- mod = from_fx(graph_model, input_info)
+ if use_dynamo:
+ args = []
+ for info in input_info:
+ args.append(torch.zeros(*info[0], dtype=_convert_data_type(info[1])))
+ graph_model = dynamo.export(torch_model, *args)[0]
+ else:
+ graph_model = fx.symbolic_trace(torch_model)
+ print(graph_model.code)
+ mod = from_fx(graph_model, input_info, unwrap_unit_return_tuple=True)
+ print(mod.script())
Review Comment:
Debugging lines to remove.
--
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] jinhongyii commented on a diff in pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on code in PR #14348:
URL: https://github.com/apache/tvm/pull/14348#discussion_r1142883069
##########
tests/python/relax/test_frontend_from_fx.py:
##########
@@ -19,20 +19,51 @@
import tvm
from tvm import relax
import tvm.testing
-from tvm.script.parser import ir as I, relax as R, tir as T
+from tvm.script import ir as I
+from tvm.script import relax as R
+from tvm.script import tir as T
-def verify_model(torch_model, input_info, binding, expected):
+def verify_model(
Review Comment:
I'm not sure either. I think we don't need it here given we don't have any low-level implementation in the IR after fx translator.
--
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] jinhongyii commented on a diff in pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on code in PR #14348:
URL: https://github.com/apache/tvm/pull/14348#discussion_r1142852660
##########
tests/python/relax/test_frontend_from_fx.py:
##########
@@ -19,20 +19,51 @@
import tvm
from tvm import relax
import tvm.testing
-from tvm.script.parser import ir as I, relax as R, tir as T
+from tvm.script import ir as I
+from tvm.script import relax as R
+from tvm.script import tir as T
-def verify_model(torch_model, input_info, binding, expected):
+def verify_model(torch_model, input_info, binding, expected, use_dynamo=False):
+ import torch
+ import torch._dynamo as dynamo
from torch import fx
from tvm.relax.frontend.torch import from_fx
- graph_model = fx.symbolic_trace(torch_model)
- mod = from_fx(graph_model, input_info)
+ if use_dynamo:
+ args = []
+ for info in input_info:
+ args.append(torch.zeros(*info[0], dtype=_convert_data_type(info[1])))
+ graph_model = dynamo.export(torch_model, *args)[0]
+ else:
+ graph_model = fx.symbolic_trace(torch_model)
+ print(graph_model.code)
+ mod = from_fx(graph_model, input_info, unwrap_unit_return_tuple=True)
Review Comment:
this is just a workaround to avoid the unit tuple printing bug. If that bug is fixed, we can change this line back later
--
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] yongwww commented on a diff in pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "yongwww (via GitHub)" <gi...@apache.org>.
yongwww commented on code in PR #14348:
URL: https://github.com/apache/tvm/pull/14348#discussion_r1142867702
##########
tests/python/relax/test_frontend_from_fx.py:
##########
@@ -19,20 +19,51 @@
import tvm
from tvm import relax
import tvm.testing
-from tvm.script.parser import ir as I, relax as R, tir as T
+from tvm.script import ir as I
+from tvm.script import relax as R
+from tvm.script import tir as T
-def verify_model(torch_model, input_info, binding, expected):
+def verify_model(
Review Comment:
Question (out scope of this pr) for pt converter verification: Looks we verify the IR, not very sure if we need to verify the inference result as well?
--
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] jinhongyii commented on pull request #14348: [Unity] Add More Ops For FX Translator
Posted by "jinhongyii (via GitHub)" <gi...@apache.org>.
jinhongyii commented on PR #14348:
URL: https://github.com/apache/tvm/pull/14348#issuecomment-1477272106
Is there a way to skip some of the tests?
--
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