[reland] move rebuild buckets from end of first iteration to beginning of second iteration #44798

zhaojuanmao · 2020-09-16T17:27:57Z

Stack from ghstack:

[reland] move rebuild buckets from end of first iteration to beginning of second iteration #44798 [reland] move rebuild buckets from end of first iteration to beginning of second iteration
refactor intialize bucket views #44330 refactor intialize bucket views

[test all]

Update for relanding: in ddp.join(), moved _rebuild_buckets from end of backward to beginning of forward as well.

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: D23735185

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

…g of second iteration [test all] Update for relanding: in ddp.join(), moved _rebuild_buckets from end of backward to beginning of forward as well. 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: [D23735185](https://our.internmc.facebook.com/intern/diff/D23735185/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23735185/)! [ghstack-poisoned]

…to beginning of second iteration" [test all] Update for relanding: in ddp.join(), moved _rebuild_buckets from end of backward to beginning of forward as well. 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: [D23735185](https://our.internmc.facebook.com/intern/diff/D23735185/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23735185/)! [ghstack-poisoned]

dr-ci · 2020-09-16T17:48:55Z

💊 CI failures summary and remediations

As of commit 333a3c8 (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_cuda10_2_cudnn7_py3_ge_config_legacy_test (1/1)

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

Sep 17 15:13:26 ERROR [5.779s]: test_tensorexpr_cpp_cuda (test_jit.TestJit)

Sep 17 15:13:25   test_factory_ops_requires_grad_fail (jit.test_unsupported_ops.TestUnsupportedOps) ... ok (0.004s) 
Sep 17 15:13:25   test_init_ops (jit.test_unsupported_ops.TestUnsupportedOps) ... ok (0.021s) 
Sep 17 15:13:25   test_ops_bound_in_functional (jit.test_unsupported_ops.TestUnsupportedOps) ... ok (0.003s) 
Sep 17 15:13:26   test_with_as (jit.test_with.TestWith) ... ok (0.084s) 
Sep 17 15:13:26   test_with_errors (jit.test_with.TestWith) ... ok (0.016s) 
Sep 17 15:13:26   test_with_exceptions (jit.test_with.TestWith) ... ok (0.006s) 
Sep 17 15:13:26   test_with_no_as (jit.test_with.TestWith) ... ok (0.085s) 
Sep 17 15:13:26   test_with_no_grad (jit.test_with.TestWith) ... ok (0.020s) 
Sep 17 15:13:26  
Sep 17 15:13:26 ====================================================================== 
Sep 17 15:13:26 ERROR [5.779s]: test_tensorexpr_cpp_cuda (test_jit.TestJit) 
Sep 17 15:13:26 ---------------------------------------------------------------------- 
Sep 17 15:13:26 Traceback (most recent call last): 
Sep 17 15:13:26   File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_utils.py", line 818, in wrapper 
Sep 17 15:13:26     method(*args, **kwargs) 
Sep 17 15:13:26   File "/var/lib/jenkins/workspace/test/test_jit.py", line 1431, in test_tensorexpr_cpp_cuda 
Sep 17 15:13:26     torch._C._run_tensorexpr_cpp_tests(run_cuda=True) 
Sep 17 15:13:26 RuntimeError: std::fabs((v1[kPaddingSize + i]) - (v2[kPaddingSize + i])) < (abs_error) INTERNAL ASSERT FAILED at "/var/lib/jenkins/workspace/test/cpp/tensorexpr/padded_buffer.h":258, please report a bug to PyTorch. index: 0, v1: (b_v, 688448), v2: (b_ref, 778240) 
Sep 17 15:13:26  
Sep 17 15:13:26 ---------------------------------------------------------------------- 
Sep 17 15:13:26 Ran 2716 tests in 270.304s

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

…to beginning of second iteration" [test all] Update for relanding: in ddp.join(), moved _rebuild_buckets from end of backward to beginning of forward as well. 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: [D23735185](https://our.internmc.facebook.com/intern/diff/D23735185/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23735185/)! [ghstack-poisoned]

…g of second iteration Pull Request resolved: #44798 [test all] Update for relanding: in ddp.join(), moved _rebuild_buckets from end of backward to beginning of forward as well. 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: 112244222 ghstack-source-id: 112244222 Differential Revision: [D23735185](https://our.internmc.facebook.com/intern/diff/D23735185/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23735185/)!

…to beginning of second iteration" [test all] Update for relanding: in ddp.join(), moved _rebuild_buckets from end of backward to beginning of forward as well. 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: [D23735185](https://our.internmc.facebook.com/intern/diff/D23735185/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23735185/)! [ghstack-poisoned]

…g of second iteration Pull Request resolved: #44798 [test all] Update for relanding: in ddp.join(), moved _rebuild_buckets from end of backward to beginning of forward as well. 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: 112279261 ghstack-source-id: 112279261 Differential Revision: [D23735185](https://our.internmc.facebook.com/intern/diff/D23735185/) **NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D23735185/)!

zhaojuanmao · 2020-09-17T21:43:10Z

failures are not related, ci-all tests also passed in #44893

rohan-varma

LGTM, the changes in join() will ensure we don't run into desync issues as discussed.

mrshenli · 2020-09-17T23:46:46Z

torch/nn/parallel/distributed.py

@@ -583,6 +584,15 @@ def forward(self, *inputs, **kwargs):
                work, ones, self.ddp_join_divide_by_initial_world_size
            )

+        # Calling _rebuild_buckets before forward compuation,


compuation -> computation

facebook-github-bot · 2020-09-18T00:12:55Z

This pull request has been merged in e14b208.

…g of second iteration (#44798) Summary: Pull Request resolved: #44798 [test all] Update for relanding: in ddp.join(), moved _rebuild_buckets from end of backward to beginning of forward as well. 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: 112279261 ghstack-source-id: 112279261 Test Plan: unit tests Reviewed By: rohan-varma Differential Revision: D23735185 fbshipit-source-id: c26e0efeecb3511640120faa1122a2c856cd694e

…_buckets fails Occasionally users run DDP with models with unused params, in this case we would like to surface an error message telling them to run with find_unused_params=True. However, a recent change to rebuild_buckets logic (#44798) made it so that we raise a size mismatch error when this happens, but the information about unused parameters is likely to be more useful and likely to be the most common case of failure. Prefer raising this error over the subsequent size mismatch errors. Differential Revision: [D24151256](https://our.internmc.facebook.com/intern/diff/D24151256/) [ghstack-poisoned]

…hen rebuild_buckets fails" Occasionally users run DDP with models with unused params, in this case we would like to surface an error message telling them to run with find_unused_params=True. However, a recent change to rebuild_buckets logic (#44798) made it so that we raise a size mismatch error when this happens, but the information about unused parameters is likely to be more useful and likely to be the most common case of failure. Prefer raising this error over the subsequent size mismatch errors. Differential Revision: [D24151256](https://our.internmc.facebook.com/intern/diff/D24151256/) [ghstack-poisoned]

…_buckets fails Pull Request resolved: #45933 Occasionally users run DDP with models with unused params, in this case we would like to surface an error message telling them to run with find_unused_params=True. However, a recent change to rebuild_buckets logic (#44798) made it so that we raise a size mismatch error when this happens, but the information about unused parameters is likely to be more useful and likely to be the most common case of failure. Prefer raising this error over the subsequent size mismatch errors. ghstack-source-id: 113713050 Differential Revision: [D24151256](https://our.internmc.facebook.com/intern/diff/D24151256/)

…hen rebuild_buckets fails" Occasionally users run DDP with models with unused params, in this case we would like to surface an error message telling them to run with find_unused_params=True. However, a recent change to rebuild_buckets logic (#44798) made it so that we raise a size mismatch error when this happens, but the information about unused parameters is likely to be more useful and likely to be the most common case of failure. Prefer raising this error over the subsequent size mismatch errors. Differential Revision: [D24151256](https://our.internmc.facebook.com/intern/diff/D24151256/) [ghstack-poisoned]

…_buckets fails (#45933) Summary: Pull Request resolved: #45933 Occasionally users run DDP with models with unused params, in this case we would like to surface an error message telling them to run with find_unused_params=True. However, a recent change to rebuild_buckets logic (#44798) made it so that we raise a size mismatch error when this happens, but the information about unused parameters is likely to be more useful and likely to be the most common case of failure. Prefer raising this error over the subsequent size mismatch errors. ghstack-source-id: 113914759 Test Plan: Added unittest Reviewed By: mrshenli Differential Revision: D24151256 fbshipit-source-id: 5d349a988b4aac7d3e0ef7b3cd84dfdcbe9db675

zhaojuanmao requested review from apaszke, mrshenli and pietern as code owners September 16, 2020 17:27

zhaojuanmao mentioned this pull request Sep 16, 2020

refactor intialize bucket views #44330

Closed

zhaojuanmao mentioned this pull request Sep 16, 2020

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

Closed

zhaojuanmao requested review from pritamdamania87 and rohan-varma as code owners September 16, 2020 23:32

zhaojuanmao mentioned this pull request Sep 17, 2020

[reland ci-all tests] move rebuild buckets from end of first iteration to beginning of second iteration #44893

Closed

rohan-varma approved these changes Sep 17, 2020

View reviewed changes

mrshenli approved these changes Sep 17, 2020

View reviewed changes

facebook-github-bot closed this in e14b208 Sep 18, 2020

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

facebook-github-bot deleted the gh/zhaojuanmao/54/head branch September 21, 2020 14:14

rohan-varma mentioned this pull request Oct 6, 2020

Prioritize raising error message about unused parameters when rebuild_buckets fails #45933

Closed

This was referenced Oct 9, 2020

[v1.7 patch] Prioritize raising error message about unused parameters when rebuild_buckets fails #46099

Merged

[v1.7.0] Release Tracker #45592

Closed

mruberry added the Merged label Oct 28, 2020

pritamdamania87 mentioned this pull request Nov 4, 2020

PyTorch bug:rebuilt parameter indices size is not same as original model parameters size.156 versus 22776 #47050

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[reland] move rebuild buckets from end of first iteration to beginning of second iteration #44798

[reland] move rebuild buckets from end of first iteration to beginning of second iteration #44798

zhaojuanmao commented Sep 16, 2020 •

edited

dr-ci bot commented Sep 16, 2020 •

edited

zhaojuanmao commented Sep 17, 2020

rohan-varma left a comment

mrshenli Sep 17, 2020

facebook-github-bot commented Sep 18, 2020

[reland] move rebuild buckets from end of first iteration to beginning of second iteration #44798

[reland] move rebuild buckets from end of first iteration to beginning of second iteration #44798

Conversation

zhaojuanmao commented Sep 16, 2020 • edited

dr-ci bot commented Sep 16, 2020 • edited

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_linux_xenial_cuda10_2_cudnn7_py3_ge_config_legacy_test (1/1)

zhaojuanmao commented Sep 17, 2020

rohan-varma left a comment

Choose a reason for hiding this comment

mrshenli Sep 17, 2020

Choose a reason for hiding this comment

facebook-github-bot commented Sep 18, 2020

zhaojuanmao commented Sep 16, 2020 •

edited

dr-ci bot commented Sep 16, 2020 •

edited