ONNX export: Add Crop, Deconvolution and fix the default stride of Pooling to 1 #12399

ptrendx · 2018-08-29T17:24:15Z

Description

Add operators to ONNX exporter:

Crop
Deconvolution
Fix pooling default stride to 1.
Fixes: No conversion function registered for op type Deconvolution yet #12807

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

vandanavk · 2018-08-29T20:40:21Z

@ptrendx Did you get a chance to test these operators with ONNX? onnx_backend_test.py

Roshrini · 2018-08-30T16:49:01Z

@ptrendx Thanks for submitting this PR. :)
You should add 'test_ConvTranspose2d' in "Basic Model Tests" to make sure "Deconvolution" operator is running fine in this file. /~https://github.com/apache/incubator-mxnet/blob/master/tests/python-pytest/onnx/export/onnx_backend_test.py

Roshrini · 2018-08-30T16:55:44Z

python/mxnet/contrib/onnx/mx2onnx/_op_translations.py

+    pad_dims = list(parse_helper(attrs, "pad", [0, 0]))
+    num_group = int(attrs.get("num_group", 1))
+    dilations = list(parse_helper(attrs, "dilate", [1, 1]))
+    adj_dims = list(parse_helper(attrs, "adj"))


Can you please add default value here?

python/mxnet/contrib/onnx/mx2onnx/_op_translations.py

anirudhacharya

can you add onnx backend tests here -/~https://github.com/apache/incubator-mxnet/blob/master/tests/python-pytest/onnx/export/onnx_backend_test.py

sandeep-krishnamurthy · 2018-09-11T05:10:28Z

@ptrendx - ping :-)

sandeep-krishnamurthy · 2018-09-21T04:22:33Z

@ptrendx - Thanks for your contributions! Can you please address few review comments and we will be good to go?

vandanavk · 2018-09-28T19:51:44Z

ONNX 1.3 has been merged. Please rebase this PR. Thanks.

Roshrini · 2018-10-18T22:47:13Z

@ptrendx Can you please address review comments and rebase this PR? Thanks.

ptrendx · 2018-10-18T22:51:37Z

Hi, I'm sorry for lack of communication on my part. Unfortunately I got pulled into other high priority work and will be able to revisit this PR afterwards.

roywei · 2018-10-29T22:20:58Z

Hi @ptrendx , were you able to get some time on this PR? Do you need any help to continue with this?

kalyc · 2018-11-12T23:08:43Z

Hi @ptrendx - could you please update on the status for this PR? We are looking forward to merge it into MXNet. Incase if you dont have the bandwidth to address comments on the PR, you can close it at the moment and then re-open it whenever you have the time.
Thanks, please let us know!

ptrendx · 2018-11-20T18:37:40Z

Hi, I'm sorry for the long delay in answering the comments. It's rebased onto the latest master.

python/mxnet/contrib/onnx/mx2onnx/_op_translations.py

vandanavk · 2018-11-21T00:22:19Z

python/mxnet/contrib/onnx/mx2onnx/_op_translations.py

+        "Its definition can change.")
+
+    return [crop_node]
+


could you add a test for crop too? - in mxnet_export_test.py maybe

@vandanavk Could you advise where should I put the test for just export? I made the test but am unsure where should it go after the refactor. My understanding is that test_nodes.py is for things that can be both exported and imported, should I put it in mxnet_export_test.py then?

@ptrendx export tests usually perform an import-export-import on an ONNX model and then compare inference results.

if you are planning to add an ONNX import for crop, then adding the test would be straightforward - just add a test_case in test_node.py.

if you are adding export alone for crop, then in test_node.py, create a new test case list (same format as the existing maybe), add a new function test_export() in class TestNode(unittest.TestCase), export to onnx format. not sure how to test inference in this case though.

vandanavk · 2018-11-27T18:20:58Z

@mxnet-label-bot update [pr-awaiting-testing, ONNX]

Roshrini

Thanks for addressing the comments. Added minor comments. Can you please take care of these as well? Thanks!

python/mxnet/contrib/onnx/mx2onnx/_op_translations.py

Roshrini · 2018-12-07T18:36:56Z

python/mxnet/contrib/onnx/mx2onnx/_op_translations.py

+        "Its definition can change.")
+
+    return [crop_node]
+


Roshrini · 2018-12-18T02:30:44Z

@ptrendx Can you address the review comments so that we can merge this PR? :)

Roshrini · 2019-01-02T19:24:53Z

@ptrendx Can you please rebase this PR?

stu1130 · 2019-01-15T00:43:51Z

@Roshrini could you take a look at this?Thanks

Roshrini · 2019-01-18T22:45:06Z

Hi @ptrendx, if you have test ready for crop, can you please add it? Or maybe create a separate PR for crop operator? Other part of this PR looks good and is ready to be merged.

Roshrini · 2019-01-26T00:04:01Z

Thank you for working on this ops @ptrendx :) I will add test case for crop in my PR as I am already adding tests for export ops in this PR: #13981

