[DDP] multiple forward support for static graph #103487

rohan-varma · 2023-06-13T04:03:00Z

Stack from ghstack (oldest at bottom):

Adds support for multiple forward before bwd call for
static_graph=True.

There are 2 changes:

Change tracking of accounting of when to populate static grap related maps
from relying on forward iteration to backward calls
In DDP python, don't rely on num_forward iterations == 1 to enqueue the
delay allreduce. Instead use a flag.

Differential Revision: D46673736

Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/) [ghstack-poisoned]

pytorch-bot · 2023-06-13T04:03:04Z

🔗 Helpful Links

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

📄 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.

❌ 4 New Failures, 2 Unrelated Failures

As of commit 1edd7d2:

NEW FAILURES - The following jobs have failed:

linux-bionic-cuda12.1-py3.10-gcc9-sm86 / test (default, 2, 5, linux.g5.4xlarge.nvidia.gpu) (gh)
linux-bionic-cuda12.1-py3.10-gcc9-sm86 / test (default, 5, 5, linux.g5.4xlarge.nvidia.gpu) (gh)
linux-bionic-py3.11-clang9 / test (default, 1, 3, linux.2xlarge) (gh)
linux-bionic-py3.8-clang9 / test (default, 2, 3, linux.2xlarge) (gh)

BROKEN TRUNK - The following jobs failed but were present on the merge base 3f6f508:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/) ghstack-source-id: 191995486 Pull Request resolved: #103487

Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/) [ghstack-poisoned]

Pull Request resolved: #103487 Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. ghstack-source-id: 192041100 Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/)

Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/) [ghstack-poisoned]

Pull Request resolved: #103487 Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. ghstack-source-id: 192115313 Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/)

awgu

This makes sense to me!

torch/nn/parallel/distributed.py

torch/csrc/distributed/c10d/reducer.cpp

torch/testing/_internal/distributed/distributed_test.py

Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/) [ghstack-poisoned]

Pull Request resolved: #103487 Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. ghstack-source-id: 192138853 Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/)

Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/) [ghstack-poisoned]

Pull Request resolved: #103487 Adds support for multiple forward before bwd call for static_graph=True. There are 2 changes: 1) Change tracking of accounting of when to populate static grap related maps from relying on forward iteration to backward calls 2) In DDP python, don't rely on num_forward iterations == 1 to enqueue the delay allreduce. Instead use a flag. ghstack-source-id: 192139275 Differential Revision: [D46673736](https://our.internmc.facebook.com/intern/diff/D46673736/)

rohan-varma · 2023-06-14T16:12:23Z

@pytorchbot merge -f "CI failure unrelated"

pytorchmergebot · 2023-06-14T16:14:42Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

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

osalpekar · 2023-06-14T18:19:22Z

@pytorchbot revert -m "This causes distributed/test_c10d_gloo.py::DistributedDataParallelTest::test_ddp_checkpointing_weight_sharing_use_reentrant_False to fail on trunk starting this commit. See https://github.com/pytorch/pytorch/actions/runs/5269586345/jobs/9528524605 for more details" -c landrace

pytorchmergebot · 2023-06-14T18:21:19Z

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot · 2023-06-14T18:21:22Z

Can't revert PR that was landed via phabricator as D46673736. Please revert by going to the internal diff and clicking Unland.

…)" This reverts commit 80139fc.

…103873) Per the discussion in #103629 (comment), I preemptively create this revert PR to revert all commits in the stack. This seems like a safer option than using the bot as the commit has already been in trunk since last week. Pull Request resolved: #103873 Approved by: https://github.com/rohan-varma

…03487)" (#103873)" Differential Revision: [D46883396](https://our.internmc.facebook.com/intern/diff/D46883396/) [ghstack-poisoned]

…03487)" (#103873)" Differential Revision: [D46883396](https://our.internmc.facebook.com/intern/diff/D46883396/) ghstack-source-id: 192716301 Pull Request resolved: #103938

…03487)" (#103873)" (#103938) Differential Revision: [D46883396](https://our.internmc.facebook.com/intern/diff/D46883396/) Pull Request resolved: #103938 Approved by: https://github.com/awgu, https://github.com/fegin

rohan-varma · 2023-11-27T20:01:21Z

This also fixes torch.compile w/staticgraph=True

Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/) [ghstack-poisoned]

Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/) ghstack-source-id: 208386778 Pull Request resolved: #114621

…_graph=True" Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/) cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab mrshenli zhaojuanmao kiukchung d4l3k lucasllc XilunWu tianyu-l [ghstack-poisoned]

Pull Request resolved: #114621 Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. ghstack-source-id: 208389966 @exported-using-ghexport Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/)

…_graph=True" Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/) cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab mrshenli zhaojuanmao kiukchung d4l3k lucasllc XilunWu tianyu-l [ghstack-poisoned]

Pull Request resolved: #114621 Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. ghstack-source-id: 208392149 @exported-using-ghexport Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/)

