Support TorchScript call over remote API (RRef) by xush6528 · Pull Request #32466 · pytorch/pytorch

xush6528 · 2020-01-21T23:26:12Z

Stack from ghstack:

Support TorchScript call over remote API (RRef) #32466 Support TorchScript call over remote API (RRef)

It's a follow-up work of #32197.

In #32197, rpc.sync_rpc(..) and rpc.rpc_async(..) support taking a TorchScript annotated Python function as the user function for RPC.

This PR extend along this direction by making rpc.remote(..) support taking a TorchScript annotated Python function as well.

Differential Revision: D19440633

It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` supports taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by make `rpc.remote(..)` support taking a TorchScript annotated Python function as well. Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/) [ghstack-poisoned]

kostmo · 2020-01-21T23:39:51Z

💊 CircleCI build failures summary and remediations

As of commit 1633a86:

None of the build failures appear to be your fault.

1/1 broken upstream at merge base e0ffe72 since Jan 25
Please rebase on the viable/strict branch (expand for instructions)

If your commit is newer than viable/strict, you can try basing on an older, stable commit:
```
git fetch origin viable/strict
git rebase --onto viable/strict $(git merge-base origin/master HEAD)
```
If your commit is older than viable/strict:
```
git fetch origin viable/strict
git rebase viable/strict
```
Check out the recency history of this "viable master" tracking branch.

Detailed failure analysis

One may explore the probable reasons each build failed interactively on the Dr. CI website.

🚧 1 upstream failure recognized by patterns:

These builds matched patterns, but were probably caused by upstream breakages:

pytorch_macos_10_13_py3_test from Jan 25

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 29 times.

It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/) [ghstack-poisoned]

Pull Request resolved: #32466 It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. ghstack-source-id: 97007290 Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/)

zhaojuanmao

overall looks good! thanks for working on this. also, would you please add a test in rpc_test.py that can show the remote script call can receive remote exception?

torch/csrc/distributed/rpc/python_functions.cpp

torch/csrc/distributed/rpc/script_functions.cpp

zhaojuanmao · 2020-01-23T19:07:21Z

torch/csrc/distributed/rpc/script_functions.cpp

  return futPtr;
 }

+PyRRef remoteTorchscript(


could we refactor pyRemoteBuiltin and make it to be used by both builtin and torchscript call?

I tried, the inheritance setup about ScripCall, RemoteCall, ScriptCall makes it hard to consolidate in this PR. It's could be another PR's work to streamline them.

We want to consolidation

pyRpcBuiltin and rpcTorchscript.

pyRemoteBuiltin and remoteTorchscript.

Maybe after removing template on RRef classes is a good time to re-visit this wish.

torch/distributed/rpc/api.py

torch/csrc/distributed/rpc/script_functions.cpp

torch/csrc/distributed/rpc/python_functions.h

It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/) [ghstack-poisoned]

Pull Request resolved: #32466 It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. ghstack-source-id: 97137716 Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/)

xush6528 · 2020-01-24T00:11:44Z

Update to resolve comments. Will wait for and rebase after #30630.

It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/) [ghstack-poisoned]

Pull Request resolved: #32466 It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. ghstack-source-id: 97192240 Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/)

zhaojuanmao

lgtm, please address inline comments and make sure all tests are green before landing

torch/csrc/distributed/rpc/request_callback_impl.cpp

torch/distributed/rpc/api.py

zhaojuanmao · 2020-01-24T23:33:12Z

test failures are real

It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/) [ghstack-poisoned]

Pull Request resolved: #32466 It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. ghstack-source-id: 97207196 Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/)

It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/) [ghstack-poisoned]

Pull Request resolved: #32466 It's a follow-up work of #32197. In #32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. ghstack-source-id: 97211168 Differential Revision: [D19440633](https://our.internmc.facebook.com/intern/diff/D19440633/)

xush6528 · 2020-01-25T09:12:28Z

The 2 failures are irrelevant to distributed module.

facebook-github-bot · 2020-01-25T11:08:15Z

This pull request has been merged in 6ad9e5c.

Summary: Pull Request resolved: pytorch#32466 It's a follow-up work of pytorch#32197. In pytorch#32197, `rpc.sync_rpc(..) `and `rpc.rpc_async(..)` support taking a TorchScript annotated Python function as the user function for RPC. This PR extend along this direction by making `rpc.remote(..)` support taking a TorchScript annotated Python function as well. ghstack-source-id: 97211168 Test Plan: # Unit tests ``` buck test mode/dev-nosan //caffe2/test/distributed/rpc:rpc_fork -- test_script_function_exception buck build mode/dev-nosan //caffe2/test/distributed/rpc:rpc_fork buck-out/gen/caffe2/test/distributed/rpc/rpc_fork\#binary.par -r test_script_function_exception ``` ``` buck test mode/dev-nosan //caffe2/test/distributed/rpc:dist_autograd_fork -- test_backward_simple_script_call buck build mode/dev-nosan //caffe2/test/distributed/rpc:dist_autograd_fork buck-out/gen/caffe2/test/distributed/rpc/dist_autograd_fork\#binary.par -r test_backward_simple_script_call ``` Differential Revision: D19440633 fbshipit-source-id: d37f6dcdc0b80d35ac7bcba46ad6f9b831c3779b

Pull Request resolved: #36266 RecordFunction starts but never ends at an appropriate location. Might be recording wrong time numbers. This is following up on #32197 and #32466 ghstack-source-id: 101798149 Differential Revision: [D7857991](https://our.internmc.facebook.com/intern/diff/D7857991/)

xush6528 requested review from mrshenli, pritamdamania87 and zhaojuanmao as code owners January 21, 2020 23:26

xush6528 requested a review from wanchaol January 21, 2020 23:27

xush6528 changed the title ~~[WIP] Support TorchScript call over remote API (RRef)~~ Support TorchScript call over remote API (RRef) Jan 22, 2020

zhaojuanmao reviewed Jan 23, 2020

View reviewed changes

mrshenli reviewed Jan 23, 2020

View reviewed changes

xush6528 mentioned this pull request Jan 24, 2020

Reduce RPC branches for Python/Built-inOp/TorchScript #32565

Open

zhaojuanmao approved these changes Jan 24, 2020

View reviewed changes

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

torch/distributed/rpc/api.py Outdated Show resolved Hide resolved

torch/distributed/rpc/api.py Outdated Show resolved Hide resolved

facebook-github-bot closed this in 6ad9e5c Jan 25, 2020

facebook-github-bot added the merged label Jan 25, 2020

facebook-github-bot deleted the gh/xush6528/62/head branch January 28, 2020 15:17

zhaojuanmao mentioned this pull request Feb 4, 2020

[WIP] Support TorchScript call over remote API (RRef) #32465

Closed

xush6528 mentioned this pull request Apr 8, 2020

[RPC x JIT] Integrate Python RPC TorchScript call with profiler #36266

Closed

mruberry added the Merged label Oct 28, 2020

Conversation

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

Uh oh!

Uh oh!

kostmo commented Jan 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CircleCI build failures summary and remediations

Detailed failure analysis

🚧 1 upstream failure recognized by patterns:

Uh oh!

zhaojuanmao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhaojuanmao Jan 23, 2020

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xush6528 commented Jan 24, 2020

Uh oh!

zhaojuanmao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhaojuanmao commented Jan 24, 2020

Uh oh!

xush6528 commented Jan 25, 2020

Uh oh!

facebook-github-bot commented Jan 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

xush6528 commented Jan 21, 2020 •

edited

Loading

kostmo commented Jan 21, 2020 •

edited

Loading

xush6528 Jan 23, 2020 •

edited

Loading