Add helpful information to the gradient/inplace operation exception #18523
Conversation
|
looks like a great change, thank you! I'm tagging @apaszke for review |
There was a problem hiding this comment.
nit: I don't think you want the "not" here.
There was a problem hiding this comment.
It was on purpose, but I can see how it's ambiguous, thanks. I meant to say "it's at version x, not at the expected version, which is y". I'll change it to "is at version x, expected y instead".
There was a problem hiding this comment.
Is it going to point to the forward pass op that modified it? Or the one that needs it?
There was a problem hiding this comment.
Good spot. From what I read in #15803 (comment), I assumed that it would find the inplace operation, but after debugging my problem, I found that this was a red herring. I will change the hint accordingly!
There was a problem hiding this comment.
It would be quite interesting to actually be able to point out both for debugging purposes !
But it would be a bit tricky to do :/
There was a problem hiding this comment.
Well, where do the version counters live? Would it be possible to have a mode that warns if any gradient-required tensor is involved when increasing a version counter? Or a mode that simply warns on any in-place operation?
There was a problem hiding this comment.
There are usually many (many) inplace ops. So that mode might be too verbose to be useful.
But maybe it's something we want to add to the anomaly detection mode: track which python line made the version counter bump so that we can use that information here for better error printing.
But that would be a much more involved change.
f0k
force-pushed
the
informative-inplace-error
branch
from
b31ab22 to
2dad0b1
Compare
|
Following the review, I made the following updates:
Let me know if you think I overdid it with the hints or the best wishes, but I guess many newcomers will hit this error sooner or later, and this would minimize the time spent on search engines. |
There was a problem hiding this comment.
@soumith is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
5 participants
To debug a
one of the variables needed for gradient computation has been modified by an inplace operationerror, I wanted to know which variable has been modified, so I extended the error message with what information is easily available at this point.Before:
After:
The hint to enable anomaly detection is only shown when it is not enabled. It's meant to save people some googling. I'd even go further and reference
torch.autograd.set_detect_anomaly(True), but maybe we're not running Python?Disclaimer: I haven't looked at other parts of the code to check if using
std::stringstreamis acceptable practice, let me know if it isn't. Similarly, I haven't checked about indentation practices.