[c10d] Move sync_params to C++#9805
Conversation
|
@pytorchbot retest this please |
|
cc @teng-li would you like to review this PR too? |
|
@goldsborough We do have a test covering this change right? |
|
@teng-li I ran the c10d DDP test that I wrote. Should we also write some unit tests for these individual functions? |
e115f43 to
3523820
Compare
teng-li
left a comment
There was a problem hiding this comment.
Look great, thank you for adding those test cases.
Please see my comments.
test/test_c10d.py
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
test/test_c10d.py
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@pytorchbot retest this please |
c3851ae to
c031d29
Compare
|
@pytorchbot retest this please |
|
might possibly need a rebase. cc: @teng-li |
|
@pytorchbot retest this please |
1 similar comment
|
@pytorchbot retest this please |
21cd574 to
46bf4f1
Compare
Rebase fixes Rebase Wrote unit test for dist_broadcast_coalesced Wrote unit test for sync_params Make the bucke size 20 bytes and increase test timeout to 10s 5 tensors per device Increase timeout
56474d2 to
ec7d381
Compare
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: The next function I'm moving to C++ is `sync_params`. It is stacked on top of pytorch#9729, so some changes will go away when it lands and I rebase. I also split code into a `.h` and `.cpp` file for better code organization. The controller you requested could not be found. pietern apaszke Pull Request resolved: pytorch#9805 Differential Revision: D9688604 Pulled By: goldsborough fbshipit-source-id: 4467104d3f9e2354425503b9e4edbd59603e20a8
The next function I'm moving to C++ is
sync_params. It is stacked on top of #9729, so some changes will go away when it lands and I rebase.I also split code into a
.hand.cppfile for better code organization.@teng-li @pietern @apaszke