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

[ONNX] Add a post-pass for If folding #49410

Merged
merged 33 commits into from
Jan 6, 2021
Merged

Conversation

KsenijaS
Copy link
Contributor

@KsenijaS KsenijaS commented Dec 15, 2020

Currently ONNX Runtime is doing Shape and Type Inference on both branches of the If operator, regardless of which branch is executing in runtime. This can cause Runtime errors in some cases:

  • Condition of the If node is based on shape / size of the input
  • then and else branch have different return types

This pass is folding an If node and it enables following tests:

  • test_embedding_sequential_1
  • test_embedding_sequential_2
  • test_nllloss
  • test_crossentropy
  • test_embedding_bag_1d_per_sample_weights
  • test_embedding_bag_2d_per_sample_weights

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Dec 15, 2020

💊 CI failures summary and remediations

As of commit c652fee (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

This comment has been revised 101 times.

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Dec 15, 2020
torch/csrc/jit/passes/onnx/shape_type_inference.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/onnx/fold_if_node.h Outdated Show resolved Hide resolved
torch/csrc/jit/passes/onnx/fold_if_node.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/onnx/fold_if_node.cpp Show resolved Hide resolved
torch/csrc/jit/passes/onnx/fold_if_node.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/onnx/fold_if_node.cpp Show resolved Hide resolved
torch/csrc/jit/passes/onnx/fold_if_node.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@BowenBao BowenBao left a comment

Choose a reason for hiding this comment

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

LGTM, minor suggestions.

torch/csrc/jit/passes/onnx/fold_if_node.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/onnx/shape_type_inference.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@jiafatom jiafatom left a comment

Choose a reason for hiding this comment

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

LGTM!

@BowenBao BowenBao merged commit 33f26ed into pytorch:onnx_ms_1 Jan 6, 2021
spandantiwari pushed a commit to spandantiwari/pytorch that referenced this pull request Jan 8, 2021
* add pass

* add tests

* enable tests

* update dtype symbolic

* update pass

* clang format

* update utils file

* fix clang_tidy errors

* update pass

* update pass

* clang format

* update pass

* update pass

* update pass

* update pass

* update pass

* update pass

* update pass

* update pass

* fix mypy tests

* update comment

* update pass

* add new line

* update pass

* empty commit

* empty commit

* fix merge conflict
facebook-github-bot pushed a commit that referenced this pull request Jan 13, 2021
Summary:
[ONNX] ONNX dev branch merge 01-06-2021
- [ONNX] Support onnx if/loop sequence output in opset 13 - (#49270)
- Symbolic function for torch.square (#49446)
- [ONNX] Add checks in ONNXSetDynamicInputShape (#49783) …
- [ONNX] Enable export af aten::__derive_index (#49514) …
- [ONNX] Update symbolic for unfold (#49378) …
- [ONNX] Update the sequence of initializers in exported graph so that it is as same as inputs. (#49798)
- [ONNX] Enable opset 13 ops (#49612) …
- [ONNX] Improve error message for supported model input types in ONNX export API. (#50119)
- [ONNX] Add a post-pass for If folding (#49410)

Pull Request resolved: #50163

Reviewed By: pbelevich

Differential Revision: D25821059

Pulled By: SplitInfinity

fbshipit-source-id: 9f511a93d9d5812d0ab0a49d61ed0fa5f8066948
BowenBao pushed a commit that referenced this pull request Jan 20, 2021
…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.
BowenBao added a commit that referenced this pull request Jan 21, 2021
…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]
BowenBao added a commit that referenced this pull request Jan 21, 2021
…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]
BowenBao added a commit that referenced this pull request Jan 22, 2021
…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]
BowenBao added a commit that referenced this pull request Jan 22, 2021
…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]
BowenBao added a commit that referenced this pull request Jan 25, 2021
…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]
BowenBao added a commit that referenced this pull request Jan 25, 2021
…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]
BowenBao added a commit that referenced this pull request Jan 26, 2021
…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]
facebook-github-bot pushed a commit that referenced this pull request Jan 28, 2021
…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
BowenBao added a commit to BowenBao/pytorch that referenced this pull request Jan 28, 2021
…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: 53b04161a00c3f7ae959dada3780be360e3071b7
Pull Request resolved: pytorch#50910
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed oncall: jit Add this issue/PR to JIT oncall triage queue open source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants