[RPC] Create local RRef<ModuleInterface> remotely in Python, use it remotely in TorchScript #34183

xush6528 · 2020-03-04T02:07:41Z

Stack from ghstack:

[RPC] Create local RRef<ModuleInterface> remotely in Python, use it remotely in TorchScript #34183 [RPC] Create local RRef remotely in Python, use it remotely in TorchScript
[RPC] [Minor] RPC entry point cleanup #34292 [WIP] [RPC] Increase ref count before releasing GIL

#33263 enhanced the RRef Python constructor to infer most types, by jit::tryToInferType(..).

But this helper function can't infer ScriptModule type due to ScriptModule's special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained ScriptModule.

Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified ModuleInterface type.

We added an optional argument type_hint for users to set an RRef for what ModuleInterface type it's holds.

Differential Revision: D7065050

` Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

` Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) ghstack-source-id: 99451392 Pull Request resolved: #34183

dr-ci · 2020-03-04T03:13:31Z

💊 CircleCI build failures summary and remediations

As of commit 1da85ab (more details on the Dr. CI page):

Commit 1da85ab was recently pushed. Waiting for builds...

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker.

This comment has been revised 22 times.

torch/csrc/distributed/rpc/py_rref.cpp

torch/csrc/distributed/rpc/init.cpp

torch/csrc/distributed/rpc/py_rref.cpp

torch/testing/_internal/distributed/rpc/jit/rpc_test.py

…rchScript" ` Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

zzzwen

Q: Can you add a test again "torchscript class interface"? not sure if this is the correct name, but the code is something like this

@torch.jit.interface
class Interface(object):

torch/csrc/distributed/rpc/init.cpp

…rchScript" #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to track the type of the `ScriptModule` it holds.. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more helpful to mark it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to mark an `RRef` for what `ModuleInterface` type it's holds. Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

Pull Request resolved: #34183 #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to track the type of the `ScriptModule` it holds.. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more helpful to mark it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to mark an `RRef` for what `ModuleInterface` type it's holds. ghstack-source-id: 99536698 Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/)

…it remotely in TorchScript" #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to set an `RRef` for what `ModuleInterface` type it's holds. Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

… in TorchScript Pull Request resolved: #34183 #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to mark an `RRef` for what `ModuleInterface` type it's holds. ghstack-source-id: 99563605 Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/)

mrshenli

LGTM! Thanks for adding this!

torch/testing/_internal/distributed/rpc/jit/rpc_test.py

torch/csrc/distributed/rpc/py_rref.cpp

wanchaol

There're several type check/ user facing error message we should fix before we merge this.

torch/csrc/distributed/rpc/py_rref.cpp

…it remotely in TorchScript" #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to set an `RRef` for what `ModuleInterface` type it's holds. Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

… in TorchScript Pull Request resolved: #34183 #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to mark an `RRef` for what `ModuleInterface` type it's holds. ghstack-source-id: 99619039 Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/)

wanchaol

looking good, some nits and some questions.

torch/csrc/distributed/rpc/py_rref.cpp

torch/testing/_internal/distributed/rpc/jit/rpc_test.py

…it remotely in TorchScript" #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to set an `RRef` for what `ModuleInterface` type it's holds. Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

… in TorchScript Pull Request resolved: #34183 #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to mark an `RRef` for what `ModuleInterface` type it's holds. ghstack-source-id: 99630694 Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/)

wanchaol

looks good! one minor suggestion on merging two checks.

torch/csrc/distributed/rpc/py_rref.cpp

…it remotely in TorchScript" #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to set an `RRef` for what `ModuleInterface` type it's holds. Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

… in TorchScript Pull Request resolved: #34183 #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to mark an `RRef` for what `ModuleInterface` type it's holds. ghstack-source-id: 99648617 Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/)

…n, use it remotely in TorchScript" #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to set an `RRef` for what `ModuleInterface` type it's holds. Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

…emotely in TorchScript Pull Request resolved: #34183 #33263 enhanced the RRef Python constructor to infer most types, by `jit::tryToInferType(..)`. But this helper function can't infer `ScriptModule` type due to `ScriptModule`'s special per-Module type singleton logic, so it's still not possible for an Python-created RRef to know the JIT type of it's contained `ScriptModule`. Instead of inferring the specific type of a Module, which could leads to too many candidate types (due to Module's multiple inheritance possibility), it's more straightforward to set it's type as a user-specified `ModuleInterface` type. We added an optional argument `type_hint` for users to mark an `RRef` for what `ModuleInterface` type it's holds. ghstack-source-id: 99649379 Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/)

