Delay reduction of unused parameters until first autograd hook is called #22219
Conversation
pietern
added
oncall: distributed
Reduction of gradients for unused parameters should happen as soon as possible, because they potentially block reduction of gradients for used parameters. This used to happen instantly when `prepare_for_backward` was called and it found parameters that didn't contribute. This meant that if you have a model with unused parameters, and you want to discard the model output (i.e. not call backward on some loss), reduction of the gradients of those unused parameters would have been kicked off, and you'd see an error the next time you called `forward`. In this commit, this original approach is slightly changed to delay reduction of the gradients of those unused parameters until the first autograd hook is called. This means that you can now discard the model output regardless of the model having unused parameters or not. This is a prerequisite for making the `find_unused_parameters` argument to DDP default to `True`.
pietern
force-pushed
the
reducer-reduce-on-backward
branch
from
392b3c8 to
216c384
Compare
mrshenli
left a comment
mrshenli
left a comment
There was a problem hiding this comment.
It breaks test_no_used_parameters as no post hook will be called at all. Seems we need to add another special case for it when all params are unused, or is it possible to use the queue_callback to register it upfront?
|
Regarding |
|
@pytorchbot retest this please |
|
@pytorchbot retest this please |
|
After checking in with CircleCI it is clear that the error for |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@pietern is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@pietern has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
…led (pytorch#22219) Summary: Reduction of gradients for unused parameters should happen as soon as possible, because they potentially block reduction of gradients for used parameters. This used to happen instantly when `prepare_for_backward` was called and it found parameters that didn't contribute. This meant that if you have a model with unused parameters, and you want to discard the model output (i.e. not call backward on some loss), reduction of the gradients of those unused parameters would have been kicked off, and you'd see an error the next time you called `forward`. In this commit, this original approach is slightly changed to delay reduction of the gradients of those unused parameters until the first autograd hook is called. This means that you can now discard the model output regardless of the model having unused parameters or not. This is a prerequisite for making the `find_unused_parameters` argument to DDP default to `True`. Pull Request resolved: pytorch#22219 Differential Revision: D16028698 Pulled By: pietern fbshipit-source-id: c6aec2cd39c4a77746495d9cb1c9fb9c5ac61983
4 participants
Reduction of gradients for unused parameters should happen as soon as
possible, because they potentially block reduction of gradients for
used parameters. This used to happen instantly when
prepare_for_backwardwas called and it found parameters that didn'tcontribute. This meant that if you have a model with unused
parameters, and you want to discard the model output (i.e. not call
backward on some loss), reduction of the gradients of those unused
parameters would have been kicked off, and you'd see an error the next
time you called
forward.In this commit, this original approach is slightly changed to delay
reduction of the gradients of those unused parameters until the first
autograd hook is called. This means that you can now discard the model
output regardless of the model having unused parameters or not.
This is a prerequisite for making the
find_unused_parametersargument to DDP default to
True.