Consider storage_changed for assigning alias_of_input in aot_autograd when computing differentiable outputs that alias each other #115315

voznesenskym · 2023-12-07T01:16:14Z

Stack from ghstack (oldest at bottom):

[Do not review] Top of FSDP stack - to be broken up #115410
[fsdp][torch.compile] FSDP changes #115497
[fsdp] Replace acc_grad hooking with register_post_accumulate_grad_hook on flat_param #112184
-> Consider storage_changed for assigning alias_of_input in aot_autograd when computing differentiable outputs that alias each other #115315

cc @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov @kadeng

… when computing differentiable outputs that alias each other [ghstack-poisoned]

pytorch-bot · 2023-12-07T01:16:18Z

🔗 Helpful Links

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

📄 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.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 3b51fe6 with merge base f591933 ():

UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:

trunk / linux-focal-rocm5.7-py3.8 / test (default, 1, 1, linux.rocm.gpu, unstable) (gh)

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

…ot_autograd when computing differentiable outputs that alias each other" [ghstack-poisoned]

… when computing differentiable outputs that alias each other ghstack-source-id: 7e9e529 Pull Request resolved: #115315 finger moved line

voznesenskym · 2023-12-07T08:53:53Z

torch/_functorch/_aot_autograd/collect_metadata_analysis.py

        intermediate_base_tensor_id_to_output_idx: Dict[int, int] = {}
        intermediate_bases: List[torch.Tensor] = []
+        # Why do we care if storage changed?
+        # There is a really care class of situations, which basically only happen with something


care -> rare

voznesenskym · 2023-12-07T08:54:33Z

torch/_functorch/_aot_autograd/collect_metadata_analysis.py

+        #
+        #     return out
+        #
+        # Esentially, what his code does is calls set_() with no_grad() - aka, our simulation


Mention the unsafe autograd preservation too, and that this is what fsdp does lol

voznesenskym · 2023-12-07T08:55:05Z

torch/_functorch/_aot_autograd/collect_metadata_analysis.py

+        #
+        #     return out
+        #
+        # Esentially, what his code does is calls set_() with no_grad() - aka, our simulation


his -> this

voznesenskym · 2023-12-07T09:28:07Z

o needs a type check

…ot_autograd when computing differentiable outputs that alias each other" [ghstack-poisoned]

… when computing differentiable outputs that alias each other ghstack-source-id: 6062d66 Pull Request resolved: #115315 finger moved line Fixes

…ot_autograd when computing differentiable outputs that alias each other" [ghstack-poisoned]

…ot_autograd when computing differentiable outputs that alias each other" cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng [ghstack-poisoned]

… when computing differentiable outputs that alias each other ghstack-source-id: 937d4d4 Pull Request resolved: #115315 finger moved line Fixes Fix reword

pytorchmergebot · 2023-12-12T20:04:53Z

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

voznesenskym · 2023-12-12T23:19:11Z

@pytorchbot merge

pytorchmergebot · 2023-12-12T23:21:18Z

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

…ok on flat_param (#112184) Pull Request resolved: #112184 Approved by: https://github.com/albanD ghstack dependencies: #115315

Support for something we need for both FSDP and optimizers. For sourced args that are not inputs (params, etc) - we use the dynamic_getattr flow on tensors. This soundly handles the storage and registration and guarding downstream of tensor_wrap for the grad values. For non sourced (true intermediates), we only support None (the idea being that if we have a true intermediate in the graph with grad, we are already doing something weird). Pull Request resolved: #115898 Approved by: https://github.com/bdhirsh ghstack dependencies: #115315, #112184

… when computing differentiable outputs that alias each other (pytorch#115315) Pull Request resolved: pytorch#115315 Approved by: https://github.com/bdhirsh

…ok on flat_param (pytorch#112184) Pull Request resolved: pytorch#112184 Approved by: https://github.com/albanD ghstack dependencies: pytorch#115315

Support for something we need for both FSDP and optimizers. For sourced args that are not inputs (params, etc) - we use the dynamic_getattr flow on tensors. This soundly handles the storage and registration and guarding downstream of tensor_wrap for the grad values. For non sourced (true intermediates), we only support None (the idea being that if we have a true intermediate in the graph with grad, we are already doing something weird). Pull Request resolved: pytorch#115898 Approved by: https://github.com/bdhirsh ghstack dependencies: pytorch#115315, pytorch#112184

… when computing differentiable outputs that alias each other (pytorch#115315) Pull Request resolved: pytorch#115315 Approved by: https://github.com/bdhirsh

…ok on flat_param (pytorch#112184) Pull Request resolved: pytorch#112184 Approved by: https://github.com/albanD ghstack dependencies: pytorch#115315

Support for something we need for both FSDP and optimizers. For sourced args that are not inputs (params, etc) - we use the dynamic_getattr flow on tensors. This soundly handles the storage and registration and guarding downstream of tensor_wrap for the grad values. For non sourced (true intermediates), we only support None (the idea being that if we have a true intermediate in the graph with grad, we are already doing something weird). Pull Request resolved: pytorch#115898 Approved by: https://github.com/bdhirsh ghstack dependencies: pytorch#115315, pytorch#112184

Consider storage_changed for assigning alias_of_input in aot_autograd…

9211648

… when computing differentiable outputs that alias each other [ghstack-poisoned]

github-actions bot requested review from SherlockNoMad, albanD, antoniojkim, bdhirsh, ezyang and miladm December 7, 2023 01:16

Update on "Consider storage_changed for assigning alias_of_input in a…

98d9ee7

…ot_autograd when computing differentiable outputs that alias each other" [ghstack-poisoned]

github-actions bot added the ciflow/inductor label Dec 7, 2023

voznesenskym added the ciflow/trunk Trigger trunk jobs on your pull request label Dec 7, 2023

voznesenskym commented Dec 7, 2023

View reviewed changes

voznesenskym marked this pull request as draft December 7, 2023 09:27

albanD removed their request for review December 7, 2023 19:28

Update on "Consider storage_changed for assigning alias_of_input in a…

4175e4a

…ot_autograd when computing differentiable outputs that alias each other" [ghstack-poisoned]

Update on "Consider storage_changed for assigning alias_of_input in a…

6293684

…ot_autograd when computing differentiable outputs that alias each other" [ghstack-poisoned]

github-actions bot added the module: dynamo label Dec 8, 2023

voznesenskym marked this pull request as ready for review December 8, 2023 03:31

github-actions bot requested a review from albanD December 8, 2023 03:31

voznesenskym mentioned this pull request Dec 8, 2023

[Do not review] Top of FSDP stack - to be broken up #115410

Closed

albanD removed their request for review December 8, 2023 15:36

pytorchmergebot added the merging label Dec 12, 2023

pytorchmergebot removed the merging label Dec 12, 2023

voznesenskym mentioned this pull request Dec 12, 2023

forward decl #115709

Closed

voznesenskym added the topic: not user facing topic category label Dec 12, 2023

pytorchmergebot added the merging label Dec 12, 2023

pytorchmergebot added the Merged label Dec 12, 2023

pytorchmergebot closed this in 76ced0d Dec 12, 2023

pytorchmergebot removed the merging label Dec 12, 2023

pytorchmergebot pushed a commit that referenced this pull request Dec 13, 2023

[fsdp] Replace acc_grad hooking with register_post_accumulate_grad_ho…

310f6ab

…ok on flat_param (#112184) Pull Request resolved: #112184 Approved by: https://github.com/albanD ghstack dependencies: #115315

voznesenskym mentioned this pull request Dec 15, 2023

Support non grapharg and intermediary grad access #115898

Closed

facebook-github-bot deleted the gh/voznesenskym/290/head branch December 16, 2023 15:26

voznesenskym restored the gh/voznesenskym/290/head branch December 18, 2023 18:35

facebook-github-bot deleted the gh/voznesenskym/290/head branch December 19, 2023 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Consider storage_changed for assigning alias_of_input in aot_autograd when computing differentiable outputs that alias each other #115315

Consider storage_changed for assigning alias_of_input in aot_autograd when computing differentiable outputs that alias each other #115315

Uh oh!

voznesenskym commented Dec 7, 2023 •

edited

Loading

Uh oh!

pytorch-bot bot commented Dec 7, 2023 •

edited

Loading

Uh oh!

voznesenskym Dec 7, 2023

Uh oh!

voznesenskym Dec 7, 2023

Uh oh!

voznesenskym Dec 7, 2023

Uh oh!

voznesenskym commented Dec 7, 2023

Uh oh!

pytorchmergebot commented Dec 12, 2023

Uh oh!

voznesenskym commented Dec 12, 2023

Uh oh!

pytorchmergebot commented Dec 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Consider storage_changed for assigning alias_of_input in aot_autograd when computing differentiable outputs that alias each other #115315

Consider storage_changed for assigning alias_of_input in aot_autograd when computing differentiable outputs that alias each other #115315

Uh oh!

Conversation

voznesenskym commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Dec 7, 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/115315

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

voznesenskym Dec 7, 2023

Choose a reason for hiding this comment

Uh oh!

voznesenskym Dec 7, 2023

Choose a reason for hiding this comment

Uh oh!

voznesenskym Dec 7, 2023

Choose a reason for hiding this comment

Uh oh!

voznesenskym commented Dec 7, 2023

Uh oh!

pytorchmergebot commented Dec 12, 2023

Merge failed

Uh oh!

voznesenskym commented Dec 12, 2023

Uh oh!

pytorchmergebot commented Dec 12, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

voznesenskym commented Dec 7, 2023 •

edited

Loading

pytorch-bot bot commented Dec 7, 2023 •

edited

Loading