[ONNX] Fix ShuffleNetV2 model export issue. #3158
Conversation
… if dynamic_axes field was provided.
|
Hi @fatcat-z! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
There was a problem hiding this comment.
Thanks for the PR!
Can you also add some basic ONNX tests for shufflenet in https://github.com/pytorch/vision/blob/master/test/test_onnx.py so that we can track this?
Codecov Report
@@ Coverage Diff @@
## master #3158 +/- ##
=======================================
Coverage 72.79% 72.79%
=======================================
Files 98 98
Lines 8964 8964
Branches 1430 1430
=======================================
Hits 6525 6525
Misses 1992 1992
Partials 447 447
Continue to review full report at Codecov.
|
|
@fmassa Thanks for your comments. The test has been added. |
There was a problem hiding this comment.
@fatcat-z LGTM! Thank you!
I tested your unit-test without the patch and it fails as expected. Thanks for submitting the fix.
Summary: * Fix an issue that ShuffleNetV2 model is exported to a wrong ONNX file if dynamic_axes field was provided. * Add a ut for the bug fix. * Fix flake8 issue. * Don't access each element in x.shape, use x.size() instead. Reviewed By: datumbox Differential Revision: D25531034 fbshipit-source-id: 9a7ea77ba6ac5a5b80cb15e7f6cba1a8a47f9289 Co-authored-by: Vasilis Vryniotis <[email protected]>
| def test_shufflenet_v2_dynamic_axes(self): | ||
| model = models.shufflenet_v2_x0_5(pretrained=True) | ||
| dummy_input = torch.randn(1, 3, 224, 224, requires_grad=True) | ||
| test_inputs = torch.cat([dummy_input, dummy_input, dummy_input], 0) |
There was a problem hiding this comment.
Is there a particular reason to concatenate dummy_input to test_inputs? Can I use test_inputs = torch.randn(3, 3, 224, 224, requires_grad=True)?
There was a problem hiding this comment.
@fatcat-z any thoughts on this? Is it so that we can compare the output by passing dummy_input and test_inputs (as they are equal on both dimensions)?
As described in Issue #48093, torch.onnx.export() method failed to export a correct ONNX file for ShuffleNetV2 model while dynamic_axes parameter was provided.
This PR fixes this issue by changing the way to read parameters in channel_shuffle() method.