Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Revert clearing backward cache for retain_graph flag #4249

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

BarclayII
Copy link
Collaborator

This change is introduced in #3386 where it aims to resolve memory leaks for certain PyTorch versions (1.8.1 at that time; see #3230 and #3365). The fix however introduced new problems where retain_graph=True during backward is broken (see #4078). I reverted this PR and found that the memory leak no longer appears in at least PyTorch 1.11.0 and 1.12.0.

@BarclayII BarclayII marked this pull request as ready for review July 13, 2022 04:53
@dgl-bot
Copy link
Collaborator

dgl-bot commented Jul 13, 2022

To trigger regression tests:

  • @dgl-bot run [instance-type] [which tests] [compare-with-branch];
    For example: @dgl-bot run g4dn.4xlarge all dmlc/master or @dgl-bot run c5.9xlarge kernel,api dmlc/master

@dgl-bot
Copy link
Collaborator

dgl-bot commented Jul 13, 2022

Commit ID: a51ba85

Build ID: 1

Status: ✅ CI test succeeded

Report path: link

Full logs path: link

@BarclayII
Copy link
Collaborator Author

The problem also did not appear in 1.9.1. I think we are good.

@jermainewang jermainewang added the Release Candidate Candidate PRs for the upcoming release label Jul 14, 2022
@dgl-bot
Copy link
Collaborator

dgl-bot commented Jul 14, 2022

Commit ID: c29f330

Build ID: 2

Status: ✅ CI test succeeded

Report path: link

Full logs path: link

@yaox12 yaox12 merged commit 2efdaa5 into dmlc:master Jul 14, 2022
BarclayII added a commit to BarclayII/dgl that referenced this pull request Aug 10, 2022
Co-authored-by: Minjie Wang <wmjlyjemaine@gmail.com>
@frozenbugs frozenbugs removed the Release Candidate Candidate PRs for the upcoming release label Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants