New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[rpc] add default arg for init_method #30208
Conversation
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. ghstack-source-id: 94325445 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. ghstack-source-id: 94327311 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. ghstack-source-id: 94329214 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
@@ -15,6 +15,7 @@ namespace rpc { | |||
struct RpcBackendOptions { | |||
RpcBackendOptions() noexcept = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an relevant compiling error.
Try removing noexcept
?
I guess it's a mismatch with std::string's default constructor.
init_method(str): backend specific init arguments. | ||
init_method(str): URL specifying how to initialize the | ||
RPC backend. Default is "env://" if no | ||
``init_method`` is specified. | ||
rank (int): a globally unique id/rank of this node. | ||
world_size (int): The number of workers in the group. | ||
rpc_backend_options (RpcBackendOptions): The options passed to RpcAgent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This can come in a followup PR) Do we have docs for rpc_backend_options to show how to construct it, probably with a customized init_method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll work on this in a follow up PR.
torch/distributed/rpc/__init__.py
Outdated
@@ -50,15 +49,24 @@ def init_rpc( | |||
``Worker1``) Name can only contain number, alphabet, | |||
underscore, and/or dash, and must be shorter than | |||
128 characters. | |||
init_method(str): backend specific init arguments. | |||
init_method(str): URL specifying how to initialize the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we still have this in the docs? Isn't this parameter removed from init_rpc
now?
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. ghstack-source-id: 94407223 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. ghstack-source-id: 94411767 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. ghstack-source-id: 94417175 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stack looks good to go. Test failures are irrelevant:
Nov 22 10:11:16 ======================================================================
Nov 22 10:11:16 FAIL [0.024s]: test_pytorch_graph (__main__.TestTensorBoardPytorchGraph)
Nov 22 10:11:16 ----------------------------------------------------------------------
Nov 22 10:11:16 Traceback (most recent call last):
Nov 22 10:11:16 File "test_tensorboard.py", line 507, in test_pytorch_graph
Nov 22 10:11:16 self.assertTrue(compare_proto(graphdef, self))
Nov 22 10:11:16 AssertionError: False is not true
Nov 22 10:11:16
Nov 22 10:11:16 ----------------------------------------------------------------------
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. ghstack-source-id: 94459011 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Also fixes some docs. ghstack-source-id: 94480407 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Removes `init_method` arg from rpc.init_rpc. Also fixes some docs. ghstack-source-id: 94490244 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/) [ghstack-poisoned]
Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Removes `init_method` arg from rpc.init_rpc. Also fixes some docs. ghstack-source-id: 94500475 Differential Revision: [D18630074](https://our.internmc.facebook.com/intern/diff/D18630074/)
Summary: Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Removes `init_method` arg from rpc.init_rpc. Also fixes some docs. ghstack-source-id: 94500475 Test Plan: Unit tests pass. Reviewed By: mrshenli Differential Revision: D18630074 fbshipit-source-id: 04b7dd7ec96f4c4da311b71d250233f1f262135a
Summary: Pull Request resolved: #30208 Adds default arg for init_method so users don't have to pass this in, and moves it to `RpcBackendOptions` struct. Removes `init_method` arg from rpc.init_rpc. Also fixes some docs. ghstack-source-id: 94500475 Test Plan: Unit tests pass. Reviewed By: mrshenli Differential Revision: D18630074 fbshipit-source-id: 04b7dd7ec96f4c4da311b71d250233f1f262135a
Stack from ghstack:
Adds default arg for init_method so users don't have to pass this in,
and moves it to
RpcBackendOptions
struct.Differential Revision: D18630074