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.

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

[ONNX] Improve lower tuples and handle control flow #57650

Merged
merged 8 commits into from
May 18, 2021
Merged

[ONNX] Improve lower tuples and handle control flow #57650

merged 8 commits into from
May 18, 2021

Conversation

neginraoof
Copy link
Contributor

Improving the logic for finding tuple patterns within control flow.
Also fixes: #56914

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

facebook-github-bot commented May 5, 2021 •
edited
Loading

💊 CI failures summary and remediations

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

  • 2/2 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_xla_linux_bionic_py3_6_clang9_build (1/1)

Step: "Build" (full log | diagnosis details | 🔁 rerun)

May 18 02:07:44 AssertionError: Found an invalid operator name: _copy_from_and_resize 
May 18 02:07:44   File "/opt/conda/lib/python3.6/runpy.py", line 193, in _run_module_as_main
May 18 02:07:44     "__main__", mod_spec)
May 18 02:07:44   File "/opt/conda/lib/python3.6/runpy.py", line 85, in _run_code
May 18 02:07:44     exec(code, run_globals)
May 18 02:07:44   File "/var/lib/jenkins/workspace/tools/codegen/gen_backend_stubs.py", line 126, in <module>
May 18 02:07:44     main()
May 18 02:07:44   File "/var/lib/jenkins/workspace/tools/codegen/gen_backend_stubs.py", line 87, in main
May 18 02:07:44     cpp_namespace, external_backend_functions = parse_backend_yaml(options.source_yaml, grouped_native_functions)
May 18 02:07:44   File "/var/lib/jenkins/workspace/tools/codegen/gen_backend_stubs.py", line 61, in parse_backend_yaml
May 18 02:07:44     raise AssertionError(f"Found an invalid operator name: {op_name}")
May 18 02:07:44 AssertionError: Found an invalid operator name: _copy_from_and_resize
May 18 02:07:44 ~/workspace/xla
May 18 02:07:44 + OPTS=()
May 18 02:07:44 + getopts O: OPTION
May 18 02:07:44 + case $OPTION in
May 18 02:07:44 + for i in ${OPTARG}
May 18 02:07:44 + OPTS+=("--cxxopt=${i}")
May 18 02:07:44 + getopts O: OPTION
May 18 02:07:44 + shift 2
May 18 02:07:44 + CMD=install
May 18 02:07:44 ++ dirname /var/lib/jenkins/workspace/xla/build_torch_xla_libs.sh

1 failure not recognized by patterns:

Job Step Action
GitHub Actions pytorch_python_doc_build Chown workspace 🔁 rerun
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.

