Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Exporting Mask Rcnn to ONNX #1461

Merged
merged 6 commits into from
Oct 28, 2019

Conversation

lara-hdr
Copy link
Contributor

In addition to #1401 , these are the last changes to export Maskrcnn to ONNX.
pytorch/pytorch#27566 and pytorch/pytorch#26549.
Like for Faster rcnn, for now we will only be able to export with a batch of 1 (until we support dynamic cases for split).

The test is disabled until the referenced PRs are merged and Resize is implemented in opset 11.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks pretty good, thanks!

I'd like to have some more tests for the functions that have custom implementation for ONNX, so that we can enforce that they are not going to be broken in the future by changes to one function or another.

torchvision/models/detection/roi_heads.py Outdated Show resolved Hide resolved
torchvision/models/detection/roi_heads.py Show resolved Hide resolved
torchvision/models/detection/roi_heads.py Show resolved Hide resolved
@fmassa fmassa closed this Oct 23, 2019
@fmassa fmassa reopened this Oct 23, 2019
@codecov-io
Copy link

codecov-io commented Oct 23, 2019

Codecov Report

Merging #1461 into master will decrease coverage by 0.65%.
The diff coverage is 45.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1461      +/-   ##
==========================================
- Coverage   64.34%   63.69%   -0.66%     
==========================================
  Files          83       83              
  Lines        6454     6561     +107     
  Branches      992     1010      +18     
==========================================
+ Hits         4153     4179      +26     
- Misses       2006     2087      +81     
  Partials      295      295
Impacted Files Coverage Δ
torchvision/models/detection/roi_heads.py 54.36% <45.61%> (-1.78%) ⬇️
torchvision/io/video.py 28.32% <0%> (-46.68%) ⬇️
torchvision/datasets/svhn.py 66.66% <0%> (-2.09%) ⬇️
torchvision/datasets/utils.py 57.66% <0%> (-0.72%) ⬇️
torchvision/io/__init__.py 100% <0%> (ø) ⬆️
torchvision/datasets/mnist.py 51.94% <0%> (+0.23%) ⬆️
torchvision/datasets/video_utils.py 65.08% <0%> (+1.77%) ⬆️
torchvision/__init__.py 70.96% <0%> (+3.22%) ⬆️
torchvision/datasets/samplers/clip_sampler.py 79.54% <0%> (+23.98%) ⬆️
torchvision/io/_video_opt.py 65.85% <0%> (+26.49%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f46f2c1...bb5842c. Read the comment docs.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did one more pass, let me know what you think

torchvision/models/detection/roi_heads.py Show resolved Hide resolved
test/test_onnx.py Outdated Show resolved Hide resolved
torchvision/models/detection/roi_heads.py Show resolved Hide resolved
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost good to go, thanks!

There is a lint failure, and I'd like to have one more test to make sure that the scripted model indeed captures control-flow as expected.

test/test_onnx.py Outdated Show resolved Hide resolved
test/test_onnx.py Outdated Show resolved Hide resolved
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot Lara!

@fmassa fmassa merged commit f612182 into pytorch:master Oct 28, 2019
@niaoyu
Copy link

niaoyu commented Nov 2, 2019

LGTM, thanks a lot Lara!

Hi, i just wonder which version of torch and torchvison would be suitable for export maskrcnn to onnx.

I tried the nightly version of both , but all failed

@lara-hdr
Copy link
Contributor Author

lara-hdr commented Nov 3, 2019

@niaoyu, as explained in pytorch/pytorch#27969 there are some PRs that are not yet merged in PyTorch to export this model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants