Fix standalone compile for op with multiple outputs by Valentine233 · Pull Request #96936 · pytorch/pytorch

Valentine233 · 2023-03-16T06:15:30Z

Op-benchmark directly uses fx.Graph to create nodes without dynamo and then compiles the graph with inductor. Currently, operators with multiple outputs, e.g. native_layer_norm, would fail to run caused by standalone torch._inductor.compile() API #95594. Actually, the graph's result is a node with several outputs instead of a tuple with several nodes. However, the standalone API forces a non-tuple result be a tuple, i.e., a tuple with one node-type element with several outputs. This PR considers a return node with several outputs as a tuple to avoid errors.

cc @soumith @voznesenskym @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @desertfire

pytorch-bot · 2023-03-16T06:15:33Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/96936

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 26ef729:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

jgong5

LGTM except for a small nit in the test code.

jgong5 · 2023-03-17T03:09:55Z

test/inductor/test_standalone_compile.py

can we pass torch.ops.aten.native_layer_norm.default directly to gen_gm_and_inputs?

Valentine233 · 2023-03-19T02:42:12Z

@pytorchbot merge

pytorchmergebot · 2023-03-19T02:43:58Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

kit1980 · 2023-03-19T20:28:34Z

@pytorchbot revert -m "Broke inductor tests on macos-12-py3-arm64 https://github.com/pytorch/pytorch/actions/runs/4458548491/jobs/7830566793" -c nosignal

kit1980 · 2023-03-19T20:29:16Z

@Valentine233 sorry I have to revert this. Please fix for macos-12-py3-arm64 and re-land.

pytorchmergebot · 2023-03-19T20:32:07Z

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot · 2023-03-19T20:32:18Z

@Valentine233 your PR has been successfully reverted.

This reverts commit 37cde56. Reverted #96936 on behalf of https://github.com/kit1980 due to Broke inductor tests on macos-12-py3-arm64 https://github.com/pytorch/pytorch/actions/runs/4458548491/jobs/7830566793

kit1980 · 2023-03-19T21:43:43Z

Looks like this PR was not the reason for the test failure, it was a mistake to revert it.
I'll reland it soon.

kit1980 · 2023-03-20T01:18:52Z

@pytorchbot rebase

pytorchmergebot · 2023-03-20T01:20:53Z

@pytorchbot successfully started a rebase job. Check the current status here

pytorchmergebot · 2023-03-20T01:20:56Z

Rebase failed due to

Aborting rebase because rebasing the branch resulted in the same sha as the target branch.
This usually happens because the PR has already been merged.  Please rebase locally and push.

Raised by https://github.com/pytorch/pytorch/actions/runs/4463722719

kit1980 · 2023-03-20T01:57:37Z

@pytorchbot merge

pytorchmergebot · 2023-03-20T02:02:43Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2023-03-20T03:14:28Z

Merge failed

Reason: 1 jobs have failed, first few of them are: periodic / linux-focal-rocm5.4.2-py3.8 / test (distributed, 1, 2, linux.rocm.gpu)

Details for Dev Infra team

Raised by workflow job

kit1980 · 2023-03-20T06:27:13Z

@pytorchbot merge

pytorchmergebot · 2023-03-20T06:29:05Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

Op-benchmark directly uses fx.Graph to create nodes without dynamo and then compiles the graph with inductor. Currently, operators with multiple outputs, e.g. native_layer_norm, would fail to run caused by standalone torch._inductor.compile() API #95594. Actually, the graph's result is a node with several outputs instead of a tuple with several nodes. However, the standalone API forces a non-tuple result be a tuple, i.e., a tuple with one node-type element with several outputs. This PR considers a return node with several outputs as a tuple to avoid errors. Pull Request resolved: pytorch/pytorch#96936 Approved by: https://github.com/jgong5, https://github.com/jansel

This reverts commit 37cde56. Reverted pytorch/pytorch#96936 on behalf of https://github.com/kit1980 due to Broke inductor tests on macos-12-py3-arm64 https://github.com/pytorch/pytorch/actions/runs/4458548491/jobs/7830566793

