Implement @data_distributed Decorator #1098
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
breakds
requested review from
emailweixu and
hnyu
and removed request for
emailweixu
emailweixu
reviewed
Dec 1, 2021
|
PR updated based on the comments. |
Initial implemented of data_distributed
breakds
force-pushed
the
PR/breakds/data_distributed_decorator
branch
from
06b1c97
to
fc1c5c6
Compare
breakds
force-pushed
the
PR/breakds/data_distributed_decorator
branch
from
fc1c5c6
to
3f71b8d
Compare
hnyu
requested changes
Dec 1, 2021
|
|
||
|
|
||
| def data_distributed(method): | ||
| """This decorator makes a target method of a module capable of being data |
There was a problem hiding this comment.
It seems that this decorator can only apply to methods of Algorithm? If so, the docstring should clarify this.
There was a problem hiding this comment.
It is just for Algorithm, but actually all nn.Module derived classes. One of the prerequisite is that it needs to have self._ddp_activated_rank, which is stated in the docstring as a contract.
hnyu
approved these changes
Dec 2, 2021
pd-perry
pushed a commit
to pd-perry/alf
that referenced
this pull request
Dec 11, 2021
* Implement @data_distributed Initial implemented of data_distributed * Update the documentation according to the comments. * Address comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Part of the effort to #1096 .
Originally we only "DDP"-fied the
unrollofRLAlgorithm, but later we found that such DDP wrapping can be made more general. To be more specific, we want to have a generally applicable, simple and transparent utility for this. It is almost always the case that we want to wrap a method of a torch Module. Therefore, a decorator for module methods is desirable.Solution
Implemented the
@data_distributeddecorator that can be applied on Module (such asRLAlogirhtm) methods. It will convert the method to be DDP-capable. A DDP-capable method will run with DDP wrapper automatically if the module it belongs to has DDP activated. A module is DDP-acitvated if its_ddp_activated_rankis set to a non-negative value. ForAlgorithmand its derived classes, this can be done by callingactivate_ddp(rank).Also removed
_UnrollPerformer, and use@data_distributedto wrapunrollinstead. The code should be much cleaner.NOTE: this still only enables DDP for the on-policy training. Will follow up on off-policy training too, which is also based on this PR.
Testing
Run
ac_breakoutwith both DDP turned on and off.Note that with and without DDP, the training curve almost coincides.