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
Fix: Bad autograd side effects from printing #51364
Fix: Bad autograd side effects from printing #51364
Conversation
💊 CI failures summary and remediationsAs of commit c85aee0 (more details on the Dr. CI page):
This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
Codecov Report
@@ Coverage Diff @@
## master #51364 +/- ##
==========================================
- Coverage 80.84% 80.84% -0.01%
==========================================
Files 1931 1931
Lines 210933 210932 -1
==========================================
- Hits 170532 170529 -3
- Misses 40401 40403 +2 |
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.
Looks good to me!
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.
@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@jbschlosser merged this pull request in 8f0968f. |
Fixes #49756
Background
Fix applied here is to remove the grad enabled check from
collect_next_edges
, unconditionally returning the actual collected edges. This pushes the responsibility for determining whether the function should be called without grad mode to its call-sites. With this update,collect_next_edges
will no longer incorrectly return an empty list, which caused the problem described in the issue. Three call-sites depended on this behavior and have been updated.Beyond bad printing side effects, this fix addresses the more general issue of accessing
grad_fn
with grad mode disabled after an in-place operation on a view. The included test verifies this without the use of print.Test Plan