facebook-github-bot · 2020-03-06T17:16:20Z

This pull request has been merged in 17ceb69.

[RPC] Create RRef<ModuleInterface> in Python, use it in TorchScript

a7f5021

` Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

xush6528 requested review from mrshenli, pritamdamania87 and zhaojuanmao as code owners March 4, 2020 02:07

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Mar 4, 2020

mrshenli reviewed Mar 4, 2020

View reviewed changes

Update on "[RPC] Create RRef<ModuleInterface> in Python, use it in To…

7dc7754

…rchScript" ` Differential Revision: [D7065050](https://our.internmc.facebook.com/intern/diff/D7065050/) [ghstack-poisoned]

xush6528 requested review from wanchaol and zzzwen March 4, 2020 23:19

zzzwen reviewed Mar 4, 2020

View reviewed changes

torch/csrc/distributed/rpc/init.cpp Show resolved Hide resolved

xush6528 changed the title ~~[RPC] Create RRef<ModuleInterface> in Python, use it in TorchScript~~ [RPC] Create RRef<ModuleInterface> locally in Python, use it remotely in TorchScript Mar 5, 2020

xush6528 mentioned this pull request Mar 5, 2020

[RPC] [Minor] RPC entry point cleanup #34292

Closed

mrshenli approved these changes Mar 5, 2020

View reviewed changes

torch/testing/_internal/distributed/rpc/jit/rpc_test.py Outdated Show resolved Hide resolved

wanchaol reviewed Mar 5, 2020

View reviewed changes

torch/csrc/distributed/rpc/py_rref.cpp Outdated Show resolved Hide resolved

torch/csrc/distributed/rpc/py_rref.cpp Outdated Show resolved Hide resolved

torch/csrc/distributed/rpc/py_rref.cpp Outdated Show resolved Hide resolved

wanchaol reviewed Mar 5, 2020

View reviewed changes

torch/csrc/distributed/rpc/py_rref.cpp Outdated Show resolved Hide resolved

wanchaol reviewed Mar 5, 2020

View reviewed changes

torch/csrc/distributed/rpc/py_rref.cpp Outdated Show resolved Hide resolved

wanchaol requested changes Mar 5, 2020

View reviewed changes

wanchaol reviewed Mar 5, 2020

View reviewed changes

torch/csrc/distributed/rpc/py_rref.cpp Outdated Show resolved Hide resolved

torch/csrc/distributed/rpc/py_rref.cpp Outdated Show resolved Hide resolved

wanchaol reviewed Mar 6, 2020

View reviewed changes

xush6528 requested a review from wanchaol March 6, 2020 01:11

wanchaol approved these changes Mar 6, 2020

View reviewed changes

torch/csrc/distributed/rpc/py_rref.cpp Show resolved Hide resolved

xush6528 changed the title ~~[RPC] Create RRef<ModuleInterface> locally in Python, use it remotely in TorchScript~~ Create local RRef<ModuleInterface> remotely in Python, use it remotely in TorchScript Mar 6, 2020

xush6528 changed the title ~~Create local RRef<ModuleInterface> remotely in Python, use it remotely in TorchScript~~ [RPC] Create local RRef<ModuleInterface> remotely in Python, use it remotely in TorchScript Mar 6, 2020

facebook-github-bot closed this in 17ceb69 Mar 6, 2020

facebook-github-bot added the merged label Mar 6, 2020

facebook-github-bot deleted the gh/xush6528/75/head branch March 10, 2020 14:17

mruberry added the Merged label Oct 28, 2020

[RPC] Create local RRef<ModuleInterface> remotely in Python, use it remotely in TorchScript #34183

[RPC] Create local RRef<ModuleInterface> remotely in Python, use it remotely in TorchScript #34183

Uh oh!

Conversation

xush6528 commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CircleCI build failures summary and remediations

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zzzwen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mrshenli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wanchaol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wanchaol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wanchaol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot commented Mar 6, 2020

Uh oh!

Uh oh!

xush6528 commented Mar 4, 2020 •

edited

Loading

dr-ci bot commented Mar 4, 2020 •

edited

Loading