Skip to content
This repository has been archived by the owner on Mar 12, 2024. It is now read-only.

Fix ONNX export for panotpic model #180

Merged
merged 5 commits into from
Aug 19, 2020
Merged

Conversation

m3at
Copy link
Contributor

@m3at m3at commented Aug 5, 2020

The operation .view_as(an_other_tensor) is not supported by ONNX.
view(an_other_tensor.size()) is identical and supported, this fix allow to export panoptic models.

The operation `.view_as(an_other_tensor)` is not supported by ONNX, `view(an_other_tensor.size())` is identical and supported. This fix allow to export panoptic models.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 5, 2020
Copy link
Contributor

@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.

Thanks for the fix!

Can you add tests for ONNX export for the panoptic model?

m3at added 3 commits August 5, 2020 20:06
Add onnx export test for panoptic model
@m3at
Copy link
Contributor Author

m3at commented Aug 5, 2020

Sure, just adapted the existing one. The CI test got killed midway and I can't relaunch it myself apparently, sorry.

@fmassa
Copy link
Contributor

fmassa commented Aug 5, 2020

I rerun the CI, let's see what happens

@fmassa
Copy link
Contributor

fmassa commented Aug 7, 2020

CI is being killed, maybe it's due to out-of-memory error or something wrong with the ONNX export.

Does it work for your locally?

@m3at
Copy link
Contributor Author

m3at commented Aug 15, 2020

Sorry for the late reply. Yes it work for me locally, I can export the panoptic models as ONNX and they run fine with onnx-runtime.

Copy link
Contributor

@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.

Let's skip this test for now until we increase CI limits and then we re-enable this test.

Once the test is skipped with unittest.skip it will be good to merge.

Thanks a lot!

test_all.py Show resolved Hide resolved
@m3at
Copy link
Contributor Author

m3at commented Aug 19, 2020

Just added your suggestion @fmassa , should be good to merge.
Thanks again for you work on detr 🙂

Copy link
Contributor

@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.

Thanks!

@fmassa fmassa merged commit 5e66b4c into facebookresearch:master Aug 19, 2020
@m3at m3at deleted the patch-1 branch August 20, 2020 03:48
bjuncek pushed a commit to bjuncek/detr that referenced this pull request Apr 28, 2021
* Fix ONNX export for panotpic model

The operation `.view_as(an_other_tensor)` is not supported by ONNX, `view(an_other_tensor.size())` is identical and supported. This fix allow to export panoptic models.

* Update test_all.py

Add onnx export test for panoptic model

* fix lint

* fix lint

* Skip test on OOM CI error
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants