move rebuild buckets from end of first iteration to beginning of second iteration by zhaojuanmao · Pull Request #44326 · pytorch/pytorch

zhaojuanmao · 2020-09-08T20:43:11Z

Stack from ghstack:

refactor intialize bucket views #44330 refactor intialize bucket views
move rebuild buckets from end of first iteration to beginning of second iteration #44326 move rebuild buckets from end of first iteration to beginning of second iteration

Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration

Differential Revision: D23583017

NOTE FOR REVIEWERS: This PR has internal Facebook specific changes or comments, please review them on Phabricator!

…nd iteration Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration Differential Revision: [D23583017](https://our.internmc.facebook.com/intern/diff/D23583017/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23583017/)! [ghstack-poisoned]

…nd iteration Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration Differential Revision: [D23583017](https://our.internmc.facebook.com/intern/diff/D23583017/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23583017/)! ghstack-source-id: 111611582 Pull Request resolved: #44326

dr-ci · 2020-09-08T20:57:19Z

💊 CI failures summary and remediations

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

1/1 failures introduced in this PR

🕵️ 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_ge_config_simple_test (1/1)

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

Sep 14 21:34:08 [E request_callback_no_python.cpp:618] Received error while processing request type 2: RuntimeError: Can not pickle torch.futures.Future

Sep 14 21:34:08 At: 
Sep 14 21:34:08   /opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/internal.py(93): serialize 
Sep 14 21:34:08   /opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/internal.py(145): serialize 
Sep 14 21:34:08  
Sep 14 21:34:08 [E request_callback_no_python.cpp:618] Received error while processing request type 2: RuntimeError: Can not pickle torch.futures.Future 
Sep 14 21:34:08  
Sep 14 21:34:08 At: 
Sep 14 21:34:08   /opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/internal.py(93): serialize 
Sep 14 21:34:08   /opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/internal.py(145): serialize 
Sep 14 21:34:08  
Sep 14 21:34:08 [E request_callback_no_python.cpp:618] Received error while processing request type 2: RuntimeError: Can not pickle torch.futures.Future 
Sep 14 21:34:08  
Sep 14 21:34:08 At: 
Sep 14 21:34:08   /opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/internal.py(93): serialize 
Sep 14 21:34:08   /opt/conda/lib/python3.6/site-packages/torch/distributed/rpc/internal.py(145): serialize 
Sep 14 21:34:08  
Sep 14 21:34:09 ok (1.540s) 
Sep 14 21:34:10   test_return_future_remote (__main__.ProcessGroupRpcTestWithSpawn) ... ok (1.539s) 
Sep 14 21:34:12   test_return_local_rrefs (__main__.ProcessGroupRpcTestWithSpawn) ... ok (1.540s) 
Sep 14 21:34:13   test_rpc_profiling_remote_record_function (__main__.ProcessGroupRpcTestWithSpawn) ... ok (1.542s) 
Sep 14 21:34:15   test_rpc_return_rref (__main__.ProcessGroupRpcTestWithSpawn) ... ok (1.543s)

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

…ing of second iteration" Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration Differential Revision: [D23583017](https://our.internmc.facebook.com/intern/diff/D23583017/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23583017/)! [ghstack-poisoned]

…nd iteration Pull Request resolved: #44326 Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration ghstack-source-id: 111630708 Differential Revision: [D23583017](https://our.internmc.facebook.com/intern/diff/D23583017/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23583017/)!

…ing of second iteration" Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration Differential Revision: [D23583017](https://our.internmc.facebook.com/intern/diff/D23583017/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23583017/)! [ghstack-poisoned]

…nd iteration Pull Request resolved: #44326 Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration ghstack-source-id: 111790710 Differential Revision: [D23583017](https://our.internmc.facebook.com/intern/diff/D23583017/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23583017/)!

codecov · 2020-09-10T22:55:42Z

Codecov Report

Merging #44326 into gh/zhaojuanmao/51/base will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@                    Coverage Diff                     @@
##           gh/zhaojuanmao/51/base   #44326      +/-   ##
==========================================================
+ Coverage                   68.00%   68.04%   +0.04%     
==========================================================
  Files                         384      382       -2     
  Lines                       49599    49468     -131     
==========================================================
- Hits                        33728    33661      -67     
+ Misses                      15871    15807      -64

Impacted Files	Coverage Δ
torch/nn/parallel/distributed.py	`43.17% <100.00%> (+2.03%)`	⬆️
torch/quantization/quantize_fx.py	`90.56% <0.00%> (-3.78%)`	⬇️
torch/utils/_benchmark/utils/timer.py	`85.18% <0.00%> (-3.71%)`	⬇️
torch/quantization/fuse_modules.py	`96.29% <0.00%> (-1.86%)`	⬇️
torch/quantization/__init__.py	`85.71% <0.00%> (-1.79%)`	⬇️
torch/_tensor_str.py	`86.07% <0.00%> (-1.69%)`	⬇️
torch/testing/_internal/expecttest.py	`77.55% <0.00%> (-1.03%)`	⬇️
torch/quantization/fx/quantize.py	`96.31% <0.00%> (-0.89%)`	⬇️
torch/quantization/fx/fusion_patterns.py	`87.20% <0.00%> (-0.89%)`	⬇️
torch/autograd/__init__.py	`84.84% <0.00%> (-0.87%)`	⬇️
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c68a99b...4ea2a11. Read the comment docs.

mrshenli

Would I be correct if assume the motivation is to rebuild buckets after the backward pass, to avoid increasing peak memory consumption?

mrshenli · 2020-09-14T02:20:14Z

torch/nn/parallel/distributed.py


    def forward(self, *inputs, **kwargs):
+        # Calling _rebuild_buckets before forward compuation,
+        # It may allocate new buckets before deallocating old buckets


It may allocate new buckets before deallocating old buckets

Why? at this point, looks like old buckets are no longer necessary. Is it possible to first delete old buckets and then allocate new buckets?

grad points to old buckets, we can not simply delete old buckets as users may not want to delete the grads

zhaojuanmao · 2020-09-14T16:55:23Z

yes, memory usage is low after backward pass and before forward computation, rebuildBuckets right before forward computation can save peak memory usage if we point grad to bucket_views later on

…ing of second iteration" Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration Differential Revision: [D23583017](https://our.internmc.facebook.com/intern/diff/D23583017/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23583017/)! [ghstack-poisoned]

…nd iteration Pull Request resolved: #44326 Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration ghstack-source-id: 112011490 Differential Revision: [D23583017](https://our.internmc.facebook.com/intern/diff/D23583017/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23583017/)!

facebook-github-bot · 2020-09-15T18:11:33Z

This pull request has been merged in f5d231d.

…nd iteration (#44326) Summary: Pull Request resolved: #44326 Part of relanding PR #41954, this refactoring is to move rebuild_buckets call from end of first iteration to beginning of second iteration ghstack-source-id: 112011490 Test Plan: unit tests Reviewed By: mrshenli Differential Revision: D23583017 fbshipit-source-id: ef67f79437a820d9b5699b651803622418499a83

zhaojuanmao requested review from apaszke, mrshenli and pietern as code owners September 8, 2020 20:43

zhaojuanmao mentioned this pull request Sep 8, 2020

refactor intialize bucket views #44330

Closed

zhaojuanmao mentioned this pull request Sep 8, 2020

[reland] Make grad point to bucket buffer in DDP to save memory usage #44344

Closed

mrshenli approved these changes Sep 14, 2020

View reviewed changes

facebook-github-bot closed this in f5d231d Sep 15, 2020

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

facebook-github-bot deleted the gh/zhaojuanmao/51/head branch September 19, 2020 14:23

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move rebuild buckets from end of first iteration to beginning of second iteration#44326

move rebuild buckets from end of first iteration to beginning of second iteration#44326
zhaojuanmao wants to merge 5 commits intogh/zhaojuanmao/51/basefrom
gh/zhaojuanmao/51/head

zhaojuanmao commented Sep 8, 2020 •

edited

Loading

Uh oh!

dr-ci bot commented Sep 8, 2020 •

edited

Loading

Uh oh!

codecov bot commented Sep 10, 2020 •

edited

Loading

Uh oh!

mrshenli left a comment

Uh oh!

mrshenli Sep 14, 2020

Uh oh!

zhaojuanmao Sep 14, 2020

Uh oh!

zhaojuanmao commented Sep 14, 2020

Uh oh!

facebook-github-bot commented Sep 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

zhaojuanmao commented Sep 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented Sep 8, 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_xenial_py3_6_gcc5_4_ge_config_simple_test (1/1)

Uh oh!

codecov bot commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mrshenli left a comment

Choose a reason for hiding this comment

Uh oh!

mrshenli Sep 14, 2020

Choose a reason for hiding this comment

Uh oh!

zhaojuanmao Sep 14, 2020

Choose a reason for hiding this comment

Uh oh!

zhaojuanmao commented Sep 14, 2020

Uh oh!

facebook-github-bot commented Sep 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zhaojuanmao commented Sep 8, 2020 •

edited

Loading

dr-ci bot commented Sep 8, 2020 •

edited

Loading

codecov bot commented Sep 10, 2020 •

edited

Loading