[core IR] Add lift_fresh, split.Tensor, and unbind decompositions to core ATen decomp table #110102

SS-JIA · 2023-09-26T19:58:20Z

Stack from ghstack (oldest at bottom):

-> [core IR] Add lift_fresh, split.Tensor, and unbind decompositions to core ATen decomp table #110102

Context

Add existing decomps for lift_fresh, split.Tensor, and unbind to the core ATen decomposition table. Do not use them in inductor, since Inductor currently lowers these directly.

One note though is that lift_fresh's decomposition has a note saying it's not correct under autograd. However, my understanding is that these decompositions are registered to the "post_autograd" decomposition table, meaning autograd wouldn't be a factor. Would like some confirmation that this premise is correct.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov @ColinPeppler

…p table [ghstack-poisoned]

pytorch-bot · 2023-09-26T19:58:23Z

🔗 Helpful Links

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

📄 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, 1 Pending

As of commit 230b584 with merge base 34ded74 ():
💚 Looks good so far! There are no failures yet. 💚

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

… ATen decomp table" ## Context Add existing decomps for `lift_fresh` and `unbind` to the core ATen decomposition table. Do not use them in inductor, since Inductor currently lowers these directly. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

…p table ghstack-source-id: e801c7f Pull Request resolved: #110102

lezcano · 2023-09-26T20:25:48Z

torch/_decomp/__init__.py

            aten.leaky_relu_backward,
            aten.lerp,
            aten.lerp_,
+            aten.lift_fresh,


The lift_fresh decomposition is just a noop, and it has a note saying that it's not correct under autograd. Do we want to add it as a core decomp?

My understanding is that the core decompositions are registered to the "post_autograd" decomposition table (see torch/_decomp/__init__.py), presumably that means that these decompositions should be used when autograd is not a factor.

This is a good point though, would like to get additional confirmation that what I said above makes sense.

cc: @SherlockNoMad @bdhirsh @jansel

hmm @ezyang - today when we see a tensor constant during tracing, that turns into a constant_lifted = aten.lift_fresh.default(self._tensor_constant0) in the graph. Do you think lift_fresh should be a first-class concept for backends?

In a functionalized graph the no-op decomp is OK.

In a graph with mutation, it is required to clone the constant, because there may be a mutation that modifies the module attribute.

I don't really have an opinion outside of this requirement.

we guarantee that a graph of core ATen IR is always functional (right @SherlockNoMad?). Then it sounds reasonable for core aten to no-op lift_fresh.

… ATen decomp table" ## Context Add existing decomps for `lift_fresh` and `unbind` to the core ATen decomposition table. Do not use them in inductor, since Inductor currently lowers these directly. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

…core ATen decomp table ghstack-source-id: 2123513 Pull Request resolved: #110102

…sitions to core ATen decomp table" ## Context Add existing decomps for `lift_fresh`, `split.Tensor`, and `unbind` to the core ATen decomposition table. Do not use them in inductor, since Inductor currently lowers these directly. One note though is that `lift_fresh`'s decomposition has a note saying it's not correct under autograd. However, my understanding is that these decompositions are registered to the `"post_autograd"` decomposition table, meaning autograd wouldn't be a factor. Would like some confirmation that this premise is correct. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

…core ATen decomp table ghstack-source-id: 1224515 Pull Request resolved: #110102

SS-JIA · 2023-09-27T16:34:50Z

@pytorchbot merge

pytorchmergebot · 2023-09-27T16:36:31Z

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

…sitions to core ATen decomp table" ## Context Add existing decomps for `lift_fresh`, `split.Tensor`, and `unbind` to the core ATen decomposition table. Do not use them in inductor, since Inductor currently lowers these directly. One note though is that `lift_fresh`'s decomposition has a note saying it's not correct under autograd. However, my understanding is that these decompositions are registered to the `"post_autograd"` decomposition table, meaning autograd wouldn't be a factor. Would like some confirmation that this premise is correct. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

pytorchmergebot · 2023-09-27T20:49:52Z

Merge failed

Reason: New commits were pushed while merging. Please rerun the merge command.

Details for Dev Infra team

Raised by workflow job

…sitions to core ATen decomp table" ## Context Add existing decomps for `lift_fresh`, `split.Tensor`, and `unbind` to the core ATen decomposition table. Do not use them in inductor, since Inductor currently lowers these directly. One note though is that `lift_fresh`'s decomposition has a note saying it's not correct under autograd. However, my understanding is that these decompositions are registered to the `"post_autograd"` decomposition table, meaning autograd wouldn't be a factor. Would like some confirmation that this premise is correct. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