jiafatom
jiafatom reviewed May 7, 2021
View reviewed changes
torch/csrc/jit/passes/lower_tuples.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/lower_tuples.cpp Outdated
if (supported_ops.count(n->kind()) > 0) {
if ((n->kind() == prim::Loop)) {
if (input->node()->kind() == prim::TupleConstruct) {
flattenTupleInBlockParam(n, i);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this function only suitable for prim::Loop? Then we may rename the function name to indicate that...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be used for any blocks with tuple inputs. Renamed it for now.

torch/csrc/jit/passes/lower_tuples.cpp Show resolved Hide resolved
BowenBao
BowenBao reviewed May 12, 2021
View reviewed changes
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.

Looks good overall, some inline comments.

torch/csrc/jit/passes/lower_tuples.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/lower_tuples.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/lower_tuples.cpp Show resolved Hide resolved
torch/csrc/jit/passes/lower_tuples.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/lower_tuples.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/lower_tuples.cpp Show resolved Hide resolved
torch/csrc/jit/passes/lower_tuples.cpp Outdated Show resolved Hide resolved
torch/csrc/jit/passes/lower_tuples.cpp Outdated Show resolved Hide resolved
jiafatom
jiafatom approved these changes May 13, 2021
View reviewed changes
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!

@neginraoof neginraoof closed this May 18, 2021
@neginraoof neginraoof force-pushed the neraoof/lowertuple branch from e0dc9e3 to 94ef2b9 Compare May 18, 2021 00:48
@neginraoof neginraoof reopened this May 18, 2021
@neginraoof neginraoof force-pushed the neraoof/lowertuple branch from e0dc9e3 to 22d1f74 Compare May 18, 2021 01:43
@BowenBao BowenBao merged commit 5d52e90 into pytorch:onnx_ms_1 May 18, 2021
BowenBao pushed a commit that referenced this pull request May 19, 2021
@neginraoof @BowenBao
[ONNX] Improve lower tuples and handle control flow (#57650)
bb72312 
Improving the logic for finding tuple patterns within control flow.
Also fixes: #56914

Co-authored-by: neginraoof <neginmr@utexas.edu>

[ghstack-poisoned]
This was referenced May 19, 2021
Closed
Closed
Closed
Closed
This was referenced May 19, 2021
Closed
Closed
Closed
Closed
BowenBao pushed a commit that referenced this pull request May 19, 2021
@neginraoof @BowenBao
[ONNX] Improve lower tuples and handle control flow (#57650)
8bac7b2 
Improving the logic for finding tuple patterns within control flow.
Also fixes: #56914

Co-authored-by: neginraoof <neginmr@utexas.edu>
BowenBao pushed a commit that referenced this pull request May 20, 2021
@neginraoof @BowenBao
[ONNX] Improve lower tuples and handle control flow (#57650)
f6c7a81 
Improving the logic for finding tuple patterns within control flow.
Also fixes: #56914

Co-authored-by: neginraoof <neginmr@utexas.edu>

[ghstack-poisoned]
This was referenced May 20, 2021
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
BowenBao added a commit that referenced this pull request May 20, 2021
@BowenBao
Update on "[ONNX] Improve lower tuples and handle control flow (#57650)"
966339a 
Improving the logic for finding tuple patterns within control flow.
Also fixes: #56914

Co-authored-by: neginraoof <neginmr@utexas.edu>

[ghstack-poisoned]
BowenBao added a commit that referenced this pull request May 26, 2021
@BowenBao
Update on "[ONNX] Improve lower tuples and handle control flow (#57650)"
b0a824f 
Improving the logic for finding tuple patterns within control flow.
Also fixes: #56914

Co-authored-by: neginraoof <neginmr@utexas.edu>

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

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request May 27, 2021
@BowenBao @neginraoof @facebook-github-bot
[ONNX] Improve lower tuples and handle control flow (#57650) (#58694)
b27fc0f 
Summary:
Pull Request resolved: #58694

Improving the logic for finding tuple patterns within control flow.
Also fixes: #56914

Test Plan: Imported from OSS

Reviewed By: driazati

Differential Revision: D28714806

Pulled By: SplitInfinity

fbshipit-source-id: 1552100cf9cc88e6f58df2e90758e8898ba0a9b3

Co-authored-by: neginraoof <neginmr@utexas.edu>
deniskokarev pushed a commit to deniskokarev/pytorch that referenced this pull request Jun 9, 2021
@BowenBao @neginraoof @deniskokarev
[ONNX] Improve lower tuples and handle control flow (pytorch#57650) (p…
159dcb5 
…ytorch#58694)

Summary:
Pull Request resolved: pytorch#58694

Improving the logic for finding tuple patterns within control flow.
Also fixes: pytorch#56914

Test Plan: Imported from OSS

Reviewed By: driazati

Differential Revision: D28714806

Pulled By: SplitInfinity

fbshipit-source-id: 1552100cf9cc88e6f58df2e90758e8898ba0a9b3

Co-authored-by: neginraoof <neginmr@utexas.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiafatom jiafatom jiafatom approved these changes

@BowenBao BowenBao BowenBao approved these changes

@spandantiwari spandantiwari Awaiting requested review from spandantiwari

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.
5 participants
@neginraoof @facebook-github-bot @BowenBao @jiafatom @pytorchbot