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/05/25 00:12:23 UTC

[GitHub] [tvm] CircleSpin opened a new pull request, #11446: [Onnx] Round operator

CircleSpin opened a new pull request, #11446:
URL: https://github.com/apache/tvm/pull/11446

   Completed with reference to the tutorial presented at tvmconf: https://www.youtube.com/watch?v=fZvNG-oKK5Q&amp;t=20s
   
   Onnx uses an abnormal round operator where .5 is rounded to the nearest even integer, which differs comparative to other round definitions.
   
   @MargaretQian @jwfromm @sfvaroglu 
   


-- 
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] CircleSpin commented on a diff in pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
CircleSpin commented on code in PR #11446:
URL: https://github.com/apache/tvm/pull/11446#discussion_r882249441


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -4978,10 +4978,30 @@ def _impl_v1(cls, inputs, attr, params):
         return _expr.TupleWrapper(_expr.Tuple(result), len(result))
 
 
+class Round(OnnxOpConverter):
+    """Operator converter for round op."""
+
+    @classmethod
+    def _impl_v11(cls, inputs, attr, params):
+        # See the tutorial for full implementation details:
+        # https://www.youtube.com/watch?v=fZvNG-oKK5Q&list=PL_4zDggB-DBpynCEnC9hV-1euZrP3xDRK&index=4

Review Comment:
   @jwfromm Perhaps flipping the comments so that line 4989 comes before the youtube link is better readability? 



-- 
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 merged pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo merged PR #11446:
URL: https://github.com/apache/tvm/pull/11446


-- 
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 a diff in pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #11446:
URL: https://github.com/apache/tvm/pull/11446#discussion_r887160056


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -4978,10 +4978,30 @@ def _impl_v1(cls, inputs, attr, params):
         return _expr.TupleWrapper(_expr.Tuple(result), len(result))
 
 
+class Round(OnnxOpConverter):
+    """Operator converter for round op."""
+
+    @classmethod
+    def _impl_v11(cls, inputs, attr, params):
+        # See the tutorial for full implementation details:
+        # https://www.youtube.com/watch?v=fZvNG-oKK5Q&list=PL_4zDggB-DBpynCEnC9hV-1euZrP3xDRK&index=4

Review Comment:
   I personally think the code is simple enough where the implementation details are self-documenting



-- 
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] CircleSpin commented on a diff in pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
CircleSpin commented on code in PR #11446:
URL: https://github.com/apache/tvm/pull/11446#discussion_r882248283


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -4978,10 +4978,30 @@ def _impl_v1(cls, inputs, attr, params):
         return _expr.TupleWrapper(_expr.Tuple(result), len(result))
 
 
+class Round(OnnxOpConverter):
+    """Operator converter for round op."""
+
+    @classmethod
+    def _impl_v11(cls, inputs, attr, params):
+        # See the tutorial for full implementation details:
+        # https://www.youtube.com/watch?v=fZvNG-oKK5Q&list=PL_4zDggB-DBpynCEnC9hV-1euZrP3xDRK&index=4

Review Comment:
   @jwfromm Do you feel that the line 4989 doesn't give enough context in regards to what onnx is doing? 
   
   @AndrewZhaoLuo This was more about having the value of Josh's tutorial accessible. 



-- 
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] CircleSpin commented on a diff in pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
CircleSpin commented on code in PR #11446:
URL: https://github.com/apache/tvm/pull/11446#discussion_r887243912


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -4978,10 +4978,30 @@ def _impl_v1(cls, inputs, attr, params):
         return _expr.TupleWrapper(_expr.Tuple(result), len(result))
 
 
+class Round(OnnxOpConverter):
+    """Operator converter for round op."""
+
+    @classmethod
+    def _impl_v11(cls, inputs, attr, params):
+        # See the tutorial for full implementation details:
+        # https://www.youtube.com/watch?v=fZvNG-oKK5Q&list=PL_4zDggB-DBpynCEnC9hV-1euZrP3xDRK&index=4

Review Comment:
   sounds good :) 
   



-- 
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] jwfromm commented on a diff in pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
jwfromm commented on code in PR #11446:
URL: https://github.com/apache/tvm/pull/11446#discussion_r882025015


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -4978,10 +4978,30 @@ def _impl_v1(cls, inputs, attr, params):
         return _expr.TupleWrapper(_expr.Tuple(result), len(result))
 
 
+class Round(OnnxOpConverter):
+    """Operator converter for round op."""
+
+    @classmethod
+    def _impl_v11(cls, inputs, attr, params):
+        # See the tutorial for full implementation details:
+        # https://www.youtube.com/watch?v=fZvNG-oKK5Q&list=PL_4zDggB-DBpynCEnC9hV-1euZrP3xDRK&index=4

Review Comment:
   It would be better to explain what onnx is doing (and why we need to handle it specially) in the comment rather than linking to youtube.



-- 
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 a diff in pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #11446:
URL: https://github.com/apache/tvm/pull/11446#discussion_r882141395


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -4978,10 +4978,30 @@ def _impl_v1(cls, inputs, attr, params):
         return _expr.TupleWrapper(_expr.Tuple(result), len(result))
 
 
+class Round(OnnxOpConverter):
+    """Operator converter for round op."""
+
+    @classmethod
+    def _impl_v11(cls, inputs, attr, params):
+        # See the tutorial for full implementation details:
+        # https://www.youtube.com/watch?v=fZvNG-oKK5Q&list=PL_4zDggB-DBpynCEnC9hV-1euZrP3xDRK&index=4

Review Comment:
   If you want, you can just credit josh in the PR text if this is about credit



-- 
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] CircleSpin commented on a diff in pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
CircleSpin commented on code in PR #11446:
URL: https://github.com/apache/tvm/pull/11446#discussion_r882249441


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -4978,10 +4978,30 @@ def _impl_v1(cls, inputs, attr, params):
         return _expr.TupleWrapper(_expr.Tuple(result), len(result))
 
 
+class Round(OnnxOpConverter):
+    """Operator converter for round op."""
+
+    @classmethod
+    def _impl_v11(cls, inputs, attr, params):
+        # See the tutorial for full implementation details:
+        # https://www.youtube.com/watch?v=fZvNG-oKK5Q&list=PL_4zDggB-DBpynCEnC9hV-1euZrP3xDRK&index=4

Review Comment:
   @jwfromm Perhaps flipping the comments so that like 4989 comes before the youtube link is better readability? 



-- 
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] jwfromm commented on a diff in pull request #11446: [Onnx] Round operator

Posted by GitBox <gi...@apache.org>.
jwfromm commented on code in PR #11446:
URL: https://github.com/apache/tvm/pull/11446#discussion_r887158713


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -4978,10 +4978,30 @@ def _impl_v1(cls, inputs, attr, params):
         return _expr.TupleWrapper(_expr.Tuple(result), len(result))
 
 
+class Round(OnnxOpConverter):
+    """Operator converter for round op."""
+
+    @classmethod
+    def _impl_v11(cls, inputs, attr, params):
+        # See the tutorial for full implementation details:
+        # https://www.youtube.com/watch?v=fZvNG-oKK5Q&list=PL_4zDggB-DBpynCEnC9hV-1euZrP3xDRK&index=4

Review Comment:
   I just would prefer us not to have links to youtube in the codebase.



-- 
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