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 2021/03/26 00:58:50 UTC
[GitHub] [tvm] jwfromm opened a new pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
jwfromm opened a new pull request #7747:
URL: https://github.com/apache/tvm/pull/7747
Although not officially supported by Onnx, Pytorch will sometimes produce graphs with Aten nodes when it cant figure out how to correctly export. One example of this is DLRM models, which use EmbeddingBag layers. This PR introduces a converter for handling ATen operators and adds the ability to import `Aten:EmbeddingBag` Nodes. This allows TVM to correctly import full DLRM graphs which is pretty neat.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tmoreau89 merged pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
tmoreau89 merged pull request #7747:
URL: https://github.com/apache/tvm/pull/7747
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbrookhart commented on pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#issuecomment-809509540
:/ It looks like your hitting a bunch of vta unit test errors on the latest run? Possibly an issue in upstream CI? Rebase?
What if we convert the onnx model into a string and assert that "Aten:EmbeddingBag" appears in the model? My main worry here is that this will silently stop testing the thing we want to 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tmoreau89 commented on a change in pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
tmoreau89 commented on a change in pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#discussion_r603638068
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -3200,7 +3272,7 @@ def from_onnx(model, shape=None, dtype="float32", opset=None, freeze_params=Fals
# try use onnx's own model checker before converting any model
try:
onnx.checker.check_model(model)
- except onnx.onnx_cpp2py_export.checker.ValidationError as e: # pylint: disable=c-extension-no-member
+ except Exception as e: # pylint: disable=c-extension-no-member, broad-except
Review comment:
Agreed, large models may fail the check but can still run
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on a change in pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#discussion_r602647038
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4278,6 +4278,33 @@ def test_wrong_input():
relay.frontend.from_onnx(model, shape=wrong_shape_dict)
+def test_aten():
+ torch.set_grad_enabled(False)
+
+ def _convert_to_onnx(model, inputs):
+ file_name = "{}.onnx".format("aten_model")
+ torch.onnx.export(
+ model, inputs, file_name, export_params=True, verbose=False, opset_version=10
+ )
Review comment:
I added `aten=True` which makes all nodes in the graph `aten` rather than just embedding_bag so I've also added a few extra converters to the dispatch.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbrookhart commented on a change in pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#discussion_r601973492
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4278,6 +4278,33 @@ def test_wrong_input():
relay.frontend.from_onnx(model, shape=wrong_shape_dict)
+def test_aten():
+ torch.set_grad_enabled(False)
+
+ def _convert_to_onnx(model, inputs):
+ file_name = "{}.onnx".format("aten_model")
+ torch.onnx.export(
+ model, inputs, file_name, export_params=True, verbose=False, opset_version=10
+ )
Review comment:
Can we assert that this actually contains the expected aten model? What conditions mean that pytorch instead exports a valid onnx graph?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on a change in pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#discussion_r602782705
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4278,6 +4278,33 @@ def test_wrong_input():
relay.frontend.from_onnx(model, shape=wrong_shape_dict)
+def test_aten():
+ torch.set_grad_enabled(False)
+
+ def _convert_to_onnx(model, inputs):
+ file_name = "{}.onnx".format("aten_model")
+ torch.onnx.export(
+ model, inputs, file_name, export_params=True, verbose=False, opset_version=10
+ )
Review comment:
Actually it looks like the version of torch we're using for CI doesnt yet support `aten`. For now we'll have to add this without `aten=True` but we can enable it later if we update versions. The good news is old versions of torch always use ATen fallback ops so we'll be testing embeddingbag.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tmoreau89 commented on pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
tmoreau89 commented on pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#issuecomment-810640773
Thank you @jwfromm @mbrookhart @masahi the PR has been merged
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jwfromm commented on a change in pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#discussion_r603607809
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4278,6 +4278,33 @@ def test_wrong_input():
relay.frontend.from_onnx(model, shape=wrong_shape_dict)
+def test_aten():
+ torch.set_grad_enabled(False)
+
+ def _convert_to_onnx(model, inputs):
+ file_name = "{}.onnx".format("aten_model")
+ torch.onnx.export(
+ model, inputs, file_name, export_params=True, verbose=False, opset_version=10
+ )
Review comment:
Yeah its kind of goofy, `aten` is present but always causes an error with `1.7.0`. You're right that its suggested replacement works fine. I've added that in the latest commit.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#discussion_r603599665
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4278,6 +4278,33 @@ def test_wrong_input():
relay.frontend.from_onnx(model, shape=wrong_shape_dict)
+def test_aten():
+ torch.set_grad_enabled(False)
+
+ def _convert_to_onnx(model, inputs):
+ file_name = "{}.onnx".format("aten_model")
+ torch.onnx.export(
+ model, inputs, file_name, export_params=True, verbose=False, opset_version=10
+ )
Review comment:
> Actually it looks like the version of torch we're using for CI doesnt yet support `aten`. For now we'll have to add this without `aten=True` but we can enable it later if we update versions. The good news is old versions of torch always use ATen fallback ops so we'll be testing embeddingbag.
Really? The link I posted above is for PT v1.7 and this is what we run on CI.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#discussion_r602028659
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4278,6 +4278,33 @@ def test_wrong_input():
relay.frontend.from_onnx(model, shape=wrong_shape_dict)
+def test_aten():
+ torch.set_grad_enabled(False)
+
+ def _convert_to_onnx(model, inputs):
+ file_name = "{}.onnx".format("aten_model")
+ torch.onnx.export(
+ model, inputs, file_name, export_params=True, verbose=False, opset_version=10
+ )
Review comment:
https://github.com/pytorch/pytorch/blob/release/1.7/torch/onnx/__init__.py#L72-L74
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #7747: [Relay]Frontend][Onnx] Add a converter for ATen Nodes
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7747:
URL: https://github.com/apache/tvm/pull/7747#discussion_r603599665
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4278,6 +4278,33 @@ def test_wrong_input():
relay.frontend.from_onnx(model, shape=wrong_shape_dict)
+def test_aten():
+ torch.set_grad_enabled(False)
+
+ def _convert_to_onnx(model, inputs):
+ file_name = "{}.onnx".format("aten_model")
+ torch.onnx.export(
+ model, inputs, file_name, export_params=True, verbose=False, opset_version=10
+ )
Review comment:
> Actually it looks like the version of torch we're using for CI doesnt yet support `aten`. For now we'll have to add this without `aten=True` but we can enable it later if we update versions. The good news is old versions of torch always use ATen fallback ops so we'll be testing embeddingbag.
Really? The link I posted above is for PT v1.7 and this is what we run on CI.
As the comment in the doc says, `aten` option is being deprecated. We should probably use the suggested alternative.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org