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/01/17 00:35:29 UTC
[GitHub] [tvm] jwfromm opened a new pull request #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
jwfromm opened a new pull request #7300:
URL: https://github.com/apache/tvm/pull/7300
I noticed that many of our onnx frontend tests compare to results produced by numpy rather than onnx itself. This is somewhat counterproductive because it makes assumptions about what onnx should do rather than checking what it actually does. I decided to go through all the tests in `test_forward.py` and make them use the new `verify_with_ort` helper function. This makes our testing suite more consistent and align more closely with their intention. In the process of making this conversion, I discovered many bugs with the importer that are also fixed in this PR. Although this PR might be a little painful to review due to its scope, I think that the result is an overall much cleaner and easier to maintain test suite.
----------------------------------------------------------------
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 pull request #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
jwfromm commented on pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#issuecomment-761705160
@masahi @mbrookhart can you guys let me know what you think of this PR?
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559262920
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -1065,7 +1062,7 @@ def _impl_v9(cls, inputs, attr, params):
scale_w,
layout=layout,
method=method,
- align_corners=align_corners,
+ align_corners=False,
Review comment:
Interesting. The current ONNX spec says Upsample does only nearest https://github.com/onnx/onnx/blob/master/docs/Operators.md#Upsample, while older versions had `mode` parameter. In either case it doesn't specify what to do with `align_corners`.
Since the spec is dubious and upsample is now deprecated, perhaps we should also warn that upsample is deprecated? Maybe we should always use `method=nearest` as well.
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559492069
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4169,81 +3897,78 @@ def verify_softplus(indata):
if __name__ == "__main__":
- test_flatten()
- test_reshape()
- test_shape()
- test_expand()
- test_power()
- test_squeeze()
- test_unsqueeze()
- test_slice()
- test_floor()
- test_ceil()
- test_round()
- test_isinf()
- test_isnan()
- test_clip()
- test_clip_min_max_as_inputs()
- test_onehot()
- test_matmul()
- test_gather()
- test_gatherelements()
- test_gather_nd()
- test_scatter()
- test_lrn()
- test_instance_norm()
- test_upsample()
- test_forward_min()
- test_forward_max()
- test_forward_mean()
- test_forward_hardsigmoid()
- test_forward_arg_min_max()
- test_softmax()
- test_constantofshape()
- test_all_reduce_funcs()
- test_pad()
- test_split()
- test_binary_ops()
- test_single_ops()
- test_leaky_relu()
- test_elu()
- test_selu()
- test_prelu()
- test_ThresholdedRelu()
- test_ScaledTanh()
- test_ParametricSoftplus()
- test_Scale()
- test_LogSoftmax()
- test_resnet()
- test_inception()
- test_densenet()
- test_sign()
- test_not()
- test_and()
- test_tile()
- test_erf()
- test_where()
- test_or()
- test_depth_to_space()
- test_space_to_depth()
- test_batch_norm()
- test_batch_norm_dynamic_subgraph()
- test_conv()
- test_convtranspose()
- test_unsqueeze_constant()
- test_pooling()
- test_lppool()
- test_lstm()
- test_gru()
- test_resize()
- test_nonzero()
- test_topk()
- test_mod()
- test_xor()
- test_max_roi_pool()
- test_roi_align()
- test_range()
+ # test_flatten()
+ # test_reshape()
+ # test_shape()
+ # test_expand()
+ # test_power()
+ # test_squeeze()
+ # test_unsqueeze()
+ # test_slice()
+ # test_floor()
+ # test_ceil()
+ # test_round()
+ # test_isinf()
+ # test_isnan()
+ # test_clip()
+ # test_clip_min_max_as_inputs()
+ # test_onehot()
+ # test_matmul()
+ # test_gather()
+ # test_gatherelements()
+ # test_gather_nd()
+ # test_scatter()
+ # test_lrn()
+ # test_instance_norm()
+ # test_upsample()
+ # test_forward_min()
+ # test_forward_max()
+ # test_forward_mean()
+ # test_forward_hardsigmoid()
+ # test_forward_arg_min_max()
+ # test_softmax()
+ # test_constantofshape()
+ # test_all_reduce_funcs()
+ # test_pad()
+ # test_split()
+ # test_binary_ops()
+ # test_unary_ops()
+ # test_leaky_relu()
+ # test_elu()
+ # test_selu()
+ # test_prelu()
+ # test_ThresholdedRelu()
+ # test_LogSoftmax()
+ # test_resnet()
+ # test_inception()
+ # test_densenet()
+ # test_sign()
+ # test_not()
+ # test_and()
+ # test_tile()
+ # test_erf()
+ # test_where()
+ # test_or()
+ # test_depth_to_space()
+ # test_space_to_depth()
+ # test_batch_norm()
+ # test_batch_norm_dynamic_subgraph()
+ # test_conv()
+ # test_convtranspose()
+ # test_unsqueeze_constant()
+ # test_pooling()
+ # test_lppool()
+ # test_lstm()
+ # test_gru()
+ # test_resize()
+ # test_nonzero()
+ # test_topk()
+ # test_mod()
+ # test_xor()
+ # test_max_roi_pool()
+ # test_roi_align()
+ # test_range()
test_loop()
- test_size()
- test_maxunpool()
- test_softplus()
+ # test_size()
+ # test_maxunpool()
+ # test_softplus()
Review comment:
Forgot to uncomment them?
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559058386
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4204,15 +3932,12 @@ def verify_softplus(indata):
test_pad()
test_split()
test_binary_ops()
- test_single_ops()
+ test_unary_ops()
test_leaky_relu()
test_elu()
test_selu()
test_prelu()
test_ThresholdedRelu()
- test_ScaledTanh()
- test_ParametricSoftplus()
- test_Scale()
Review comment:
Note that I removed these tests because they don't correspond to real onnx operators.
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559223702
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -1065,7 +1062,7 @@ def _impl_v9(cls, inputs, attr, params):
scale_w,
layout=layout,
method=method,
- align_corners=align_corners,
+ align_corners=False,
Review comment:
Upsample was one of the tests that was against our numpy implementation rather than onnxruntime. Checking tvm's output directly against onnx revealed that we were producing incorrect values for bilinear interpolation and always using `align_corners=False` made the outputs match.
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559740803
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -1065,7 +1062,7 @@ def _impl_v9(cls, inputs, attr, params):
scale_w,
layout=layout,
method=method,
- align_corners=align_corners,
+ align_corners=False,
Review comment:
Upsample is actually entirely deprecated after opset 9, which did use a mode attribute. That little snippet in the opset 10 documentation is just pointing out that its officially now deprecated.
----------------------------------------------------------------
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] tqchen merged pull request #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #7300:
URL: https://github.com/apache/tvm/pull/7300
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559492069
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4169,81 +3897,78 @@ def verify_softplus(indata):
if __name__ == "__main__":
- test_flatten()
- test_reshape()
- test_shape()
- test_expand()
- test_power()
- test_squeeze()
- test_unsqueeze()
- test_slice()
- test_floor()
- test_ceil()
- test_round()
- test_isinf()
- test_isnan()
- test_clip()
- test_clip_min_max_as_inputs()
- test_onehot()
- test_matmul()
- test_gather()
- test_gatherelements()
- test_gather_nd()
- test_scatter()
- test_lrn()
- test_instance_norm()
- test_upsample()
- test_forward_min()
- test_forward_max()
- test_forward_mean()
- test_forward_hardsigmoid()
- test_forward_arg_min_max()
- test_softmax()
- test_constantofshape()
- test_all_reduce_funcs()
- test_pad()
- test_split()
- test_binary_ops()
- test_single_ops()
- test_leaky_relu()
- test_elu()
- test_selu()
- test_prelu()
- test_ThresholdedRelu()
- test_ScaledTanh()
- test_ParametricSoftplus()
- test_Scale()
- test_LogSoftmax()
- test_resnet()
- test_inception()
- test_densenet()
- test_sign()
- test_not()
- test_and()
- test_tile()
- test_erf()
- test_where()
- test_or()
- test_depth_to_space()
- test_space_to_depth()
- test_batch_norm()
- test_batch_norm_dynamic_subgraph()
- test_conv()
- test_convtranspose()
- test_unsqueeze_constant()
- test_pooling()
- test_lppool()
- test_lstm()
- test_gru()
- test_resize()
- test_nonzero()
- test_topk()
- test_mod()
- test_xor()
- test_max_roi_pool()
- test_roi_align()
- test_range()
+ # test_flatten()
+ # test_reshape()
+ # test_shape()
+ # test_expand()
+ # test_power()
+ # test_squeeze()
+ # test_unsqueeze()
+ # test_slice()
+ # test_floor()
+ # test_ceil()
+ # test_round()
+ # test_isinf()
+ # test_isnan()
+ # test_clip()
+ # test_clip_min_max_as_inputs()
+ # test_onehot()
+ # test_matmul()
+ # test_gather()
+ # test_gatherelements()
+ # test_gather_nd()
+ # test_scatter()
+ # test_lrn()
+ # test_instance_norm()
+ # test_upsample()
+ # test_forward_min()
+ # test_forward_max()
+ # test_forward_mean()
+ # test_forward_hardsigmoid()
+ # test_forward_arg_min_max()
+ # test_softmax()
+ # test_constantofshape()
+ # test_all_reduce_funcs()
+ # test_pad()
+ # test_split()
+ # test_binary_ops()
+ # test_unary_ops()
+ # test_leaky_relu()
+ # test_elu()
+ # test_selu()
+ # test_prelu()
+ # test_ThresholdedRelu()
+ # test_LogSoftmax()
+ # test_resnet()
+ # test_inception()
+ # test_densenet()
+ # test_sign()
+ # test_not()
+ # test_and()
+ # test_tile()
+ # test_erf()
+ # test_where()
+ # test_or()
+ # test_depth_to_space()
+ # test_space_to_depth()
+ # test_batch_norm()
+ # test_batch_norm_dynamic_subgraph()
+ # test_conv()
+ # test_convtranspose()
+ # test_unsqueeze_constant()
+ # test_pooling()
+ # test_lppool()
+ # test_lstm()
+ # test_gru()
+ # test_resize()
+ # test_nonzero()
+ # test_topk()
+ # test_mod()
+ # test_xor()
+ # test_max_roi_pool()
+ # test_roi_align()
+ # test_range()
test_loop()
- test_size()
- test_maxunpool()
- test_softplus()
+ # test_size()
+ # test_maxunpool()
+ # test_softplus()
Review comment:
Forgot to uncomment other 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
[GitHub] [tvm] jwfromm commented on a change in pull request #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559058323
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -2138,8 +1928,8 @@ def check_torch_conversion(model, input_size):
# Set verbose=True for more output
torch.onnx.export(model(), dummy_input, file_name, export_params=True, verbose=False)
onnx_model = onnx.load(file_name)
- input_data = np.random.uniform(size=input_size).astype("int32")
- verify_with_ort_with_inputs(onnx_model, [input_data])
+ input_data = np.random.uniform(size=input_size).astype("float32")
+ verify_with_ort_with_inputs(onnx_model, [input_data], apply_softmax=True)
Review comment:
This is a fun one that I wanted to point out. Previously we were casting inputs to `int32`, however because they were generated with `np.random.uniform` they all were just being cast to 0. Using non-zero inputs caused some minor mismatch on outputs due to numerical instability but applying softmax (which torchvision models don't use by default) reduces the numerical difference well below our test threshold.
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559741043
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -1065,7 +1062,7 @@ def _impl_v9(cls, inputs, attr, params):
scale_w,
layout=layout,
method=method,
- align_corners=align_corners,
+ align_corners=False,
Review comment:
I think its still a good idea to keep it in the importer to support older models though.
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
jwfromm commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559741316
##########
File path: tests/python/frontend/onnx/test_forward.py
##########
@@ -4169,81 +3897,78 @@ def verify_softplus(indata):
if __name__ == "__main__":
- test_flatten()
- test_reshape()
- test_shape()
- test_expand()
- test_power()
- test_squeeze()
- test_unsqueeze()
- test_slice()
- test_floor()
- test_ceil()
- test_round()
- test_isinf()
- test_isnan()
- test_clip()
- test_clip_min_max_as_inputs()
- test_onehot()
- test_matmul()
- test_gather()
- test_gatherelements()
- test_gather_nd()
- test_scatter()
- test_lrn()
- test_instance_norm()
- test_upsample()
- test_forward_min()
- test_forward_max()
- test_forward_mean()
- test_forward_hardsigmoid()
- test_forward_arg_min_max()
- test_softmax()
- test_constantofshape()
- test_all_reduce_funcs()
- test_pad()
- test_split()
- test_binary_ops()
- test_single_ops()
- test_leaky_relu()
- test_elu()
- test_selu()
- test_prelu()
- test_ThresholdedRelu()
- test_ScaledTanh()
- test_ParametricSoftplus()
- test_Scale()
- test_LogSoftmax()
- test_resnet()
- test_inception()
- test_densenet()
- test_sign()
- test_not()
- test_and()
- test_tile()
- test_erf()
- test_where()
- test_or()
- test_depth_to_space()
- test_space_to_depth()
- test_batch_norm()
- test_batch_norm_dynamic_subgraph()
- test_conv()
- test_convtranspose()
- test_unsqueeze_constant()
- test_pooling()
- test_lppool()
- test_lstm()
- test_gru()
- test_resize()
- test_nonzero()
- test_topk()
- test_mod()
- test_xor()
- test_max_roi_pool()
- test_roi_align()
- test_range()
+ # test_flatten()
+ # test_reshape()
+ # test_shape()
+ # test_expand()
+ # test_power()
+ # test_squeeze()
+ # test_unsqueeze()
+ # test_slice()
+ # test_floor()
+ # test_ceil()
+ # test_round()
+ # test_isinf()
+ # test_isnan()
+ # test_clip()
+ # test_clip_min_max_as_inputs()
+ # test_onehot()
+ # test_matmul()
+ # test_gather()
+ # test_gatherelements()
+ # test_gather_nd()
+ # test_scatter()
+ # test_lrn()
+ # test_instance_norm()
+ # test_upsample()
+ # test_forward_min()
+ # test_forward_max()
+ # test_forward_mean()
+ # test_forward_hardsigmoid()
+ # test_forward_arg_min_max()
+ # test_softmax()
+ # test_constantofshape()
+ # test_all_reduce_funcs()
+ # test_pad()
+ # test_split()
+ # test_binary_ops()
+ # test_unary_ops()
+ # test_leaky_relu()
+ # test_elu()
+ # test_selu()
+ # test_prelu()
+ # test_ThresholdedRelu()
+ # test_LogSoftmax()
+ # test_resnet()
+ # test_inception()
+ # test_densenet()
+ # test_sign()
+ # test_not()
+ # test_and()
+ # test_tile()
+ # test_erf()
+ # test_where()
+ # test_or()
+ # test_depth_to_space()
+ # test_space_to_depth()
+ # test_batch_norm()
+ # test_batch_norm_dynamic_subgraph()
+ # test_conv()
+ # test_convtranspose()
+ # test_unsqueeze_constant()
+ # test_pooling()
+ # test_lppool()
+ # test_lstm()
+ # test_gru()
+ # test_resize()
+ # test_nonzero()
+ # test_topk()
+ # test_mod()
+ # test_xor()
+ # test_max_roi_pool()
+ # test_roi_align()
+ # test_range()
test_loop()
- test_size()
- test_maxunpool()
- test_softplus()
+ # test_size()
+ # test_maxunpool()
+ # test_softplus()
Review comment:
Whoops, thanks for catching this.
----------------------------------------------------------------
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 #7300: [Relay][Frontend][Onnx] Compare against onnxruntime more consistently during testing
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7300:
URL: https://github.com/apache/tvm/pull/7300#discussion_r559168767
##########
File path: python/tvm/relay/frontend/onnx.py
##########
@@ -1065,7 +1062,7 @@ def _impl_v9(cls, inputs, attr, params):
scale_w,
layout=layout,
method=method,
- align_corners=align_corners,
+ align_corners=False,
Review comment:
why change this? I thought bilinear + `align_corners=True` makes sense?
----------------------------------------------------------------
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