Fix inlining interface call in fork subgraph #43790

bzinodev · 2020-08-28T18:19:24Z

Stack from ghstack:

Fix inlining interface call in fork subgraph #43790 Fix inlining interface call in fork subgraph

Interface calls were not handled properly when they are used in fork
subgraph. This PR fixes this issue.

Differential Revision: D23402039

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. [ghstack-poisoned]

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. ghstack-source-id: 19c117a Pull Request resolved: #43790

dr-ci · 2020-08-28T18:59:32Z

💊 CI failures summary and remediations

As of commit 63269ea (more details on the Dr. CI page):

2/2 failures possibly* introduced in this PR
- 1/2 non-CircleCI failure(s)

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_linux_bionic_py3_8_gcc9_coverage_test (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Sep 22 20:04:38 [E request_callback_no_python.cpp:618] Received error while processing request type 2: RuntimeError: Can not pickle torch.futures.Future

Sep 22 20:04:38 At: 
Sep 22 20:04:38   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(94): serialize 
Sep 22 20:04:38   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(146): serialize 
Sep 22 20:04:38  
Sep 22 20:04:38 [E request_callback_no_python.cpp:618] Received error while processing request type 2: RuntimeError: Can not pickle torch.futures.Future 
Sep 22 20:04:38  
Sep 22 20:04:38 At: 
Sep 22 20:04:38   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(94): serialize 
Sep 22 20:04:38   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(146): serialize 
Sep 22 20:04:38  
Sep 22 20:04:38 [E request_callback_no_python.cpp:618] Received error while processing request type 2: RuntimeError: Can not pickle torch.futures.Future 
Sep 22 20:04:38  
Sep 22 20:04:38 At: 
Sep 22 20:04:38   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(94): serialize 
Sep 22 20:04:38   /opt/conda/lib/python3.8/site-packages/torch/distributed/rpc/internal.py(146): serialize 
Sep 22 20:04:38  
Sep 22 20:04:39 ok (1.446s) 
Sep 22 20:04:40   test_return_future_remote (__main__.ProcessGroupRpcTestWithSpawn) ... ok (1.443s) 
Sep 22 20:04:42   test_return_local_rrefs (__main__.ProcessGroupRpcTestWithSpawn) ... ok (1.444s) 
Sep 22 20:04:43   test_rpc_profiling_remote_record_function (__main__.ProcessGroupRpcTestWithSpawn) ... ERROR:root:Caught exception:  
Sep 22 20:04:43 Traceback (most recent call last):

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm3.7-py3.6

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 or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 14 times.

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. Differential Revision: [D23402039](https://our.internmc.facebook.com/intern/diff/D23402039) [ghstack-poisoned]

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. ghstack-source-id: 07433ee Pull Request resolved: #43790

eellison · 2020-08-31T23:49:07Z

torch/csrc/jit/passes/freeze_module.cpp

@@ -479,6 +482,9 @@ class AttributePropagator {
    if (node->kind() != prim::GetAttr) {
      return;
    }
+    if (!node->output()->type()->cast<ClassType>()) {


Why are we doing this check ? Could you add a comment into the code ?

One of my unit test fired because I call a global method? I don't anticipate this to happen in practice.
I feel this is still fragile because we expect the first argument of fork to be submodule of the invoked method which is just a convention.
Anyway freezing will fail if it tries to resolve an attribute that does not exist. I guess it is okay in case someone wants to get creative and pass the submodule as second argument ...

I will add a comment

I think we should still be recursively analyzing the fork even if it's not a forked module - don't we still want to inline interface calls and such ?

I agree. I updated my PR to handle in this case

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. Differential Revision: [D23402039](https://our.internmc.facebook.com/intern/diff/D23402039) [ghstack-poisoned]

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. ghstack-source-id: 84f0fc1 Pull Request resolved: #43790

eellison

LGTM

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. Differential Revision: [D23402039](https://our.internmc.facebook.com/intern/diff/D23402039) [ghstack-poisoned]

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. ghstack-source-id: fb0eb82 Pull Request resolved: #43790

facebook-github-bot · 2020-09-23T20:18:33Z

@bzinodev merged this pull request in 4d80c8c.

Fix inlining interface call in fork subgraph

fc63bba

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. [ghstack-poisoned]

bzinodev requested a review from apaszke as a code owner August 28, 2020 18:19

bzinodev added a commit that referenced this pull request Aug 28, 2020

Fix inlining interface call in fork subgraph

99f338d

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. ghstack-source-id: 19c117a Pull Request resolved: #43790

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

Update on "Fix inlining interface call in fork subgraph"

b4e5070

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. Differential Revision: [D23402039](https://our.internmc.facebook.com/intern/diff/D23402039) [ghstack-poisoned]

bzinodev added a commit that referenced this pull request Aug 29, 2020

Fix inlining interface call in fork subgraph

c8bae0f

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. ghstack-source-id: 07433ee Pull Request resolved: #43790

bzinodev requested a review from eellison August 31, 2020 16:49

eellison reviewed Aug 31, 2020

View reviewed changes

Update on "Fix inlining interface call in fork subgraph"

2d21c39

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. Differential Revision: [D23402039](https://our.internmc.facebook.com/intern/diff/D23402039) [ghstack-poisoned]

bzinodev added a commit that referenced this pull request Sep 16, 2020

Fix inlining interface call in fork subgraph

bdf8cba

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. ghstack-source-id: 84f0fc1 Pull Request resolved: #43790

eellison approved these changes Sep 21, 2020

View reviewed changes

Update on "Fix inlining interface call in fork subgraph"

63269ea

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. Differential Revision: [D23402039](https://our.internmc.facebook.com/intern/diff/D23402039) [ghstack-poisoned]

bzinodev added a commit that referenced this pull request Sep 22, 2020

Fix inlining interface call in fork subgraph

7b9500d

Interface calls were not handled properly when they are used in fork subgraph. This PR fixes this issue. ghstack-source-id: fb0eb82 Pull Request resolved: #43790

facebook-github-bot closed this in 4d80c8c Sep 23, 2020

facebook-github-bot added the merged label Sep 23, 2020

facebook-github-bot deleted the gh/bzinodev/14/head branch September 27, 2020 14:15

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix inlining interface call in fork subgraph #43790

Fix inlining interface call in fork subgraph #43790

Uh oh!

bzinodev commented Aug 28, 2020 •

edited

Loading

Uh oh!

dr-ci bot commented Aug 28, 2020 •

edited

Loading

Uh oh!

eellison Aug 31, 2020

Uh oh!

bzinodev Sep 3, 2020

Uh oh!

eellison Sep 4, 2020

Uh oh!

bzinodev Sep 16, 2020

Uh oh!

eellison left a comment

Uh oh!

facebook-github-bot commented Sep 23, 2020

Uh oh!

Uh oh!

Fix inlining interface call in fork subgraph #43790

Fix inlining interface call in fork subgraph #43790

Uh oh!

Conversation

bzinodev commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_linux_bionic_py3_8_gcc9_coverage_test (1/1)

ci.pytorch.org: 1 failed

Uh oh!

eellison Aug 31, 2020

Choose a reason for hiding this comment

Uh oh!

bzinodev Sep 3, 2020

Choose a reason for hiding this comment

Uh oh!

eellison Sep 4, 2020

Choose a reason for hiding this comment

Uh oh!

bzinodev Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

eellison left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Sep 23, 2020

Uh oh!

Uh oh!

bzinodev commented Aug 28, 2020 •

edited

Loading

dr-ci bot commented Aug 28, 2020 •

edited

Loading