[rpc] rename join_rpc to wait_all_workers in public api #30050
Closed
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
Renames this API to wait_all_workers as discussed. Differential Revision: [D18581466](https://our.internmc.facebook.com/intern/diff/D18581466/) [ghstack-poisoned]
rohan-varma
requested review from
mrshenli,
pritamdamania87 and
zhaojuanmao
as code owners
rohan-varma
added a commit
that referenced
this pull request
Nov 18, 2019
Renames this API to wait_all_workers as discussed. Differential Revision: [D18581466](https://our.internmc.facebook.com/intern/diff/D18581466/) ghstack-source-id: 94151171 Pull Request resolved: #30050
rohan-varma
commented
Nov 18, 2019
|
|
||
|
|
||
| def join_rpc(): | ||
| def wait_all_workers(): |
Contributor
Author
There was a problem hiding this comment.
calling it wait_all_workers as per discussion, though for 1.4, it will probably still join all threads, which is more of a shutdown operation (meaning that user can't keep sending rpcs after this). maybe join_all_workers is more appropriate?
pritamdamania87
suggested changes
Nov 19, 2019
torch/distributed/rpc/api.py
Outdated
| def join_rpc(): | ||
| def wait_all_workers(): | ||
| r""" | ||
| Block until all local and remote RPC processes reach this method, process |
Contributor
There was a problem hiding this comment.
Can we expand the docstring here to clearly indicate that this method should be used as part of termination and there is no guarantee that the RPC framework will work after this method returns. Would be also helpful to have an example here of how to just wait_all_workers and shutdown together.
Contributor
Author
There was a problem hiding this comment.
shutdown has not been landed yet - if that lands first we can add this.
rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** Renames this API to wait_all_workers as discussed. Differential Revision: [D18581466](https://our.internmc.facebook.com/intern/diff/D18581466/) [ghstack-poisoned]
Contributor
Author
|
@pritamdamania87 updated to include the changes to documentation, as well as adding a small example. Did not include |
rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** Renames this API to wait_all_workers as discussed. Differential Revision: [D18581466](https://our.internmc.facebook.com/intern/diff/D18581466/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 19, 2019
Pull Request resolved: #30050 Renames this API to wait_all_workers as discussed. ghstack-source-id: 94198512 Differential Revision: [D18581466](https://our.internmc.facebook.com/intern/diff/D18581466/)
rohan-varma
added a commit
that referenced
this pull request
Nov 19, 2019
…lect correct use of rpc.init_rpc" rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc Some of the examples provided in `rpc/api.py` were not updated along with the code changes, this PR updates them. Also removes the `dist.ProcessGroup` information since `init_rpc` now initializes a default process group. Differential Revision: [D18582596](https://our.internmc.facebook.com/intern/diff/D18582596/) [ghstack-poisoned]
Contributor
|
cc @zzzwen @mingzhe09088 This API change might break your code. |
pritamdamania87
approved these changes
Nov 19, 2019
rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** Renames this API to wait_all_workers as discussed. Differential Revision: [D18581466](https://our.internmc.facebook.com/intern/diff/D18581466/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 20, 2019
…lect correct use of rpc.init_rpc" rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc Some of the examples provided in `rpc/api.py` were not updated along with the code changes, this PR updates them. Also removes the `dist.ProcessGroup` information since `init_rpc` now initializes a default process group. Differential Revision: [D18582596](https://our.internmc.facebook.com/intern/diff/D18582596/) [ghstack-poisoned]
rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** Renames this API to wait_all_workers as discussed. Differential Revision: [D18581466](https://our.internmc.facebook.com/intern/diff/D18581466/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 20, 2019
…lect correct use of rpc.init_rpc" rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc Some of the examples provided in `rpc/api.py` were not updated along with the code changes, this PR updates them. Also removes the `dist.ProcessGroup` information since `init_rpc` now initializes a default process group. Differential Revision: [D18582596](https://our.internmc.facebook.com/intern/diff/D18582596/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 20, 2019
…lect correct use of rpc.init_rpc" rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc Some of the examples provided in `rpc/api.py` were not updated along with the code changes, this PR updates them. Also removes the `dist.ProcessGroup` information since `init_rpc` now initializes a default process group. Differential Revision: [D18582596](https://our.internmc.facebook.com/intern/diff/D18582596/) [ghstack-poisoned]
rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** rpc.init_rpc * **#30050 [rpc] rename join_rpc to wait_all_workers in public api** Renames this API to wait_all_workers as discussed. Differential Revision: [D18581466](https://our.internmc.facebook.com/intern/diff/D18581466/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 20, 2019
…lect correct use of rpc.init_rpc" rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc** * #30050 [rpc] rename join_rpc to wait_all_workers in public api rpc.init_rpc Some of the examples provided in `rpc/api.py` were not updated along with the code changes, this PR updates them. Also removes the `dist.ProcessGroup` information since `init_rpc` now initializes a default process group. Differential Revision: [D18582596](https://our.internmc.facebook.com/intern/diff/D18582596/) [ghstack-poisoned]
Contributor
Author
|
CI errors are unrelated, landing this. |
Contributor
|
This pull request has been merged in f304bd5. |
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.
Stack from ghstack:
rpc.init_rpc
rpc.init_rpc
rpc.init_rpc
rpc.init_rpc
rpc.init_rpc
rpc.init_rpc
Renames this API to wait_all_workers as discussed.
Differential Revision: D18581466