Rework requires_grad on DifferentiableGraphOp #57575

jjsjann123 · 2021-05-04T22:50:50Z

Stack from ghstack:

Rework requires_grad on DifferentiableGraphOp #57575 Rework requires_grad on DifferentiableGraphOp
fixing DifferentiableGraphOp updating requires_grad on input tensor list; python test added to verify the test #57574 fixing DifferentiableGraphOp updating requires_grad on input tensor list; python test added to verify the test

This PR does two things:

reverts "Manual revert of D27369251 (Manual revert of D27369251 #56080)" in commit
92a09fb.
fixing DifferentiableGraph output with wrong requires_grad flag

Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is
retrieved from profiling information. We previously only retrieves the profiling
information on the first profile node in all its uses. However, in case where
control flows are present, we need to iteratively search for profile node with
profiling information available, in case the first use is in an inactive code
path.

e.g.

  graph(%0 : Tensor,
        %1 : Bool):
  ..., %2 : Tensor = prim::DifferentiableGraph_0(%0)
  %3 : Tensor = prim::If(%1)
    block0():
      %4 : Tensor = prim::DifferentiableGraph_1(%2)
      -> (%4)
    block1():
      %5 : Tensor = prim::DifferentiableGraph_2(%2)
      -> (%5)
  -> (%3)
with prim::DifferentiableGraph_0 = graph(%0 : Tensor):
  ...
  %out : Tensor = aten::operation(...)
  ...
  return (..., %out)
with prim::DifferentiableGraph_1 = graph(%0 : Tensor):
  %temp : Tensor = prim::profile[profiled_type=Tensor](%0)
  ...
with prim::DifferentiableGraph_2 = graph(%0 : Tensor):
  %temp : Tensor = prim::profile[profiled_type=Float(...)](%0)
  ...

Differential Revision: D29038773

facebook-github-bot · 2021-05-04T22:50:58Z

💊 CI failures summary and remediations

As of commit 8f02ee2 (more details on the Dr. CI page):

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

🕵️ 1 new failure recognized by patterns

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

pytorch_linux_xenial_py3_6_gcc5_4_test (1/1)

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

