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 2020/09/13 01:21:43 UTC

[GitHub] [incubator-tvm] masahi opened a new pull request #6467: [ONNX] Update Slice op conversion to take strides into account, clean up tests

masahi opened a new pull request #6467:
URL: https://github.com/apache/incubator-tvm/pull/6467


   Fix https://github.com/apache/incubator-tvm/issues/6441


----------------------------------------------------------------
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] [incubator-tvm] tqchen commented on pull request #6467: [ONNX] Update Slice op conversion to take strides into account, clean up tests

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6467:
URL: https://github.com/apache/incubator-tvm/pull/6467#issuecomment-691700079


   Thanks @masahi @mbrookhart !


----------------------------------------------------------------
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] [incubator-tvm] masahi commented on a change in pull request #6467: [ONNX] Update Slice op conversion to take strides into account, clean up tests

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #6467:
URL: https://github.com/apache/incubator-tvm/pull/6467#discussion_r487481546



##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -681,6 +675,32 @@ def test_slice():
         x, x, starts=(0, 0), ends=(9223372036854775807, 9223372036854775807), axes=(0, 3)
     )
 
+    def test_slice_with_strides():
+        class SliceWithStrides(torch.nn.Module):
+            def forward(self, x):
+                return x[..., 0::2] + x[..., 1::2]
+
+        class SliceWithStrides2(torch.nn.Module):
+            def forward(self, x):
+                return x[0::2, 0::2] + x[1::2, 1::2]
+

Review comment:
       Fixed




----------------------------------------------------------------
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] [incubator-tvm] masahi commented on pull request #6467: [ONNX] Update Slice op conversion to take strides into account, clean up tests

Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #6467:
URL: https://github.com/apache/incubator-tvm/pull/6467#issuecomment-691613542


   @mbrookhart I realized, you would get some conflicts in `test_forward.py` too, due to your change of `get_output_tvm` to `get_output_tvm_with_vm`. I've checked that the change is minimal. I hope it's not too bad ...  


----------------------------------------------------------------
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] [incubator-tvm] masahi commented on a change in pull request #6467: [ONNX] Update Slice op conversion to take strides into account, clean up tests

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #6467:
URL: https://github.com/apache/incubator-tvm/pull/6467#discussion_r487472687



##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -681,6 +675,32 @@ def test_slice():
         x, x, starts=(0, 0), ends=(9223372036854775807, 9223372036854775807), axes=(0, 3)
     )
 
+    def test_slice_with_strides():
+        class SliceWithStrides(torch.nn.Module):
+            def forward(self, x):
+                return x[..., 0::2] + x[..., 1::2]
+
+        class SliceWithStrides2(torch.nn.Module):
+            def forward(self, x):
+                return x[0::2, 0::2] + x[1::2, 1::2]
+

Review comment:
       yeah, I did this way because the bug report https://github.com/apache/incubator-tvm/issues/6441 came with PyTorch code and I was too lazy to manually write equivalent onnx :) I'll update this to remove pytorch dep.




----------------------------------------------------------------
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] [incubator-tvm] tqchen merged pull request #6467: [ONNX] Update Slice op conversion to take strides into account, clean up tests

Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6467:
URL: https://github.com/apache/incubator-tvm/pull/6467


   


----------------------------------------------------------------
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] [incubator-tvm] masahi edited a comment on pull request #6467: [ONNX] Update Slice op conversion to take strides into account, clean up tests

Posted by GitBox <gi...@apache.org>.
masahi edited a comment on pull request #6467:
URL: https://github.com/apache/incubator-tvm/pull/6467#issuecomment-691613542


   @mbrookhart I realized, you would get some conflicts in `test_forward.py` too, due to your change of `get_output_tvm` to `get_output_tvm_with_vm` in some tests. I've checked that the change is minimal. I hope it's not too bad ...  


----------------------------------------------------------------
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] [incubator-tvm] mbrookhart commented on a change in pull request #6467: [ONNX] Update Slice op conversion to take strides into account, clean up tests

Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6467:
URL: https://github.com/apache/incubator-tvm/pull/6467#discussion_r487470773



##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -681,6 +675,32 @@ def test_slice():
         x, x, starts=(0, 0), ends=(9223372036854775807, 9223372036854775807), axes=(0, 3)
     )
 
+    def test_slice_with_strides():
+        class SliceWithStrides(torch.nn.Module):
+            def forward(self, x):
+                return x[..., 0::2] + x[..., 1::2]
+
+        class SliceWithStrides2(torch.nn.Module):
+            def forward(self, x):
+                return x[0::2, 0::2] + x[1::2, 1::2]
+

Review comment:
       Why do this with Pytorch? Having this kind of thing in this file means that people have to install even more frameworks if they want to be running/testing ONNX TVM. Why not just construct the ONNX graph manually like most of the rest 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org