Add reinplacing pass for scatters + incremental fake tensor updating #106192

Chillee · 2023-07-28T04:40:03Z

Stack from ghstack (oldest at bottom):

Added info for each artifact option, added a help option to TORCH_LOGS, and changed the error message #107758
-> Add reinplacing pass for scatters + incremental fake tensor updating #106192

mutation for params)

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

mutation for params) [ghstack-poisoned]

pytorch-bot · 2023-07-28T04:40:06Z

🔗 Helpful Links

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

📄 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 cda004b with merge base d24c457 ():
💚 Looks good so far! There are no failures yet. 💚

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

linux-foundation-easycla · 2023-07-28T04:40:07Z

The committers listed above are authorized under a signed CLA.

✅ login: Chillee / name: Horace He (ed04a5d, c61d0c6, 42d3e39, 2a82e93, 79a06d9, e3c012c, 70edd0a, 0b3727e, 8398d14, 52ed740, 67f75a5, 848b011, 68e4920, 4c16582, 8bfb5b3, 71d3b15, 7dad5b9, b118ec6, 75309ca, e81edec, 4643d4a, d4477b6, 8644680, b426d0f, aecd433, 2298b6d, 2e32a21, f506136, 39374d7, 973884b, 36ad420, cda004b)

… cudagraph" mutation for params) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov anijain2305 [ghstack-poisoned]

mutation for params) ghstack-source-id: cbddfaa Pull Request resolved: #106192

torch/_inductor/decomposition.py

torch/_inductor/fx_passes/post_grad.py

torch/_dynamo/variables/builtin.py

… cudagraph" mutation for params) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov anijain2305 [ghstack-poisoned]

ghstack-source-id: 6be7ae9 Pull Request resolved: #106192

jansel

Test seem broken.

torch/_dynamo/utils.py

torch/_inductor/decomposition.py

torch/_inductor/graph.py

ezyang · 2023-08-13T15:46:47Z

torch/_inductor/fx_passes/post_grad.py

+    Reinplaces scatter operations in easy cases where the node being mutated
+    is only used by the scatter (users == 1)
+
+    Also handles input mutations when there is a corresponding copy node.


It's not obvious to me why we expect this to give sound results. In particular you don't have alias tracking so you don't know if it may not be safe to reinplace because there is another tensor that aliases you that would be improperly updated by this change

I think you could add alias tracking by looking at meta["val"].untyped_storage() easily enough. alternatively you could grab fw_metadata / bw_metadata, from the Tracing Context, and follow the input to index_put while its input is a view, until you either get to an input of the graph, or you find that it is produced by a node which is not a view.

I think if I just make sure the value being in-placed into doesn't share storage with another tensor, that'll be sufficient right?

yea, using the storage from fake vals is sufficient

This is only OK if Inductor doesn't introduce new aliasing, e.g., because it converted a clone() into a no-op on the fly. So no, it's not clear to me this is sufficient either.

True. You could retrace. But doing the ad-hoc analysis on the index_put example shouldnt b too tricky

This is only OK if Inductor doesn't introduce new aliasing, e.g., because it converted a clone() into a no-op on the fly.

In principle, Inductor should "support" in-place operators. So this has always been possible, but if it actually happens then it would be a bug.

Any decompositions would be applied during trace-time and so their fake tensors should be correct.

We may need to update pattern existing matcher passes to use this new propagation for this to be sound.

I'd like to do this in a follow-up PR. This PR has already expanded quite a bit, and I don't think the current state is any worse than the existing one.

torch/_inductor/fx_passes/post_grad.py

eellison · 2023-08-14T17:22:12Z

torch/_inductor/fx_passes/post_grad.py

+    Reinplaces scatter operations in easy cases where the node being mutated
+    is only used by the scatter (users == 1)
+
+    Also handles input mutations when there is a corresponding copy node.


I think you could add alias tracking by looking at meta["val"].untyped_storage() easily enough. alternatively you could grab fw_metadata / bw_metadata, from the Tracing Context, and follow the input to index_put while its input is a view, until you either get to an input of the graph, or you find that it is produced by a node which is not a view.

torch/_inductor/graph.py

… cudagraph" mutation for params) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov anijain2305 [ghstack-poisoned]

…r updating" mutation for params) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov anijain2305 [ghstack-poisoned]

Chillee · 2023-08-29T20:19:47Z

@pytorchbot merge

pytorchmergebot · 2023-08-29T20:51:27Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

EasyCLA

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

…r updating" mutation for params) cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov anijain2305 [ghstack-poisoned]

Chillee · 2023-08-30T20:39:20Z

@pytorchbot merge

pytorchmergebot · 2023-08-30T20:41:30Z

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

