Skip to content

Conversation

@bdhirsh
Copy link
Contributor

@bdhirsh bdhirsh commented Aug 8, 2024

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like split_with_sizes_copy.out)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.

Stack from ghstack (oldest at bottom):

cc @XilunWu @H-Huang @awgu @kwen2501 @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @c-p-i-o @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames @rec

@pytorch-bot
Copy link

pytorch-bot bot commented Aug 8, 2024

🔗 Helpful Links

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

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

❌ 44 New Failures

As of commit cfe4273 with merge base b040dc3 (image):

NEW FAILURES - The following jobs have failed:

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

bdhirsh added a commit that referenced this pull request Aug 8, 2024
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Nice fix

… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





[ghstack-poisoned]
bdhirsh added a commit that referenced this pull request Aug 9, 2024
at::TensorList tensors = {first, first};
at::TensorList undefined_tensors = {first, second};
at::TensorList steps = {step, step};
return at::_fused_adamw_(tensors, tensors, tensors, tensors, undefined_tensors,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

so calling _fused_adamw_ here with undefined inputs feels like it was just wrong (we now error because you are passing in a mutable, undefined tensor, which autograd is not allowed to bump the VC on).

To make the test a bit more realistic I updated it to use aten::index(). which accepts undefined tensor indices (which are not mutated)

@bdhirsh bdhirsh added the release notes: autograd release notes category label Aug 9, 2024
@bdhirsh
Copy link
Contributor Author

bdhirsh commented Aug 9, 2024

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 9, 2024
@pytorchmergebot
Copy link
Collaborator

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
Copy link
Collaborator

Merge failed

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

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Nice!

… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





[ghstack-poisoned]
@medivh-xp
Copy link
Contributor

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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

… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
bdhirsh added a commit that referenced this pull request Mar 7, 2025
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
bdhirsh added a commit that referenced this pull request Mar 8, 2025
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
… returns"

Fixes #132014

In particular, this PR should:

(1) start generating and registering kernels to the ADInplaceOrView dispatch key, for mutable ops that have no returns (like `split_with_sizes_copy.out`)

(2) prevously, the codegen would loop over mutable return names to decide which tensors to codegen VC bumps for. Now, we just loop over mutable input argument names.





cc XilunWu H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang amjames rec

[ghstack-poisoned]
bdhirsh added a commit that referenced this pull request Mar 12, 2025
@github-actions
Copy link
Contributor

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label May 13, 2025
@github-actions github-actions bot closed this Jun 12, 2025
@github-actions github-actions bot deleted the gh/bdhirsh/604/head branch July 13, 2025 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request module: dynamo oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: autograd release notes category Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants