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/03/23 17:56:43 UTC

[GitHub] [incubator-tvm] siju-samuel opened a new pull request #5135: [ONNX]Pool3d & upsample3d op support

siju-samuel opened a new pull request #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135
 
 
   Pool3d(fix) and upsample op  for onnx frontend.
   @FrozenGene  @masahi @zhiics Please help to review and merge this PR.
   
   Thanks for contributing to TVM!   Please refer to guideline https://docs.tvm.ai/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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on issue #5135: [ONNX]Pool3d & upsample3d op support

Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135#issuecomment-609002833
 
 
   Thanks @siju-samuel @jwfromm @cchung100m 

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support

Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135#discussion_r396797392
 
 

 ##########
 File path: tests/python/frontend/onnx/test_forward.py
 ##########
 @@ -741,6 +741,30 @@ def _test_upsample_nearest():
         tvm.testing.assert_allclose(out_array, tvm_out)
 
 
+def _test_upsample_nearest3d():
 
 Review comment:
   should be named `test_upsample3d_nearest`

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] zhiics commented on issue #5135: [ONNX]Pool3d & upsample3d op support

Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135#issuecomment-602778625
 
 
   cc @cchung100m and @jwfromm as you guys have more experience on ONNX

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support

Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135#discussion_r396797603
 
 

 ##########
 File path: tests/python/frontend/onnx/test_forward.py
 ##########
 @@ -800,11 +824,45 @@ def _test_upsample_bilinear_opset9():
         tvm.testing.assert_allclose(out_array, tvm_out, rtol=1e-5, atol=1e-5)
 
 
+def _test_upsample_trilinear():
 
 Review comment:
   rename to `_test_upsample3d_trilinear` for clarity.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135#discussion_r399144140
 
 

 ##########
 File path: python/tvm/relay/frontend/onnx.py
 ##########
 @@ -780,19 +784,32 @@ def _impl_v9(cls, inputs, attr, params):
             assert len(inputs) == 2, "Upsample op take 2 inputs, {} given".format(len(inputs))
             scales = params[inputs[1].name_hint].asnumpy()
             inputs = inputs[:1]
-        assert len(scales) == 4 and scales[0] == 1.0 and scales[1] == 1.0
+        assert scales[0] == 1.0 and scales[1] == 1.0
+        input_shape = infer_shape(inputs[0])
+        dims = len(input_shape)
         mode = attr.get('mode')
         if mode == b'nearest':
             method = "nearest_neighbor"
         elif mode == b'linear':
-            method = "bilinear"
+            method = "trilinear" if dims == 5 else "bilinear"
         else:
             raise tvm.error.OpAttributeInvalid(
                 'Value {} in attribute "mode" of operator Upsample is not valid.'.format(mode))
-        attr = {'scale_h': scales[-2], 'scale_w': scales[-1], 'method': method,
-                'layout': 'NCHW', 'align_corners': True}
-        return AttrCvt('upsampling')(inputs, attr)
-
+        attr = {'scale_h': scales[-2],
+                'scale_w': scales[-1],
+                'method': method}
+        if dims == 5:
+            assert len(scales) == 5
+            attr['scale_d'] = scales[-3]
+            attr['layout'] = 'NCDHW'
+            attr['coordinate_transformation_mode'] = 'half_pixel'
 
 Review comment:
   why is this hard coded for half_pixel?

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support

Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135#discussion_r396796719
 
 

 ##########
 File path: python/tvm/relay/frontend/onnx.py
 ##########
 @@ -780,19 +784,36 @@ def _impl_v9(cls, inputs, attr, params):
             assert len(inputs) == 2, "Upsample op take 2 inputs, {} given".format(len(inputs))
             scales = params[inputs[1].name_hint].asnumpy()
             inputs = inputs[:1]
-        assert len(scales) == 4 and scales[0] == 1.0 and scales[1] == 1.0
+        assert scales[0] == 1.0 and scales[1] == 1.0
+        input_shape = infer_shape(inputs[0])
+        dims = len(input_shape)
+        isUpSample3D = bool(dims == 5)
         mode = attr.get('mode')
         if mode == b'nearest':
             method = "nearest_neighbor"
         elif mode == b'linear':
-            method = "bilinear"
+            method = "trilinear" if isUpSample3D else "bilinear"
         else:
             raise tvm.error.OpAttributeInvalid(
                 'Value {} in attribute "mode" of operator Upsample is not valid.'.format(mode))
-        attr = {'scale_h': scales[-2], 'scale_w': scales[-1], 'method': method,
-                'layout': 'NCHW', 'align_corners': True}
-        return AttrCvt('upsampling')(inputs, attr)
-
+        if isUpSample3D:
+            assert len(scales) == 5
+            attr = {'scale_d': scales[-3],
+                    'scale_h': scales[-2],
+                    'scale_w': scales[-1],
+                    'method': method,
+                    'layout': 'NCDHW',
+                    'coordinate_transformation_mode':'half_pixel'}
+            op_name = 'upsampling3d'
+        else:
+            assert len(scales) == 4
+            attr = {'scale_h': scales[-2],
+                    'scale_w': scales[-1],
+                    'method': method,
+                    'layout': 'NCHW',
+                    'align_corners': True}
 
 Review comment:
   Since many of these attributes are shared between 2d and 3d, it'd be more space efficient to define the shared attributes then update the missing / different bits inside the if/else.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] jwfromm commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support

Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135#discussion_r396796126
 
 

 ##########
 File path: python/tvm/relay/frontend/onnx.py
 ##########
 @@ -780,19 +784,36 @@ def _impl_v9(cls, inputs, attr, params):
             assert len(inputs) == 2, "Upsample op take 2 inputs, {} given".format(len(inputs))
             scales = params[inputs[1].name_hint].asnumpy()
             inputs = inputs[:1]
-        assert len(scales) == 4 and scales[0] == 1.0 and scales[1] == 1.0
+        assert scales[0] == 1.0 and scales[1] == 1.0
+        input_shape = infer_shape(inputs[0])
+        dims = len(input_shape)
+        isUpSample3D = bool(dims == 5)
 
 Review comment:
   I'm not a big fan of defining a variable for this as just checking `dims == 5` in line is clear and concise enough. It also makes things cleaner for other Nd operator support.

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


With regards,
Apache Git Services

[GitHub] [incubator-tvm] masahi merged pull request #5135: [ONNX]Pool3d & upsample3d op support

Posted by GitBox <gi...@apache.org>.
masahi merged pull request #5135: [ONNX]Pool3d & upsample3d op support
URL: https://github.com/apache/incubator-tvm/pull/5135
 
 
   

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


With regards,
Apache Git Services