Support AMP in nn.parallel #43102
Closed
Support AMP in nn.parallel #43102
Conversation
This file contains hidden or 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
Merge Upstream
💊 CI failures summary and remediationsAs of commit 2072a64 (more details on the Dr. CI page):
🕵️ 4 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
mrshenli
added
module: data parallel
triaged
Author
|
@mrshenli Hi, with pleasure. It would be better if there was a corresponding test, but I do not know how to make it, I cannot also find the test code for |
Contributor
|
Hey @anoidgit, you can add this to pytorch/test/distributed/test_data_parallel.py Lines 80 to 96 in ccc831a |
Author
|
Hi @mrshenli , thanks a lot for your help. I made the test following the example, please help ensure its correctness. |
mrshenli
approved these changes
Aug 24, 2020
Contributor
mrshenli
left a comment
mrshenli
left a comment
There was a problem hiding this comment.
LGTM! Just a minor comment. Thanks for contributing!
| expected_outputs = (expected1, expected2) | ||
|
|
||
| # each input can be either a collection of positional arguments | ||
| # or an object representing the single argument |
Contributor
There was a problem hiding this comment.
any reason for the long indent before "or"?
Author
There was a problem hiding this comment.
thanks for pointing this out. It also looks strange to me, but I do not know the reason, I am following the example. If it is not right, please help fix.
Contributor
There was a problem hiding this comment.
that might be some debug vestige. No worries, I will land this as is, and submit a PR to fix both.
Contributor
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@mrshenli has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Collaborator
|
Code changes look good, thanks! DP + autocast documentation should also be updated, but I can do that myself in a separate PR. Updating documentation is not urgent because guidance in the existing documentation will continue to work after this PR, it will just become overkill. |
Author
|
@mcarilli many thanks for your efforts :) |
Contributor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Take care of the state of autocast in
parallel_apply, so there is no need to decorate model implementations.