retain_graph=True in compiled_autograd #110367

voznesenskym · 2023-10-01T21:55:42Z

Stack from ghstack (oldest at bottom):

-> retain_graph=True in compiled_autograd #110367

Adds support for retain_graph=True - known as keep_graph_ internally in the autograd engine.

cc @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov @ColinPeppler

keep_graph in compiled_autograd keep_graph in compiled_autograd [ghstack-poisoned]

pytorch-bot · 2023-10-01T21:55:45Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/110367

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (3 Unrelated Failures)

As of commit a606c35 with merge base efdf155 ():

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

voznesenskym · 2023-10-01T21:56:30Z

test/inductor/test_compiled_autograd.py

-    "test_hook_closure_cycle_use_custom_function_False_use_tensor_hook_True",  # compiled_autograd does not support keep_graph
-    "test_hook_closure_cycle_use_custom_function_True_use_tensor_hook_False",  # compiled_autograd does not support keep_graph
-    "test_hook_closure_cycle_use_custom_function_True_use_tensor_hook_True",  # compiled_autograd does not support keep_graph
+    "test_accumulate_grad",  # RuntimeError: compiled_autograd does not support create_graph


10 tests passed now, but regenerated all to get new errors.

jansel

Looks like tests are failing

jansel · 2023-10-02T05:15:54Z

torch/_dynamo/compiled_autograd.py

+def keep_graph(graph, keep_graph):
+    graph.keep_graph = keep_graph
+    return graph
+
+
+def should_keep_graph(graph):
+    return graph.keep_graph


Why does this need to be in python?

Setting breakpoints / debugging is easier ;)

I can move entirely to C++ setting, reading

keep_graph in compiled_autograd keep_graph in compiled_autograd cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

voznesenskym · 2023-10-02T08:19:06Z

Looks like tests are failing

Most of the actual tests seem to pass, I think its a static analysis catching (correctly) a falsey condition leading to uninitialized ptr access.

Fixed.

ezyang · 2023-10-04T14:09:07Z

torch/csrc/dynamo/python_compiled_autograd.cpp

-    cache->compiled_fn = check(call_end_capture(py_compiler, state.outputs));
+    PyObject* graph = check(call_end_capture(py_compiler, state.outputs));
+    cache->compiled_fn =
+        check(call_keep_graph(check(graph), graph_task.keep_graph_));


this can be removed

ezyang · 2023-10-04T14:10:14Z

I agree with the conclusion that retain_graph does not affect the graph structure (though, it WOULD if we let the compiled autograd graph steal all of the saved variables). Given this, most of the changes in this PR can be reverted and you end up with literally only the lines that toggle releasing variable at the end.

albanD · 2023-10-04T17:22:03Z

Usually you only need retain_graph for double backwards like situations, are we running into this?

It is enabled when create_graph=True and thus in this situation it is yes.
It is also used when you need to backward a given model multiple times without changing it (like in GAN use cases).

Adds support for retain_graph=True - known as keep_graph_ internally in the autograd engine. cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

keep_graph in compiled_autograd keep_graph in compiled_autograd ghstack-source-id: 6188646 Pull Request resolved: #110367 keep_graph in compiled_autograd keep_graph in compiled_autograd keep_graph in compiled_autograd Fixes, feedback

voznesenskym · 2023-10-05T23:21:26Z

@pytorchbot merge

pytorchmergebot · 2023-10-05T23:23:15Z

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

voznesenskym · 2023-10-06T07:14:09Z

@pytorchbot merge

pytorchmergebot · 2023-10-06T07:16:01Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2023-10-06T07:16:21Z

Merge failed

Reason: 14 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

voznesenskym · 2023-10-06T08:19:35Z

@pytorchbot merge -f "Flaky CI"

voznesenskym · 2023-10-06T08:19:46Z

pytorchmergebot · 2023-10-06T08:21:29Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

