[ONNX] Enable _jit_pass_onnx_fold_if only when dynamic_axes is None by jiafatom · Pull Request #50582 · pytorch/pytorch

jiafatom · 2021-01-15T15:02:19Z

Fixing pytorch/vision#3251 (PR #49410 triggers the torch vision test build failure, on three tests test_faster_rcnn, test_mask_rcnn, test_keypoint_rcnn. )

The offending PR is fine on pytorch UT, because the torchvision and pytorch test has a gap when we merge them - we are using different test API on two sides, therefore causing some discrepancy.

This PR bridge the gap for the above three tests, and ~~disable _jit_pass_onnx_fold_if pass until it gets fixed.~~
Allow _jit_pass_onnx_fold_if only when dynamic_axes is None.

…into test_faster_rcnn

BowenBao

Could you add more info as for how fold_if pass is failing?

torch/onnx/utils.py

test/onnx/test_pytorch_onnx_onnxruntime.py

jiafatom · 2021-01-15T22:24:26Z

Could you add more info as for how fold_if pass is failing?

For test_faster_rcnn, when I see the onnx model converted using dummy input, it has only one NonMaxSuppression node,
but when I see the onnx model converted using a true image, it has two NonMaxSuppression nodes.
This is because of the logic that handles differently on some conditions.
For the existing code, it does not particularly handle dynamic_axes logic, then it wants to treat the input axes as constant and do some folding. Then it gives error when I have a different input shape.

…into test_faster_rcnn

test/onnx/test_pytorch_onnx_onnxruntime.py

…into test_faster_rcnn

KsenijaS · 2021-01-19T18:18:37Z

dynamic axes are checked on this line of code: https://github.com/pytorch/pytorch/blob/master/torch/csrc/jit/passes/onnx/fold_if_node.cpp#L54

…into test_faster_rcnn

jiafatom · 2021-01-20T17:25:35Z

dynamic axes are checked on this line of code: https://github.com/pytorch/pytorch/blob/master/torch/csrc/jit/passes/onnx/fold_if_node.cpp#L54

Sure. Let's check in this PR first to include the torch vision UT. For the code as mentioned, it breaks current UT, we can fix that in a separate PR.

neginraoof

Looks good. Please follow up about the disabled pass. Thanks.

…50582) Fixing pytorch/vision#3251 (PR #49410 triggers the torch vision test build failure, on three tests test_faster_rcnn, test_mask_rcnn, test_keypoint_rcnn. ) The offending PR is fine on pytorch UT, because the torchvision and pytorch test has a gap when we merge them - we are using different test API on two sides, therefore causing some discrepancy. This PR bridge the gap for the above three tests, and disable _jit_pass_onnx_fold_if pass until it gets fixed. Allow _jit_pass_onnx_fold_if only when dynamic_axes is None. [ghstack-poisoned]

