-
Notifications
You must be signed in to change notification settings - Fork 24.9k
autograd: Add VJP and JVP rules for aten::aminmax #158241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
autograd: Add VJP and JVP rules for aten::aminmax #158241
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/158241
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit b7818f6 with merge base c8c221c ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
Adds functionally correct backward (VJP) and forward (JVP) autograd rules for the aten::aminmax operator to derivatives.yaml using existing helper functions. This ensures correct eager mode differentiation. Fixes pytorch#148808
… to handle amin and amax correctly. - Modified derivatives.yaml to reflect the changes in autograd behavior. - Updated test cases in test_autograd.py to validate the updated rules. - Updated common_methods_invocations.py to include amin and amax in relevant test cases.
Makes backward pass more efficient by: - Avoiding zeros tensor creation when only one gradient defined - Using in-place operations where possible
• Added a testcase to validate the new logic.
…abhaskar-ev/pytorch into fix-aminmax-autograd-rules
@soulitzer Adding restore_reduced_dims on grad_max and grad_min is causing Segmentation fault (core dumped) |
4c871fc
to
889bbf3
Compare
Removed restore_reduced_dims on grad_min/grad_max in aminmax_backward. |
Broadcasting doesn't work when certain dimensions are missing. restore_reduced_dims is responsible for restoring those dimensions so broadcasting can happen. |
If I add restore_reduced_dims on grad_max and grad_min, it's causing Segmentation fault (core dumped).
|
Let me rebuild and try again |
@soulitzer PR updated. |
auto grad_min_full = | ||
restore_reduced_dims(grad_min, dims, keepdim) | ||
.expand_as(min_mask) | ||
.contiguous(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't look right, we don't want a copy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed contiguous() as it cause's extra memory footprint
@soulitzer Can you review this PR? Resolved the issue's mentioned in the comments. |
Adds functionally correct backward (VJP) and forward (JVP) autograd rules for the aten::aminmax operator to derivatives.yaml using existing helper functions. This ensures correct eager mode differentiation.
Fixes #148808