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/04/26 23:05:11 UTC
[GitHub] [tvm] AndrewZhaoLuo opened a new pull request #7928: [WIP] Support dilations in pooling operators
AndrewZhaoLuo opened a new pull request #7928:
URL: https://github.com/apache/tvm/pull/7928
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/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
[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #7928: [ONNX][TOPI][Relay]Support dilations in pooling operators
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #7928:
URL: https://github.com/apache/tvm/pull/7928#discussion_r624149409
##########
File path: tests/python/relay/test_op_level5.py
##########
@@ -671,10 +671,10 @@ def verify_roi_align(
print("test on", target)
intrp1 = relay.create_executor("graph", device=dev, target=target)
op_res1 = intrp1.evaluate(func)(np_data, np_rois)
- tvm.testing.assert_allclose(op_res1.asnumpy(), ref_res, rtol=1e-4)
+ tvm.testing.assert_allclose(op_res1.asnumpy(), ref_res, rtol=2e-4)
intrp2 = relay.create_executor("debug", device=dev, target=target)
op_res2 = intrp2.evaluate(func)(np_data, np_rois)
- tvm.testing.assert_allclose(op_res2.asnumpy(), ref_res, rtol=1e-4)
+ tvm.testing.assert_allclose(op_res2.asnumpy(), ref_res, rtol=2e-4)
Review comment:
I'm not sure actually. Taking a quick look at roi_align, I don't think it should be affected at all. I'm going to try to up the strictness of the test back to before and see if we have the same issues.
--
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] AndrewZhaoLuo commented on a change in pull request #7928: [ONNX][TOPI][Relay]Support dilations in pooling operators
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #7928:
URL: https://github.com/apache/tvm/pull/7928#discussion_r624725442
##########
File path: tests/python/relay/test_op_level5.py
##########
@@ -671,10 +671,10 @@ def verify_roi_align(
print("test on", target)
intrp1 = relay.create_executor("graph", device=dev, target=target)
op_res1 = intrp1.evaluate(func)(np_data, np_rois)
- tvm.testing.assert_allclose(op_res1.asnumpy(), ref_res, rtol=1e-4)
+ tvm.testing.assert_allclose(op_res1.asnumpy(), ref_res, rtol=2e-4)
intrp2 = relay.create_executor("debug", device=dev, target=target)
op_res2 = intrp2.evaluate(func)(np_data, np_rois)
- tvm.testing.assert_allclose(op_res2.asnumpy(), ref_res, rtol=1e-4)
+ tvm.testing.assert_allclose(op_res2.asnumpy(), ref_res, rtol=2e-4)
Review comment:
Ok it passes still with the original tolerances, it was probably due to another bug that was fixed a long time ago
--
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] AndrewZhaoLuo commented on pull request #7928: [ONNX][TOPI][Relay]Support dilations in pooling operators
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #7928:
URL: https://github.com/apache/tvm/pull/7928#issuecomment-829660797
Ok ONNX integration is done now, as should be most frontend tests. (We'll see if CI agrees!)
--
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] AndrewZhaoLuo commented on pull request #7928: [TOPI][Relay]Support dilations in pooling operators
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #7928:
URL: https://github.com/apache/tvm/pull/7928#issuecomment-827964394
Moving this back to [WIP], by request I'm adding onnx integration too.
--
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 #7928: [ONNX][TOPI][Relay]Support dilations in pooling operators
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #7928:
URL: https://github.com/apache/tvm/pull/7928#issuecomment-832073035
Thanks @AndrewZhaoLuo
--
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] AndrewZhaoLuo commented on a change in pull request #7928: [ONNX][TOPI][Relay]Support dilations in pooling operators
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #7928:
URL: https://github.com/apache/tvm/pull/7928#discussion_r624148384
##########
File path: tests/python/contrib/test_arm_compute_lib/test_pooling.py
##########
@@ -17,30 +17,32 @@
"""Arm Compute Library integration pooling tests."""
import numpy as np
-
import tvm
-from tvm import relay
-from tvm import testing
+from tvm import relay, testing
from test_arm_compute_lib.infrastructure import (
- skip_runtime_test,
- skip_codegen_test,
+ Device,
build_and_run,
+ skip_codegen_test,
+ skip_runtime_test,
verify,
verify_codegen,
)
-from test_arm_compute_lib.infrastructure import Device
-def _calculate_output_shape(shape, sizes, padding, strides):
+def _calculate_output_shape(shape, sizes, padding, strides, dilation):
"""Calculate pooling output shape."""
- output_height = ((shape[1] - sizes[0] + padding[0] + padding[2]) / strides[0]) + 1
- output_width = ((shape[2] - sizes[1] + padding[1] + padding[3]) / strides[1]) + 1
+ output_height = (
+ (shape[1] - (sizes[0] - 1) * dilation[0] - 1 + padding[0] + padding[2]) / strides[0]
+ ) + 1
+ output_width = (
+ (shape[2] - (sizes[1] - 1) * dilation[1] - 1 + padding[1] + padding[3]) / strides[1]
+ ) + 1
Review comment:
I've factored out some terms into a "receptive_field" variable. It was the prettiest way I could figure out.
--
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 #7928: [ONNX][TOPI][Relay]Support dilations in pooling operators
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #7928:
URL: https://github.com/apache/tvm/pull/7928#discussion_r623983933
##########
File path: tests/python/relay/test_op_level5.py
##########
@@ -671,10 +671,10 @@ def verify_roi_align(
print("test on", target)
intrp1 = relay.create_executor("graph", device=dev, target=target)
op_res1 = intrp1.evaluate(func)(np_data, np_rois)
- tvm.testing.assert_allclose(op_res1.asnumpy(), ref_res, rtol=1e-4)
+ tvm.testing.assert_allclose(op_res1.asnumpy(), ref_res, rtol=2e-4)
intrp2 = relay.create_executor("debug", device=dev, target=target)
op_res2 = intrp2.evaluate(func)(np_data, np_rois)
- tvm.testing.assert_allclose(op_res2.asnumpy(), ref_res, rtol=1e-4)
+ tvm.testing.assert_allclose(op_res2.asnumpy(), ref_res, rtol=2e-4)
Review comment:
Why did your changes to pooling change the precision of roi_align?
##########
File path: tests/python/contrib/test_arm_compute_lib/test_pooling.py
##########
@@ -17,30 +17,32 @@
"""Arm Compute Library integration pooling tests."""
import numpy as np
-
import tvm
-from tvm import relay
-from tvm import testing
+from tvm import relay, testing
from test_arm_compute_lib.infrastructure import (
- skip_runtime_test,
- skip_codegen_test,
+ Device,
build_and_run,
+ skip_codegen_test,
+ skip_runtime_test,
verify,
verify_codegen,
)
-from test_arm_compute_lib.infrastructure import Device
-def _calculate_output_shape(shape, sizes, padding, strides):
+def _calculate_output_shape(shape, sizes, padding, strides, dilation):
"""Calculate pooling output shape."""
- output_height = ((shape[1] - sizes[0] + padding[0] + padding[2]) / strides[0]) + 1
- output_width = ((shape[2] - sizes[1] + padding[1] + padding[3]) / strides[1]) + 1
+ output_height = (
+ (shape[1] - (sizes[0] - 1) * dilation[0] - 1 + padding[0] + padding[2]) / strides[0]
+ ) + 1
+ output_width = (
+ (shape[2] - (sizes[1] - 1) * dilation[1] - 1 + padding[1] + padding[3]) / strides[1]
+ ) + 1
Review comment:
nitpick: the way this got formatted is hard to read. Perhaps an extra set of parentheses outside would help?
--
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 merged pull request #7928: [ONNX][TOPI][Relay]Support dilations in pooling operators
Posted by GitBox <gi...@apache.org>.
mbrookhart merged pull request #7928:
URL: https://github.com/apache/tvm/pull/7928
--
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