Skip to content

Back out "Make grad point to bucket buffer in DDP to save memory usage" #43557

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.

Sign up for GitHub

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

Jump to bottom
Closed
liaimi wants to merge 1 commit into from
Closed

Back out "Make grad point to bucket buffer in DDP to save memory usage" #43557

liaimi wants to merge 1 commit into from

Conversation

@liaimi
Copy link

@liaimi liaimi commented Aug 25, 2020

Summary: backout the diff that caused some errors in pytext distributed training

Test Plan: Tested by rayhou who verified reverting the diff works

Differential Revision: D23320238

@facebook-github-bot
Back out "Make grad point to bucket buffer in DDP to save memory usage"
ff2bcb9 
Summary: backout the diff that caused some errors in pytext distributed training

Test Plan: Tested by rayhou who verified reverting the diff works

Differential Revision: D23320238

fbshipit-source-id: 7f3edff4a8cbdeb332449b9863c8e9ac18b3009f
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 25, 2020

This pull request was exported from Phabricator. Differential Revision: D23320238

@dr-ci
Copy link

dr-ci bot commented Aug 25, 2020 •
edited
Loading

💊 CI failures summary and remediations

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

  • 1/2 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)
  • 1/2 broken upstream at merge base 6a2d7a0 since Aug 25

🚧 1 ongoing upstream failure:

These were probably caused by upstream breakages that are not fixed yet:

ci.pytorch.org: 1 failed

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

@zhaojuanmao
Copy link
Contributor

zhaojuanmao commented Aug 25, 2020 •
edited
Loading

some internal application depends on third party zero_grad where we can not easily change how detach_() is called there. So reverting this diff right now to unblock internal application.

As follow up, we are planning to add one option in DDP API, the feature of "grad is view of bucket in DDP" will be disabled in default. If users want to enable this feature, they need to explicitly enable this feature and make sure detach_() is called properly in their zero_grad() codes. ===> this can resolve the back compatibility issues.

@zhaojuanmao zhaojuanmao mentioned this pull request Aug 25, 2020
Closed
12 tasks
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 26, 2020

This pull request has been merged in f35e069.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhaojuanmao zhaojuanmao zhaojuanmao approved these changes

@albanD albanD Awaiting requested review from albanD

@apaszke apaszke Awaiting requested review from apaszke

@mrshenli mrshenli Awaiting requested review from mrshenli

@pietern pietern Awaiting requested review from pietern

@pritamdamania87 pritamdamania87 Awaiting requested review from pritamdamania87

Assignees

No one assigned

Labels

fb-exported Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@liaimi @facebook-github-bot @zhaojuanmao @mruberry