This change is needed after pytorch/test-infra#4579 and pytorch/test-infra#4610. All invalid cancelled signals have been removed from Dr.CI and HUD. So trymerge should ignore them accordingly for a consistent experience. ### Testing #110367 (comment) is the PR where a bunch of invalid cancelled signals showed up and blocked merges Pull Request resolved: #110690 Approved by: https://github.com/clee2000, https://github.com/ZainRizvi

keep_graph in compiled_autograd

a72a78f

keep_graph in compiled_autograd keep_graph in compiled_autograd [ghstack-poisoned]

voznesenskym requested review from albanD and soulitzer as code owners October 1, 2023 21:55

voznesenskym mentioned this pull request Oct 1, 2023

Change compiled_autograd tests to xfail instead of skip #110348

Closed

github-actions bot added module: inductor module: dynamo ciflow/inductor labels Oct 1, 2023

github-actions bot requested review from SherlockNoMad, antoniojkim, bdhirsh, ezyang, miladm and wconstab October 1, 2023 21:55

voznesenskym commented Oct 1, 2023

View reviewed changes

voznesenskym requested a review from jansel October 1, 2023 22:28

jansel requested changes Oct 2, 2023

View reviewed changes

voznesenskym requested review from jansel and removed request for SherlockNoMad, albanD, antoniojkim, bdhirsh, ezyang, miladm, soulitzer and wconstab October 2, 2023 08:19

ezyang reviewed Oct 4, 2023

View reviewed changes

voznesenskym changed the title ~~keep_graph in compiled_autograd~~ retain_graph=True in compiled_autograd Oct 5, 2023

voznesenskym requested a review from jansel October 5, 2023 21:02

jansel approved these changes Oct 5, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 5, 2023

pytorchmergebot added the merging label Oct 5, 2023

pytorchmergebot removed the merging label Oct 5, 2023

voznesenskym added the topic: not user facing topic category label Oct 6, 2023

pytorchmergebot added the merging label Oct 6, 2023

pytorchmergebot removed the merging label Oct 6, 2023

pytorchmergebot added the merging label Oct 6, 2023

pytorchmergebot added Merged and removed merging labels Oct 6, 2023

pytorchmergebot closed this in 7d98549 Oct 6, 2023

huydhn mentioned this pull request Oct 6, 2023

Handle invalid cancellation signals in trymerge #110690

Closed

facebook-github-bot deleted the gh/voznesenskym/232/head branch October 9, 2023 14:24

retain_graph=True in compiled_autograd #110367

retain_graph=True in compiled_autograd #110367

Uh oh!

Conversation

voznesenskym commented Oct 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Oct 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/110367

✅ You can merge normally! (3 Unrelated Failures)

Uh oh!

voznesenskym Oct 1, 2023

Choose a reason for hiding this comment

Uh oh!

jansel left a comment

Choose a reason for hiding this comment

Uh oh!

jansel Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

voznesenskym Oct 2, 2023 • 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!

voznesenskym commented Oct 2, 2023

Uh oh!

ezyang Oct 4, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang commented Oct 4, 2023

Uh oh!

albanD commented Oct 4, 2023

Uh oh!

voznesenskym commented Oct 5, 2023

Uh oh!

pytorchmergebot commented Oct 5, 2023

Merge failed

Uh oh!

voznesenskym commented Oct 6, 2023

Uh oh!

pytorchmergebot commented Oct 6, 2023

Merge started

Uh oh!

pytorchmergebot commented Oct 6, 2023

Merge failed

Uh oh!

voznesenskym commented Oct 6, 2023

Uh oh!

voznesenskym commented Oct 6, 2023

Uh oh!

pytorchmergebot commented Oct 6, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

voznesenskym commented Oct 1, 2023 •

edited

Loading

pytorch-bot bot commented Oct 1, 2023 •

edited

Loading

voznesenskym Oct 2, 2023 •

edited

Loading