…S, and changed the error message (#107758) New message when invalid option is provided <img width="1551" alt="image" src="https://github.com/pytorch/pytorch/assets/6355099/8b61534a-ee55-431e-94fe-2ffa25b7fd5c"> TORCH_LOGS="help" <img width="1558" alt="image" src="https://github.com/pytorch/pytorch/assets/6355099/72e8939c-92fa-4141-8114-79db71451d42"> TORCH_LOGS="+help" <img width="1551" alt="image" src="https://github.com/pytorch/pytorch/assets/6355099/2cdc94ac-505a-478c-aa58-0175526075d2"> Pull Request resolved: #107758 Approved by: https://github.com/ezyang, https://github.com/mlazos ghstack dependencies: #106192

…S, and changed the error message (#107758) (#108365) New message when invalid option is provided <img width="1551" alt="image" src="https://github.com/pytorch/pytorch/assets/6355099/8b61534a-ee55-431e-94fe-2ffa25b7fd5c"> TORCH_LOGS="help" <img width="1558" alt="image" src="https://github.com/pytorch/pytorch/assets/6355099/72e8939c-92fa-4141-8114-79db71451d42"> TORCH_LOGS="+help" <img width="1551" alt="image" src="https://github.com/pytorch/pytorch/assets/6355099/2cdc94ac-505a-478c-aa58-0175526075d2"> Pull Request resolved: #107758 Approved by: https://github.com/ezyang, https://github.com/mlazos ghstack dependencies: #106192

Changes needed for transformer inference (includes Elias's cudagraph

ed04a5d

mutation for params) [ghstack-poisoned]

Chillee requested a review from a team as a code owner July 28, 2023 04:40

pytorch-bot bot added the release notes: releng release notes category label Jul 28, 2023

github-actions bot added module: inductor module: dynamo ciflow/inductor labels Jul 28, 2023

github-actions bot requested a review from ezyang July 28, 2023 04:40

Chillee added a commit that referenced this pull request Jul 28, 2023

Changes needed for transformer inference (includes Elias's cudagraph

178bc29

mutation for params) ghstack-source-id: cbddfaa Pull Request resolved: #106192

ezyang removed their request for review July 29, 2023 02:56

bdhirsh reviewed Jul 29, 2023

View reviewed changes

torch/_inductor/decomposition.py Outdated Show resolved Hide resolved

bdhirsh reviewed Jul 31, 2023

View reviewed changes

torch/_inductor/fx_passes/post_grad.py Show resolved Hide resolved

voznesenskym reviewed Aug 3, 2023

View reviewed changes

torch/_dynamo/variables/builtin.py Outdated Show resolved Hide resolved

Chillee added a commit that referenced this pull request Aug 11, 2023

Changes needed for transformer inference

f8140c5

ghstack-source-id: 6be7ae9 Pull Request resolved: #106192

Chillee requested review from eellison, jansel and sgrigory August 11, 2023 22:23

jansel requested changes Aug 13, 2023

View reviewed changes

torch/_dynamo/utils.py Outdated Show resolved Hide resolved

torch/_inductor/decomposition.py Outdated Show resolved Hide resolved

ezyang reviewed Aug 13, 2023

View reviewed changes

torch/_inductor/graph.py Show resolved Hide resolved

ezyang reviewed Aug 13, 2023

View reviewed changes

eellison reviewed Aug 14, 2023

View reviewed changes

Chillee added 2 commits August 23, 2023 18:12

jansel approved these changes Aug 25, 2023

View reviewed changes

Chillee added 8 commits August 25, 2023 13:12

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

pytorchmergebot added the merging label Aug 29, 2023

pytorchmergebot removed the merging label Aug 29, 2023

pytorchmergebot added the merging label Aug 30, 2023

pytorchmergebot added Merged and removed merging labels Aug 30, 2023

pytorchmergebot closed this in 39130c7 Aug 30, 2023

Chillee mentioned this pull request Aug 31, 2023

Added info for each artifact option, added a help option to TORCH_LOG… #108365

Merged

facebook-github-bot deleted the gh/chillee/218/head branch September 3, 2023 14:23

Valentine233 mentioned this pull request Sep 18, 2023

[inductor][cpu][dynamic shapes] performance regression #108754

Closed

Add reinplacing pass for scatters + incremental fake tensor updating #106192

Add reinplacing pass for scatters + incremental fake tensor updating #106192

Uh oh!

Conversation

Chillee commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Jul 28, 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/106192

✅ No Failures

Uh oh!

linux-foundation-easycla bot commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jansel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Chillee commented Aug 29, 2023

Uh oh!

pytorchmergebot commented Aug 29, 2023

Merge failed

Uh oh!

Chillee commented Aug 30, 2023

Uh oh!

pytorchmergebot commented Aug 30, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Chillee commented Jul 28, 2023 •

edited

Loading

pytorch-bot bot commented Jul 28, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jul 28, 2023 •

edited

Loading