…oling to 1 (apache#12399) * Added Deconvolution and Crop to ONNX exporter * Added default for pool_type

ptrendx requested a review from szha as a code owner August 29, 2018 17:24

sandeep-krishnamurthy added ONNX pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Aug 30, 2018

Roshrini suggested changes Aug 30, 2018

View reviewed changes

anirudhacharya suggested changes Aug 31, 2018

View reviewed changes

piyushghai mentioned this pull request Oct 12, 2018

No conversion function registered for op type Deconvolution yet #12807

Closed

ptrendx force-pushed the onnx_add_deconvolution_crop branch from fd894b3 to 3075806 Compare November 20, 2018 18:35

vandanavk reviewed Nov 20, 2018

View reviewed changes

python/mxnet/contrib/onnx/mx2onnx/_op_translations.py Outdated Show resolved Hide resolved

vandanavk reviewed Nov 21, 2018

View reviewed changes

python/mxnet/contrib/onnx/mx2onnx/_op_translations.py Outdated Show resolved Hide resolved

vandanavk reviewed Nov 21, 2018

View reviewed changes

marcoabreu added pr-awaiting-testing PR is reviewed and waiting CI build and test ONNX and removed ONNX pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Nov 27, 2018

Roshrini suggested changes Dec 7, 2018

View reviewed changes

Roshrini mentioned this pull request Dec 7, 2018

[Feature Request] Support ONNX export of Deconvolution operator #13570

Closed

Added Deconvolution and Crop to ONNX exporter

abd6951

ptrendx force-pushed the onnx_add_deconvolution_crop branch from ba0b25e to abd6951 Compare January 3, 2019 22:11

Added default for pool_type

889c1cd

vandanavk approved these changes Jan 26, 2019

View reviewed changes

Roshrini approved these changes Jan 26, 2019

View reviewed changes

Roshrini merged commit 25e915b into apache:master Jan 26, 2019

jessr92 pushed a commit to jessr92/incubator-mxnet that referenced this pull request Jan 27, 2019

ONNX export: Add Crop, Deconvolution and fix the default stride of Po…

b43da69

…oling to 1 (apache#12399) * Added Deconvolution and Crop to ONNX exporter * Added default for pool_type

jessr92 pushed a commit to jessr92/incubator-mxnet that referenced this pull request Jan 27, 2019

ONNX export: Add Crop, Deconvolution and fix the default stride of Po…

7ccc12f

…oling to 1 (apache#12399) * Added Deconvolution and Crop to ONNX exporter * Added default for pool_type

haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019

ONNX export: Add Crop, Deconvolution and fix the default stride of Po…

5ce77f2

…oling to 1 (apache#12399) * Added Deconvolution and Crop to ONNX exporter * Added default for pool_type

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ONNX export: Add Crop, Deconvolution and fix the default stride of Pooling to 1 #12399

ONNX export: Add Crop, Deconvolution and fix the default stride of Pooling to 1 #12399

ptrendx commented Aug 29, 2018 •

edited by Roshrini

Loading

vandanavk commented Aug 29, 2018

Roshrini commented Aug 30, 2018

Roshrini Aug 30, 2018

anirudhacharya left a comment

sandeep-krishnamurthy commented Sep 11, 2018

sandeep-krishnamurthy commented Sep 21, 2018

vandanavk commented Sep 28, 2018

Roshrini commented Oct 18, 2018

ptrendx commented Oct 18, 2018

roywei commented Oct 29, 2018

kalyc commented Nov 12, 2018

ptrendx commented Nov 20, 2018

vandanavk Nov 21, 2018

Roshrini Dec 7, 2018

ptrendx Jan 3, 2019

vandanavk Jan 3, 2019

vandanavk commented Nov 27, 2018 •

edited

Loading

Roshrini left a comment

Roshrini Dec 7, 2018

Roshrini commented Dec 18, 2018

Roshrini commented Jan 2, 2019

stu1130 commented Jan 15, 2019

Roshrini commented Jan 18, 2019

Roshrini commented Jan 26, 2019

ONNX export: Add Crop, Deconvolution and fix the default stride of Pooling to 1 #12399

ONNX export: Add Crop, Deconvolution and fix the default stride of Pooling to 1 #12399

Conversation

ptrendx commented Aug 29, 2018 • edited by Roshrini Loading

Description

Checklist

Essentials

vandanavk commented Aug 29, 2018

Roshrini commented Aug 30, 2018

Roshrini Aug 30, 2018

Choose a reason for hiding this comment

anirudhacharya left a comment

Choose a reason for hiding this comment

sandeep-krishnamurthy commented Sep 11, 2018

sandeep-krishnamurthy commented Sep 21, 2018

vandanavk commented Sep 28, 2018

Roshrini commented Oct 18, 2018

ptrendx commented Oct 18, 2018

roywei commented Oct 29, 2018

kalyc commented Nov 12, 2018

ptrendx commented Nov 20, 2018

vandanavk Nov 21, 2018

Choose a reason for hiding this comment

Roshrini Dec 7, 2018

Choose a reason for hiding this comment

ptrendx Jan 3, 2019

Choose a reason for hiding this comment

vandanavk Jan 3, 2019

Choose a reason for hiding this comment

vandanavk commented Nov 27, 2018 • edited Loading

Roshrini left a comment

Choose a reason for hiding this comment

Roshrini Dec 7, 2018

Choose a reason for hiding this comment

Roshrini commented Dec 18, 2018

Roshrini commented Jan 2, 2019

stu1130 commented Jan 15, 2019

Roshrini commented Jan 18, 2019

Roshrini commented Jan 26, 2019

ptrendx commented Aug 29, 2018 •

edited by Roshrini

Loading

vandanavk commented Nov 27, 2018 •

edited

Loading