Pull Request resolved: #114621 Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. ghstack-source-id: 208875875 @exported-using-ghexport Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/)

…e works w/static_graph=True" Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/) cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab mrshenli zhaojuanmao kiukchung d4l3k lucasllc XilunWu tianyu-l [ghstack-poisoned]

…_graph=True" Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/) cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab mrshenli zhaojuanmao kiukchung d4l3k lucasllc XilunWu tianyu-l [ghstack-poisoned]

…#114621) Resolves #93672. This was actually fixed by #103487 but I didn't realize that PR also fixes torch compile at the time. Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/) Pull Request resolved: #114621 Approved by: https://github.com/wconstab

…pytorch#114621) Resolves pytorch#93672. This was actually fixed by pytorch#103487 but I didn't realize that PR also fixes torch compile at the time. Differential Revision: [D51596148](https://our.internmc.facebook.com/intern/diff/D51596148/) Pull Request resolved: pytorch#114621 Approved by: https://github.com/wconstab

rohan-varma requested review from mrshenli, zhaojuanmao, H-Huang, awgu, kwen2501, wanchaol, fegin, fduwjj, kiukchung and d4l3k as code owners June 13, 2023 04:03

pytorch-bot bot added the release notes: distributed (c10d) release notes category label Jun 13, 2023

This was referenced Jun 13, 2023

[DDP] Rename state_dict var to ddp_state #103282

Closed

[DDP] Rename num_iterations -> num_forward_calls #103283

Closed

[DDP] Refactor _DDPSink to take DDP weakref #103304

Closed

awgu approved these changes Jun 14, 2023

View reviewed changes

pytorchmergebot added the merging label Jun 14, 2023

pytorchmergebot added the Merged label Jun 14, 2023

pytorchmergebot closed this in 80139fc Jun 14, 2023

rohan-varma mentioned this pull request Jun 14, 2023

[DDP] Fix test #103629

Closed

facebook-github-bot deleted the gh/rohan-varma/707/head branch June 18, 2023 14:17

huydhn mentioned this pull request Jun 19, 2023

DISABLED test_ddp_checkpointing_weight_sharing_use_reentrant_False (__main__.DistributedDataParallelTest) #103855

Closed

rohan-varma mentioned this pull request Jun 20, 2023

DISABLED test_ddp_checkpointing_weight_sharing_use_reentrant_False (__main__.TestDDPWithWorkSubclass) #103872

Closed

huydhn added a commit to huydhn/pytorch that referenced this pull request Jun 20, 2023

Revert "[DDP] multiple forward support for static graph (pytorch#103487…

bbca36a

…)" This reverts commit 80139fc.

rohan-varma mentioned this pull request Jun 20, 2023

DISABLED test_ddp_checkpointing_weight_sharing_use_reentrant_False (__main__.TestDDPWithWorkWrapper) #103876

Closed

rohan-varma added a commit that referenced this pull request Jun 21, 2023

Back out "Revert "[DDP] multiple forward support for static graph (#1…

c074fbc

…03487)" (#103873)" Differential Revision: [D46883396](https://our.internmc.facebook.com/intern/diff/D46883396/) [ghstack-poisoned]

ringohoffman mentioned this pull request Aug 13, 2023

Multiple backward on multiple tensors returned from DDP with static_graph=True raises autograd assertion error #106953

Closed

rohan-varma mentioned this pull request Nov 27, 2023

[DDP][Compile] Test to Ensure torch.compile works w/static_graph=True #114621

Closed

rohan-varma mentioned this pull request Nov 27, 2023

[ddp] must set static_graph=False when running with dynamo #93672

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DDP] multiple forward support for static graph #103487

[DDP] multiple forward support for static graph #103487

rohan-varma commented Jun 13, 2023 •

edited

pytorch-bot bot commented Jun 13, 2023 •

edited

awgu left a comment

rohan-varma commented Jun 14, 2023

pytorchmergebot commented Jun 14, 2023

osalpekar commented Jun 14, 2023

pytorchmergebot commented Jun 14, 2023

pytorchmergebot commented Jun 14, 2023

rohan-varma commented Nov 27, 2023

[DDP] multiple forward support for static graph #103487

[DDP] multiple forward support for static graph #103487

Conversation

rohan-varma commented Jun 13, 2023 • edited

pytorch-bot bot commented Jun 13, 2023 • edited

🔗 Helpful Links

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

❌ 4 New Failures, 2 Unrelated Failures

awgu left a comment

Choose a reason for hiding this comment

rohan-varma commented Jun 14, 2023

pytorchmergebot commented Jun 14, 2023

Merge started

osalpekar commented Jun 14, 2023

pytorchmergebot commented Jun 14, 2023

pytorchmergebot commented Jun 14, 2023

rohan-varma commented Nov 27, 2023

rohan-varma commented Jun 13, 2023 •

edited

pytorch-bot bot commented Jun 13, 2023 •

edited