…core ATen decomp table ghstack-source-id: 75a04b1 Pull Request resolved: #110102

SS-JIA · 2023-09-27T20:56:50Z

@pytorchbot merge

pytorchmergebot · 2023-09-27T20:58:33Z

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

atalman · 2023-09-28T19:01:00Z

@pytorchbot -m "Breaks internal CI" -c ghfirst

pytorch-bot · 2023-09-28T19:01:03Z

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: argument command: invalid choice: 'Breaks internal CI' (choose from 'merge', 'revert', 'rebase', 'label', 'drci')

usage: @pytorchbot [-h] {merge,revert,rebase,label,drci} ...

Try @pytorchbot --help for more info.

atalman · 2023-09-28T19:01:35Z

@pytorchbot revert -m "Breaks internal CI" -c ghfirst

pytorchmergebot · 2023-09-28T19:03:20Z

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

pytorchmergebot · 2023-09-28T19:03:30Z

@SS-JIA your PR has been successfully reverted.

…ions to core ATen decomp table (#110102)" This reverts commit 22e706f. Reverted #110102 on behalf of https://github.com/atalman due to Breaks internal CI ([comment](#110102 (comment)))

bdhirsh · 2023-09-28T19:13:52Z

benchmarks/dynamo/ci_expected_accuracy/inductor_torchbench_training.csv

 tts_angular,pass,10
 vgg16,pass,8
-vision_maskrcnn,fail_accuracy,42
+vision_maskrcnn,fail_accuracy,39


any idea why this PR affected # of graph breaks in vision_maskrcnn? 🤔

So the actual cause of this was #110160.

Not sure why it was showing a failure for this PR though...

…ore ATen decomp table Summary: This is a reland of [github PR pytorch#110102]( pytorch#110102). The original PR had to be unlanded due to internal CI failures. This diff applies some small fixes to the failing tests to adjust to the new decompositions. Note that `lift_fresh` will not be decomposed for now, since it was found that [constant propogation looks specifically for `lift_fresh`](https://github.com/pytorch/pytorch/blob/main/torch/fx/experimental/proxy_tensor.py#L386). Therefore decomposing `lift_fresh` will interfere with constant propogation during export. Test Plan: Github CI and internal CI Differential Revision: D49761321

…ore ATen decomp table (pytorch#110323) Summary: X-link: pytorch/executorch#550 This is a reland of [github PR pytorch#110102]( pytorch#110102). The original PR had to be unlanded due to internal CI failures. This diff applies some small fixes to the failing tests to adjust to the new decompositions. Note that `lift_fresh` will not be decomposed for now, since it was found that [constant propogation looks specifically for `lift_fresh`](https://github.com/pytorch/pytorch/blob/main/torch/fx/experimental/proxy_tensor.py#L386). Therefore decomposing `lift_fresh` will interfere with constant propogation during export. Test Plan: Github CI and internal CI Differential Revision: D49761321

…ble (#550) Summary: Pull Request resolved: #550 X-link: pytorch/pytorch#110323 This is a reland of [github PR #110102]( pytorch/pytorch#110102). The original PR had to be unlanded due to internal CI failures. This diff applies some small fixes to the failing tests to adjust to the new decompositions. Note that `lift_fresh` will not be decomposed for now, since it was found that [constant propogation looks specifically for `lift_fresh`](https://github.com/pytorch/pytorch/blob/main/torch/fx/experimental/proxy_tensor.py#L386). Therefore decomposing `lift_fresh` will interfere with constant propogation during export. Reviewed By: kirklandsign Differential Revision: D49761321 fbshipit-source-id: 77e0f12f6c4144086b2c3dff1910bfc1cd674c7a

…ore ATen decomp table (#110323) Summary: This is a reland of [github PR #110102]( #110102). The original PR had to be unlanded due to internal CI failures. This diff applies some small fixes to the failing tests to adjust to the new decompositions. Note that `lift_fresh` will not be decomposed for now, since it was found that [constant propogation looks specifically for `lift_fresh`](https://github.com/pytorch/pytorch/blob/13af952f94e611f67ac306233898b753a9cf73d2/torch/fx/experimental/proxy_tensor.py#L381-L386). Therefore decomposing `lift_fresh` will interfere with constant propogation during export. Test Plan: Github CI and internal CI Differential Revision: D49761321 Pull Request resolved: #110323 Approved by: https://github.com/jansel

[core IR] Add lift_fresh and unbind decompositions to core ATen decom…

d324da3

…p table [ghstack-poisoned]

SS-JIA mentioned this pull request Sep 26, 2023

[core IR] Add glu as a core decomposition #110043

Closed

github-actions bot added module: inductor ciflow/inductor labels Sep 26, 2023

SS-JIA requested review from SherlockNoMad, jansel, lezcano and peterbell10 September 26, 2023 19:58

SS-JIA added a commit that referenced this pull request Sep 26, 2023

[core IR] Add lift_fresh and unbind decompositions to core ATen decom…

62986d5

…p table ghstack-source-id: e801c7f Pull Request resolved: #110102

SS-JIA added module: export release notes: export labels Sep 26, 2023

lezcano reviewed Sep 26, 2023

View reviewed changes

SS-JIA added a commit that referenced this pull request Sep 26, 2023

[core IR] Add lift_fresh, split.Tensor, and unbind decompositions to …

230cd21

…core ATen decomp table ghstack-source-id: 2123513 Pull Request resolved: #110102

SS-JIA changed the title ~~[core IR] Add lift_fresh and unbind decompositions to core ATen decomp table~~ [core IR] Add lift_fresh, split.Tensor, and unbind decompositions to core ATen decomp table Sep 26, 2023

jansel approved these changes Sep 27, 2023

View reviewed changes

SS-JIA added a commit that referenced this pull request Sep 27, 2023

[core IR] Add lift_fresh, split.Tensor, and unbind decompositions to …

da248e0

…core ATen decomp table ghstack-source-id: 1224515 Pull Request resolved: #110102

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 27, 2023

pytorchmergebot added the merging label Sep 27, 2023

github-actions bot added the module: dynamo label Sep 27, 2023

pytorchmergebot removed the merging label Sep 27, 2023

SS-JIA added a commit that referenced this pull request Sep 27, 2023

[core IR] Add lift_fresh, split.Tensor, and unbind decompositions to …

e5db9a8

…core ATen decomp table ghstack-source-id: 75a04b1 Pull Request resolved: #110102

pytorchmergebot added the merging label Sep 27, 2023

pytorchmergebot added Merged and removed merging labels Sep 28, 2023

pytorchmergebot closed this in 22e706f Sep 28, 2023

pytorchmergebot added the Reverted label Sep 28, 2023

bdhirsh reviewed Sep 28, 2023

View reviewed changes

This was referenced Sep 29, 2023

[core IR][reland] Add split.Tensor and unbind decompositions to core ATen decomp table #110323

Closed

Add split.Tensor and unbind decompositions to core ATen decomp table pytorch/executorch#550

Closed

facebook-github-bot deleted the gh/SS-JIA/175/head branch October 1, 2023 14:23

[core IR] Add lift_fresh, split.Tensor, and unbind decompositions to core ATen decomp table #110102

[core IR] Add lift_fresh, split.Tensor, and unbind decompositions to core ATen decomp table #110102

Uh oh!

Conversation

SS-JIA commented Sep 26, 2023 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Uh oh!

pytorch-bot bot commented Sep 26, 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/110102

⏳ No Failures, 1 Pending

Uh oh!

lezcano Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

SS-JIA Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

bdhirsh Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

bdhirsh Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

SS-JIA commented Sep 27, 2023

Uh oh!

pytorchmergebot commented Sep 27, 2023

Merge started

Uh oh!

pytorchmergebot commented Sep 27, 2023

Merge failed

Uh oh!

SS-JIA commented Sep 27, 2023

Uh oh!

pytorchmergebot commented Sep 27, 2023

Merge started

Uh oh!

atalman commented Sep 28, 2023

Uh oh!

pytorch-bot bot commented Sep 28, 2023

Uh oh!

atalman commented Sep 28, 2023

Uh oh!

pytorchmergebot commented Sep 28, 2023

Uh oh!

pytorchmergebot commented Sep 28, 2023

Uh oh!

bdhirsh Sep 28, 2023

Choose a reason for hiding this comment

Uh oh!

SS-JIA Sep 28, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

SS-JIA commented Sep 26, 2023 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Sep 26, 2023 •

edited

Loading