KEMBAR78
[ONNX] shape type inference fixes for control flow by BowenBao · Pull Request #59319 · pytorch/pytorch · GitHub
Skip to content

[ONNX] shape type inference fixes for control flow #59319

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 17, 2021
Merged

[ONNX] shape type inference fixes for control flow #59319

merged 2 commits into from
Jun 17, 2021

Conversation

@BowenBao
Copy link
Collaborator

@BowenBao BowenBao commented Jun 2, 2021 •
edited
Loading

  • Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure. Fixed in onnx submodule 1.9.
  • Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

@facebook-github-bot facebook-github-bot added cla signed oncall: jit Add this issue/PR to JIT oncall triage queue labels Jun 2, 2021
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 2, 2021 •
edited
Loading

💊 CI failures summary and remediations

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

  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-scanned failure(s)

ci.pytorch.org: 1 failed

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.

Click here to manually regenerate this comment.

@BowenBao BowenBao force-pushed the onnx_control_flow_special_case branch from 01244f9 to e31dfa1 Compare June 7, 2021 19:49
@jiafatom
Copy link
Contributor

Can we have a unit test for this? the onnx::Identity failure case

@BowenBao
Copy link
Collaborator Author

Can we have a unit test for this? the onnx::Identity failure case

Updated the description, onnx::identity fix no longer needed with onnx update.

shubhambhokare1
shubhambhokare1 approved these changes Jun 14, 2021
View reviewed changes
Copy link
Collaborator

@shubhambhokare1 shubhambhokare1 left a comment

Choose a reason for hiding this comment

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

LGTM

neginraoof
neginraoof approved these changes Jun 17, 2021
View reviewed changes
Copy link
Contributor

@neginraoof neginraoof left a comment

Choose a reason for hiding this comment

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

Thanks!

@BowenBao BowenBao merged commit c68a0e8 into pytorch:onnx_ms_1 Jun 17, 2021
BowenBao added a commit that referenced this pull request Jun 18, 2021
@BowenBao
[ONNX] shape type inference fixes for control flow (#59319)
08ff81e 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>
BowenBao added a commit that referenced this pull request Jun 18, 2021
@BowenBao
[ONNX] shape type inference fixes for control flow (#59319)
817a9cb 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

[ghstack-poisoned]
This was referenced Jun 18, 2021
Closed
Closed
Closed
Closed
Closed
Closed
Closed
BowenBao added a commit that referenced this pull request Jun 18, 2021
@BowenBao
Update on "[ONNX] shape type inference fixes for control flow (#59319)"
b83b874 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Jun 21, 2021
@BowenBao
Update on "[ONNX] shape type inference fixes for control flow (#59319)"
7f6c144 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Jun 22, 2021
@BowenBao
Update on "[ONNX] shape type inference fixes for control flow (#59319)"
8f126d2 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Jun 22, 2021
@BowenBao
Update on "[ONNX] shape type inference fixes for control flow (#59319)"
a6a3139 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Jun 23, 2021
@BowenBao
Update on "[ONNX] shape type inference fixes for control flow (#59319)"
8e9f8ce 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Jun 25, 2021
@BowenBao
Update on "[ONNX] shape type inference fixes for control flow (#59319)"
9183dcc 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Jun 30, 2021
@BowenBao
Update on "[ONNX] shape type inference fixes for control flow (#59319)"
f48e078 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Jul 6, 2021
@BowenBao
Update on "[ONNX] shape type inference fixes for control flow (#59319)"
d56ea19 
* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Co-authored-by: BowenBao <bowbao@microsoft.com>

Differential Revision: [D29494913](https://our.internmc.facebook.com/intern/diff/D29494913)

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request Jul 8, 2021
@BowenBao @facebook-github-bot
[ONNX] shape type inference fixes for control flow (#59319) (#60248)
00b0d82 
Summary:
Pull Request resolved: #60248

* ~~Allow shape inference to skip for blocks by checking unsupported cases recursively. Currently onnx::Identity would trigger a shape inference failure.~~ Fixed in onnx submodule 1.9.
* Remove previous special post process for if op, since that was for constant folding, and now it is handled elsewhere. Update new post process for control flow nodes to copy assign node shape from subblock output shape correctly.

Test Plan: Imported from OSS

Reviewed By: zou3519, ZolotukhinM

Differential Revision: D29494913

Pulled By: SplitInfinity

fbshipit-source-id: de274a388df86e86403981e1b89b8b4a0d1e26d1

Co-authored-by: BowenBao <bowbao@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shubhambhokare1 shubhambhokare1 shubhambhokare1 approved these changes

@spandantiwari spandantiwari Awaiting requested review from spandantiwari

+1 more reviewer

@neginraoof neginraoof neginraoof approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla signed oncall: jit Add this issue/PR to JIT oncall triage queue open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@BowenBao @facebook-github-bot @jiafatom @neginraoof @shubhambhokare1 @pytorchbot