Op-benchmark directly uses fx.Graph to create nodes without dynamo and then compiles the graph with inductor. Currently, operators with multiple outputs, e.g. native_layer_norm, would fail to run caused by standalone torch._inductor.compile() API #95594. Actually, the graph's result is a node with several outputs instead of a tuple with several nodes. However, the standalone API forces a non-tuple result be a tuple, i.e., a tuple with one node-type element with several outputs. This PR considers a return node with several outputs as a tuple to avoid errors. Pull Request resolved: pytorch/pytorch#96936 Approved by: https://github.com/jgong5, https://github.com/jansel

This reverts commit 37cde56. Reverted pytorch/pytorch#96936 on behalf of https://github.com/kit1980 due to Broke inductor tests on macos-12-py3-arm64 https://github.com/pytorch/pytorch/actions/runs/4458548491/jobs/7830566793

Op-benchmark directly uses fx.Graph to create nodes without dynamo and then compiles the graph with inductor. Currently, operators with multiple outputs, e.g. native_layer_norm, would fail to run caused by standalone torch._inductor.compile() API #95594. Actually, the graph's result is a node with several outputs instead of a tuple with several nodes. However, the standalone API forces a non-tuple result be a tuple, i.e., a tuple with one node-type element with several outputs. This PR considers a return node with several outputs as a tuple to avoid errors. Pull Request resolved: pytorch/pytorch#96936 Approved by: https://github.com/jgong5, https://github.com/jansel

github-actions bot added ciflow/inductor module: inductor labels Mar 16, 2023

pytorchbot added the open source label Mar 16, 2023

Valentine233 added the topic: not user facing topic category label Mar 16, 2023

ezyang requested review from jansel and jgong5 March 16, 2023 16:23

ezyang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Mar 16, 2023

Valentine233 force-pushed the fix_standalone_compile branch from 7dcfa56 to c8ce8e2 Compare March 17, 2023 02:21

Valentine233 added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 17, 2023

Valentine233 requested a review from EikanWang March 17, 2023 02:54

jgong5 approved these changes Mar 17, 2023

View reviewed changes

Valentine233 force-pushed the fix_standalone_compile branch from c8ce8e2 to 26ef729 Compare March 17, 2023 03:22

fix standalone compile for op with multiple outputs

26ef729

jansel approved these changes Mar 18, 2023

View reviewed changes

pytorchmergebot added the Merged label Mar 19, 2023

pytorchmergebot closed this in 37cde56 Mar 19, 2023

pytorchmergebot added the Reverted label Mar 19, 2023

kit1980 reopened this Mar 20, 2023

kit1980 added the ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR label Mar 20, 2023

kit1980 removed the Reverted label Mar 20, 2023

pytorchmergebot closed this in 16d8516 Mar 20, 2023

github-actions bot deleted the fix_standalone_compile branch September 17, 2024 01:52

Conversation

Valentine233 commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/96936

✅ No Failures

Uh oh!

jgong5 left a comment

Choose a reason for hiding this comment

Uh oh!

jgong5 Mar 17, 2023

Choose a reason for hiding this comment

Uh oh!

Valentine233 Mar 17, 2023

Choose a reason for hiding this comment

Uh oh!

Valentine233 commented Mar 19, 2023

Uh oh!

pytorchmergebot commented Mar 19, 2023

Merge started

Uh oh!

kit1980 commented Mar 19, 2023

Uh oh!

kit1980 commented Mar 19, 2023

Uh oh!

pytorchmergebot commented Mar 19, 2023

Uh oh!

pytorchmergebot commented Mar 19, 2023

Uh oh!

kit1980 commented Mar 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kit1980 commented Mar 20, 2023

Uh oh!

pytorchmergebot commented Mar 20, 2023

Uh oh!

pytorchmergebot commented Mar 20, 2023

Uh oh!

kit1980 commented Mar 20, 2023

Uh oh!

pytorchmergebot commented Mar 20, 2023

Merge started

Uh oh!

pytorchmergebot commented Mar 20, 2023

Merge failed

Uh oh!

kit1980 commented Mar 20, 2023

Uh oh!

pytorchmergebot commented Mar 20, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Valentine233 commented Mar 16, 2023 •

edited

Loading

pytorch-bot bot commented Mar 16, 2023 •

edited

Loading

kit1980 commented Mar 19, 2023 •

edited

Loading