…s is None (#50582)" Fixing pytorch/vision#3251 (PR #49410 triggers the torch vision test build failure, on three tests test_faster_rcnn, test_mask_rcnn, test_keypoint_rcnn. ) The offending PR is fine on pytorch UT, because the torchvision and pytorch test has a gap when we merge them - we are using different test API on two sides, therefore causing some discrepancy. This PR bridge the gap for the above three tests, and disable _jit_pass_onnx_fold_if pass until it gets fixed. Allow _jit_pass_onnx_fold_if only when dynamic_axes is None. [ghstack-poisoned]

…s is None (#50582)" Fixing pytorch/vision#3251 (PR #49410 triggers the torch vision test build failure, on three tests test_faster_rcnn, test_mask_rcnn, test_keypoint_rcnn. ) The offending PR is fine on pytorch UT, because the torchvision and pytorch test has a gap when we merge them - we are using different test API on two sides, therefore causing some discrepancy. This PR bridge the gap for the above three tests, and disable _jit_pass_onnx_fold_if pass until it gets fixed. Allow _jit_pass_onnx_fold_if only when dynamic_axes is None. Differential Revision: [D26023934](https://our.internmc.facebook.com/intern/diff/D26023934) [ghstack-poisoned]

…s is None (#50582)" Fixing pytorch/vision#3251 (PR #49410 triggers the torch vision test build failure, on three tests test_faster_rcnn, test_mask_rcnn, test_keypoint_rcnn. ) The offending PR is fine on pytorch UT, because the torchvision and pytorch test has a gap when we merge them - we are using different test API on two sides, therefore causing some discrepancy. This PR bridge the gap for the above three tests, and disable _jit_pass_onnx_fold_if pass until it gets fixed. Allow _jit_pass_onnx_fold_if only when dynamic_axes is None. [ghstack-poisoned]

…s is None (#50582)" Fixing pytorch/vision#3251 (PR #49410 triggers the torch vision test build failure, on three tests test_faster_rcnn, test_mask_rcnn, test_keypoint_rcnn. ) The offending PR is fine on pytorch UT, because the torchvision and pytorch test has a gap when we merge them - we are using different test API on two sides, therefore causing some discrepancy. This PR bridge the gap for the above three tests, and disable _jit_pass_onnx_fold_if pass until it gets fixed. Allow _jit_pass_onnx_fold_if only when dynamic_axes is None. Differential Revision: [D26050886](https://our.internmc.facebook.com/intern/diff/D26050886) [ghstack-poisoned]

…50582) (#50910) Summary: Pull Request resolved: #50910 Fixing pytorch/vision#3251 (PR #49410 triggers the torch vision test build failure, on three tests test_faster_rcnn, test_mask_rcnn, test_keypoint_rcnn. ) The offending PR is fine on pytorch UT, because the torchvision and pytorch test has a gap when we merge them - we are using different test API on two sides, therefore causing some discrepancy. This PR bridge the gap for the above three tests, and disable _jit_pass_onnx_fold_if pass until it gets fixed. Allow _jit_pass_onnx_fold_if only when dynamic_axes is None. Test Plan: Imported from OSS Reviewed By: pbelevich Differential Revision: D26050886 Pulled By: SplitInfinity fbshipit-source-id: b765ffe30914261866dcc761f0d0999fd16169e3

…ytorch#50582) Fixing pytorch/vision#3251 (PR pytorch#49410 triggers the torch vision test build failure, on three tests test_faster_rcnn, test_mask_rcnn, test_keypoint_rcnn. ) The offending PR is fine on pytorch UT, because the torchvision and pytorch test has a gap when we merge them - we are using different test API on two sides, therefore causing some discrepancy. This PR bridge the gap for the above three tests, and disable _jit_pass_onnx_fold_if pass until it gets fixed. Allow _jit_pass_onnx_fold_if only when dynamic_axes is None. ghstack-source-id: 53b0416 Pull Request resolved: pytorch#50910

Modify test_faster_rcnn

085938d

jiafatom changed the title ~~[ONNX] Modify test_faster_rcnn to adapt torch vision test~~ [WIP][ONNX] Modify test_faster_rcnn to adapt torch vision test Jan 15, 2021

jiafatom changed the title ~~[WIP][ONNX] Modify test_faster_rcnn to adapt torch vision test~~ [WIP][ONNX] Modify test_faster_rcnn to adopt torch vision test Jan 15, 2021

facebook-github-bot added the cla signed label Jan 15, 2021

pytorchbot added the open source label Jan 15, 2021

jiafatom added 2 commits January 15, 2021 08:31

Enable if_fold pass only when dynamic_axes is None

48c5b89

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

cd7990c

…into test_faster_rcnn

jiafatom changed the title ~~[WIP][ONNX] Modify test_faster_rcnn to adopt torch vision test~~ [WIP][ONNX] Enable _jit_pass_onnx_fold_if only when dynamic_axes is None Jan 15, 2021

jiafatom mentioned this pull request Jan 15, 2021

ONNX tests failing on master pytorch/vision#3251

Closed

jiafatom changed the title ~~[WIP][ONNX] Enable _jit_pass_onnx_fold_if only when dynamic_axes is None~~ [Waiting for build][ONNX] Enable _jit_pass_onnx_fold_if only when dynamic_axes is None Jan 15, 2021

jiafatom changed the title ~~[Waiting for build][ONNX] Enable _jit_pass_onnx_fold_if only when dynamic_axes is None~~ [ONNX] Enable _jit_pass_onnx_fold_if only when dynamic_axes is None Jan 15, 2021

BowenBao reviewed Jan 15, 2021

View reviewed changes

torch/onnx/utils.py Show resolved Hide resolved

test/onnx/test_pytorch_onnx_onnxruntime.py Outdated Show resolved Hide resolved

test/onnx/test_pytorch_onnx_onnxruntime.py Outdated Show resolved Hide resolved

jiafatom added 3 commits January 15, 2021 21:32

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

23c12c6

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

40236cf

…into test_faster_rcnn

Merge branch 'onnx_ms_1' into test_faster_rcnn

7893d39

jiafatom commented Jan 16, 2021

View reviewed changes

test/onnx/test_pytorch_onnx_onnxruntime.py Show resolved Hide resolved

jiafatom added 8 commits January 16, 2021 08:30

Merge branch 'onnx_ms_1' into test_faster_rcnn

6d585f7

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

3633627

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

cf58efb

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

524ff40

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

5e5fedb

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

5ca5e7e

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

fe154c8

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

0c89262

…into test_faster_rcnn

jiafatom added 4 commits January 19, 2021 14:04

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

2620694

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

728c19e

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

ce9da99

…into test_faster_rcnn

Merge branch 'test_faster_rcnn' of https://github.com/jiafatom/pytorch …

354526f

…into test_faster_rcnn

Merge branch 'onnx_ms_1' into test_faster_rcnn

74741dc

neginraoof approved these changes Jan 20, 2021

View reviewed changes

BowenBao merged commit 5e24665 into pytorch:onnx_ms_1 Jan 20, 2021

jiafatom deleted the test_faster_rcnn branch January 20, 2021 18:52

This was referenced Jan 22, 2021

[ONNX] Replace optional parameters of Resize with placeholder for ops13. (#50574) #50954

Closed

[ONNX] Fix param names (#50764) #50955

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ONNX] Enable _jit_pass_onnx_fold_if only when dynamic_axes is None#50582

[ONNX] Enable _jit_pass_onnx_fold_if only when dynamic_axes is None#50582
BowenBao merged 19 commits intopytorch:onnx_ms_1from
jiafatom:test_faster_rcnn

jiafatom commented Jan 15, 2021 •

edited

Loading

Uh oh!

BowenBao left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiafatom commented Jan 15, 2021

Uh oh!

Uh oh!

KsenijaS commented Jan 19, 2021

Uh oh!

jiafatom commented Jan 20, 2021

Uh oh!

neginraoof left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

jiafatom commented Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BowenBao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiafatom commented Jan 15, 2021

Uh oh!

Uh oh!

KsenijaS commented Jan 19, 2021

Uh oh!

jiafatom commented Jan 20, 2021

Uh oh!

neginraoof left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jiafatom commented Jan 15, 2021 •

edited

Loading