Jun 10 20:23:41 [E request_callback_no_python.c...quest type 275: Unexpected end of pickler archive.

Jun 10 20:23:41   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/dist_utils.py", line 100, in new_test_method
Jun 10 20:23:41     return_value = old_test_method(self, *arg, **kwargs)
Jun 10 20:23:41   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/distributed/ddp_under_dist_autograd_test.py", line 654, in test_ddp_dist_autograd_local_vs_remote
Jun 10 20:23:41     args=(remote_layer1.module_rref, context_id),
Jun 10 20:23:41   File "/opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/api.py", line 77, in wrapper
Jun 10 20:23:41     return func(*args, **kwargs)
Jun 10 20:23:41   File "/opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/api.py", line 757, in rpc_sync
Jun 10 20:23:41     return fut.wait()
Jun 10 20:23:41 RuntimeError: RPCErr:1:RPC ran for more than set timeout (60000 ms) and will now be marked with an error
Jun 10 20:23:41  exiting process with exit code: {MultiProcessTestCase.TEST_ERROR_EXIT_CODE}
Jun 10 20:23:41 [E request_callback_no_python.cpp:552] Received error while processing request type 275: Unexpected end of pickler archive.
Jun 10 20:23:41 Exception raised from readSlowWithBuffer at /var/lib/jenkins/workspace/torch/csrc/jit/serialization/unpickler.cpp:756 (most recent call first):
Jun 10 20:23:41 frame #0: c10::Error::Error(c10::SourceLocation, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) + 0x69 (0x7f2d1ba76ed9 in /opt/conda/lib/python3.6/site-packages/torch/lib/libc10.so)
Jun 10 20:23:41 frame #1: c10::detail::torchCheckFail(char const*, char const*, unsigned int, char const*) + 0xc5 (0x7f2d1ba73155 in /opt/conda/lib/python3.6/site-packages/torch/lib/libc10.so)
Jun 10 20:23:41 frame #2: <unknown function> + 0x3e4b448 (0x7f2d14347448 in /opt/conda/lib/python3.6/site-packages/torch/lib/libtorch_cpu.so)
Jun 10 20:23:41 frame #3: torch::jit::Unpickler::run() + 0xdf (0x7f2d1435197f in /opt/conda/lib/python3.6/site-packages/torch/lib/libtorch_cpu.so)
Jun 10 20:23:41 frame #4: torch::jit::Unpickler::parse_ivalue() + 0x2e (0x7f2d14351aee in /opt/conda/lib/python3.6/site-packages/torch/lib/libtorch_cpu.so)
Jun 10 20:23:41 frame #5: torch::jit::unpickle(std::function<unsigned long (char*, unsigned long)>, std::function<c10::StrongTypePtr (c10::QualifiedName const&)>, c10::ArrayRef<at::Tensor>) + 0x25c (0x7f2d143249cc in /opt/conda/lib/python3.6/site-packages/torch/lib/libtorch_cpu.so)
Jun 10 20:23:41 frame #6: torch::jit::unpickle(char const*, unsigned long, std::function<c10::StrongTypePtr (c10::QualifiedName const&)>, c10::ArrayRef<at::Tensor>) + 0xdd (0x7f2d14324edd in /opt/conda/lib/python3.6/site-packages/torch/lib/libtorch_cpu.so)
Jun 10 20:23:41 frame #7: torch::distributed::autograd::CleanupAutogradContextReq::fromMessage(torch::distributed::rpc::Message const&) + 0xcb (0x7f2d145b838b in /opt/conda/lib/python3.6/site-packages/torch/lib/libtorch_cpu.so)
Jun 10 20:23:41 frame #8: torch::distributed::rpc::deserializeRequest(torch::distributed::rpc::Message const&) + 0x1ed (0x7f2d145fec1d in /opt/conda/lib/python3.6/site-packages/torch/lib/libtorch_cpu.so)

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm4.2-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 to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

This PR does two things: 1. reverts "Manual revert of D27369251 (#56080)" in commit 92a09fb. 2. fixing DifferentiableGraph output with wrong requires_grad flag Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is retrieved from profiling information. We previously only retrieves the profiling information on the first profile node in all its uses. However, in case where control flows are present, we need to iteratively search for profile node with profiling information available, in case the first use is in an inactive code path. e.g. ``` graph(%0 : Tensor, %1 : Bool): ..., %2 : Tensor = prim::DifferentiableGraph_0(%0) %3 : Tensor = prim::If(%1) block0(): %4 : Tensor = prim::DifferentiableGraph_1(%2) -> (%4) block1(): %5 : Tensor = prim::DifferentiableGraph_2(%2) -> (%5) -> (%3) with prim::DifferentiableGraph_0 = graph(%0 : Tensor): ... %out : Tensor = aten::operation(...) ... return (..., %out) with prim::DifferentiableGraph_1 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Tensor](%0) ... with prim::DifferentiableGraph_2 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Float(...)](%0) ... ``` [ghstack-poisoned]

jjsjann123 · 2021-05-04T22:53:54Z

This is the second part in #56466 that restores the reverted code change, combined with the fixes in #55767

This PR does two things: 1. reverts "Manual revert of D27369251 (#56080)" in commit 92a09fb. 2. fixing DifferentiableGraph output with wrong requires_grad flag Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is retrieved from profiling information. We previously only retrieves the profiling information on the first profile node in all its uses. However, in case where control flows are present, we need to iteratively search for profile node with profiling information available, in case the first use is in an inactive code path. e.g. ``` graph(%0 : Tensor, %1 : Bool): ..., %2 : Tensor = prim::DifferentiableGraph_0(%0) %3 : Tensor = prim::If(%1) block0(): %4 : Tensor = prim::DifferentiableGraph_1(%2) -> (%4) block1(): %5 : Tensor = prim::DifferentiableGraph_2(%2) -> (%5) -> (%3) with prim::DifferentiableGraph_0 = graph(%0 : Tensor): ... %out : Tensor = aten::operation(...) ... return (..., %out) with prim::DifferentiableGraph_1 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Tensor](%0) ... with prim::DifferentiableGraph_2 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Float(...)](%0) ... ``` ghstack-source-id: e4ee036f68b63966ea1566dab6377e4413c26679 Pull Request resolved: #57575

This PR does two things: 1. reverts "Manual revert of D27369251 (#56080)" in commit 92a09fb. 2. fixing DifferentiableGraph output with wrong requires_grad flag Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is retrieved from profiling information. We previously only retrieves the profiling information on the first profile node in all its uses. However, in case where control flows are present, we need to iteratively search for profile node with profiling information available, in case the first use is in an inactive code path. e.g. ``` graph(%0 : Tensor, %1 : Bool): ..., %2 : Tensor = prim::DifferentiableGraph_0(%0) %3 : Tensor = prim::If(%1) block0(): %4 : Tensor = prim::DifferentiableGraph_1(%2) -> (%4) block1(): %5 : Tensor = prim::DifferentiableGraph_2(%2) -> (%5) -> (%3) with prim::DifferentiableGraph_0 = graph(%0 : Tensor): ... %out : Tensor = aten::operation(...) ... return (..., %out) with prim::DifferentiableGraph_1 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Tensor](%0) ... with prim::DifferentiableGraph_2 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Float(...)](%0) ... ``` [ghstack-poisoned]

Krovatkin · 2021-05-06T17:29:41Z

@jjsjann123 let me do some extra testing against this stack internally. Fingers crossed.

This PR does two things: 1. reverts "Manual revert of D27369251 (#56080)" in commit 92a09fb. 2. fixing DifferentiableGraph output with wrong requires_grad flag Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is retrieved from profiling information. We previously only retrieves the profiling information on the first profile node in all its uses. However, in case where control flows are present, we need to iteratively search for profile node with profiling information available, in case the first use is in an inactive code path. e.g. ``` graph(%0 : Tensor, %1 : Bool): ..., %2 : Tensor = prim::DifferentiableGraph_0(%0) %3 : Tensor = prim::If(%1) block0(): %4 : Tensor = prim::DifferentiableGraph_1(%2) -> (%4) block1(): %5 : Tensor = prim::DifferentiableGraph_2(%2) -> (%5) -> (%3) with prim::DifferentiableGraph_0 = graph(%0 : Tensor): ... %out : Tensor = aten::operation(...) ... return (..., %out) with prim::DifferentiableGraph_1 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Tensor](%0) ... with prim::DifferentiableGraph_2 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Float(...)](%0) ... ``` ghstack-source-id: 781a52aee2e152a59a031e8aec687c78416b7acc Pull Request resolved: #57575

This PR does two things: 1. reverts "Manual revert of D27369251 (#56080)" in commit 92a09fb. 2. fixing DifferentiableGraph output with wrong requires_grad flag Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is retrieved from profiling information. We previously only retrieves the profiling information on the first profile node in all its uses. However, in case where control flows are present, we need to iteratively search for profile node with profiling information available, in case the first use is in an inactive code path. e.g. ``` graph(%0 : Tensor, %1 : Bool): ..., %2 : Tensor = prim::DifferentiableGraph_0(%0) %3 : Tensor = prim::If(%1) block0(): %4 : Tensor = prim::DifferentiableGraph_1(%2) -> (%4) block1(): %5 : Tensor = prim::DifferentiableGraph_2(%2) -> (%5) -> (%3) with prim::DifferentiableGraph_0 = graph(%0 : Tensor): ... %out : Tensor = aten::operation(...) ... return (..., %out) with prim::DifferentiableGraph_1 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Tensor](%0) ... with prim::DifferentiableGraph_2 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Float(...)](%0) ... ``` [ghstack-poisoned]

Krovatkin

Krovatkin · 2021-06-09T22:02:39Z

@jjsjann123 could you please rebase?

This PR does two things: 1. reverts "Manual revert of D27369251 (#56080)" in commit 92a09fb. 2. fixing DifferentiableGraph output with wrong requires_grad flag Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is retrieved from profiling information. We previously only retrieves the profiling information on the first profile node in all its uses. However, in case where control flows are present, we need to iteratively search for profile node with profiling information available, in case the first use is in an inactive code path. e.g. ``` graph(%0 : Tensor, %1 : Bool): ..., %2 : Tensor = prim::DifferentiableGraph_0(%0) %3 : Tensor = prim::If(%1) block0(): %4 : Tensor = prim::DifferentiableGraph_1(%2) -> (%4) block1(): %5 : Tensor = prim::DifferentiableGraph_2(%2) -> (%5) -> (%3) with prim::DifferentiableGraph_0 = graph(%0 : Tensor): ... %out : Tensor = aten::operation(...) ... return (..., %out) with prim::DifferentiableGraph_1 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Tensor](%0) ... with prim::DifferentiableGraph_2 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Float(...)](%0) ... ``` [ghstack-poisoned]

This PR does two things: 1. reverts "Manual revert of D27369251 (#56080)" in commit 92a09fb. 2. fixing DifferentiableGraph output with wrong requires_grad flag Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is retrieved from profiling information. We previously only retrieves the profiling information on the first profile node in all its uses. However, in case where control flows are present, we need to iteratively search for profile node with profiling information available, in case the first use is in an inactive code path. e.g. ``` graph(%0 : Tensor, %1 : Bool): ..., %2 : Tensor = prim::DifferentiableGraph_0(%0) %3 : Tensor = prim::If(%1) block0(): %4 : Tensor = prim::DifferentiableGraph_1(%2) -> (%4) block1(): %5 : Tensor = prim::DifferentiableGraph_2(%2) -> (%5) -> (%3) with prim::DifferentiableGraph_0 = graph(%0 : Tensor): ... %out : Tensor = aten::operation(...) ... return (..., %out) with prim::DifferentiableGraph_1 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Tensor](%0) ... with prim::DifferentiableGraph_2 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Float(...)](%0) ... ``` ghstack-source-id: 7d493ce5cf179a04b7a05723e2a18117e29313f8 Pull Request resolved: #57575

This PR does two things: 1. reverts "Manual revert of D27369251 (#56080)" in commit 92a09fb. 2. fixing DifferentiableGraph output with wrong requires_grad flag Fixing requires_grad on outputs from DifferentiableGraph, the proper flag is retrieved from profiling information. We previously only retrieves the profiling information on the first profile node in all its uses. However, in case where control flows are present, we need to iteratively search for profile node with profiling information available, in case the first use is in an inactive code path. e.g. ``` graph(%0 : Tensor, %1 : Bool): ..., %2 : Tensor = prim::DifferentiableGraph_0(%0) %3 : Tensor = prim::If(%1) block0(): %4 : Tensor = prim::DifferentiableGraph_1(%2) -> (%4) block1(): %5 : Tensor = prim::DifferentiableGraph_2(%2) -> (%5) -> (%3) with prim::DifferentiableGraph_0 = graph(%0 : Tensor): ... %out : Tensor = aten::operation(...) ... return (..., %out) with prim::DifferentiableGraph_1 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Tensor](%0) ... with prim::DifferentiableGraph_2 = graph(%0 : Tensor): %temp : Tensor = prim::profile[profiled_type=Float(...)](%0) ... ``` [ghstack-poisoned]

Krovatkin · 2021-06-10T19:06:01Z

@Krovatkin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-06-14T17:37:51Z

@Krovatkin merged this pull request in 9ad0de3.

jjsjann123 mentioned this pull request May 4, 2021

fixing DifferentiableGraphOp updating requires_grad on input tensor list; python test added to verify the test #57574

Closed

facebook-github-bot added oncall: jit Add this issue/PR to JIT oncall triage queue cla signed labels May 4, 2021

This was referenced May 4, 2021

Avoid DifferentiableGraph operation mutate requires_grad flag on input tensor list #56466

Closed

fixing DifferentiableGraph output with wrong requires_grad flag #55767

Closed

pytorchbot added the open source label May 4, 2021

Krovatkin self-requested a review May 4, 2021 23:44

Krovatkin approved these changes Jun 9, 2021

View reviewed changes

facebook-github-bot closed this in 9ad0de3 Jun 14, 2021

facebook-github-bot added the Merged label Jun 14, 2021

facebook-github-bot deleted the gh/jjsjann123/6/head branch June 18, 2021 14:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework requires_grad on DifferentiableGraphOp #57575

Rework requires_grad on DifferentiableGraphOp #57575

jjsjann123 commented May 4, 2021 •

edited by Krovatkin

Loading

facebook-github-bot commented May 4, 2021 •

edited

Loading

jjsjann123 commented May 4, 2021

Krovatkin commented May 6, 2021

Krovatkin left a comment

Krovatkin commented Jun 9, 2021

Krovatkin commented Jun 10, 2021

facebook-github-bot commented Jun 14, 2021

Rework requires_grad on DifferentiableGraphOp #57575

Rework requires_grad on DifferentiableGraphOp #57575

Conversation

jjsjann123 commented May 4, 2021 • edited by Krovatkin Loading

facebook-github-bot commented May 4, 2021 • edited Loading

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_linux_xenial_py3_6_gcc5_4_test (1/1)

ci.pytorch.org: 1 failed

jjsjann123 commented May 4, 2021

Krovatkin commented May 6, 2021

Krovatkin left a comment

Choose a reason for hiding this comment

Krovatkin commented Jun 9, 2021

Krovatkin commented Jun 10, 2021

facebook-github-bot commented Jun 14, 2021

jjsjann123 commented May 4, 2021 •

edited by Krovatkin

Loading

facebook-github-bot commented May 4, 2021 •

edited

Loading