[c10d] Make allgather as a custom op by alanwaketan · Pull Request #79669 · pytorch/pytorch

alanwaketan · 2022-06-16T01:47:42Z

Stack from ghstack (oldest at bottom):

Summary:
This patch makes allgather as a custom op such that it's dispatcher
passable. It's one part of the effort to route comm ops to the dispatcher
such that tracing mechanisms that relies on the dispatcher can trace them,
e.g., LazyTensor and AOTAutograd.

Test Plan:
python test/distributed/test_c10d_nccl.py -k test_allgather_ops
python test/distributed/test_c10d_gloo.py -k test_allgather_basics
...and other existing distributed tests.

Summary: This patch makes allgather as a custom op such that it's dispatcher passable. It's one part of the effort to route comm ops to the dispatcher such that tracing mechanisms that relies on the dispatcher can trace them, e.g., LazyTensor and AOTAutograd. Test Plan: python test/distributed/test_c10d_nccl.py -k test_allgather_ops python test/distributed/test_c10d_gloo.py -k test_allgather_basics ...and other existing distributed tests. [ghstack-poisoned]

facebook-github-bot · 2022-06-16T01:47:49Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/79669
📄 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 our office hours
↩️ [fb-only] Re-run with SSH instructions

❌ 1 New Failures

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

Expand to see more

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

pull / linux-bionic-py3_7-clang8-xla / test (xla, 1, 1, linux.2xlarge) (1/1)

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

2022-06-22T22:58:19.4167431Z ##[error]Process completed with exit code 1.

2022-06-22T22:58:19.1795558Z �[1A�[K�[32mINFO: �[0mElapsed time: 164.408s
2022-06-22T22:58:19.1796044Z �[32mLoading:�[0m 0 packages loaded
2022-06-22T22:58:19.1849924Z 
2022-06-22T22:58:19.1850585Z �[1A�[K�[32mINFO: �[0m0 processes.
2022-06-22T22:58:19.1851064Z �[32mLoading:�[0m 0 packages loaded
2022-06-22T22:58:19.1851205Z 
2022-06-22T22:58:19.1851461Z �[1A�[K�[31m�[1mFAILED:�[0m Build did NOT complete successfully (0 packages loaded)
2022-06-22T22:58:19.1851647Z 
2022-06-22T22:58:19.1851857Z �[1A�[K�[31m�[1mFAILED:�[0m Build did NOT complete successfully (0 packages loaded)
2022-06-22T22:58:19.1942873Z �[0mFailed to build external libraries: ['/var/lib/jenkins/workspace/xla/build_torch_xla_libs.sh', '-O', '-D_GLIBCXX_USE_CXX11_ABI=1', 'install']
2022-06-22T22:58:19.4167431Z ##[error]Process completed with exit code 1.
2022-06-22T22:58:19.4211607Z Prepare all required actions
2022-06-22T22:58:19.4211957Z Getting action download info
2022-06-22T22:58:19.6084529Z ##[group]Run ./.github/actions/get-workflow-job-id
2022-06-22T22:58:19.6084756Z with:
2022-06-22T22:58:19.6085182Z   github-token: ***
2022-06-22T22:58:19.6085338Z env:
2022-06-22T22:58:19.6085511Z   GIT_DEFAULT_BRANCH: master
2022-06-22T22:58:19.6085694Z ##[endgroup]
2022-06-22T22:58:19.6111722Z ##[group]Run nick-fields/retry@71062288b76e2b6214ebde0e673ce0de1755740a
2022-06-22T22:58:19.6111958Z with:

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

Summary: This patch makes allgather as a custom op such that it's dispatcher passable. It's one part of the effort to route comm ops to the dispatcher such that tracing mechanisms that relies on the dispatcher can trace them, e.g., LazyTensor and AOTAutograd. Test Plan: python test/distributed/test_c10d_nccl.py -k test_allgather_ops python test/distributed/test_c10d_gloo.py -k test_allgather_basics ...and other existing distributed tests. [ghstack-poisoned]

Summary: This patch makes allgather as a custom op such that it's dispatcher passable. It's one part of the effort to route comm ops to the dispatcher such that tracing mechanisms that relies on the dispatcher can trace them, e.g., LazyTensor and AOTAutograd. Test Plan: python test/distributed/test_c10d_nccl.py -k test_allgather_ops python test/distributed/test_c10d_gloo.py -k test_allgather_basics ...and other existing distributed tests. ghstack-source-id: 98f6ccb Pull Request resolved: #79669

Summary: This patch makes allgather as a custom op such that it's dispatcher passable. It's one part of the effort to route comm ops to the dispatcher such that tracing mechanisms that relies on the dispatcher can trace them, e.g., LazyTensor and AOTAutograd. Test Plan: python test/distributed/test_c10d_nccl.py -k test_allgather_ops python test/distributed/test_c10d_gloo.py -k test_allgather_basics ...and other existing distributed tests. [ghstack-poisoned]

Summary: This patch makes allgather as a custom op such that it's dispatcher passable. It's one part of the effort to route comm ops to the dispatcher such that tracing mechanisms that relies on the dispatcher can trace them, e.g., LazyTensor and AOTAutograd. Test Plan: python test/distributed/test_c10d_nccl.py -k test_allgather_ops python test/distributed/test_c10d_gloo.py -k test_allgather_basics ...and other existing distributed tests. ghstack-source-id: 8676b05 Pull Request resolved: #79669

mrshenli

LGTM! I'd vote for let's some tests to cover both forward and backward (i.e., torch.distributed.nn.*). It does not have to be in this PR, can be a PR on top. Just wanna verify we didn't miss anything. :)

mrshenli · 2022-06-24T14:34:24Z

torch/csrc/distributed/c10d/Ops.cpp

+    const c10::intrusive_ptr<ProcessGroup>& process_group,
+    int64_t timeout) {
+  return process_group->allgather(
+      const_cast<std::vector<std::vector<at::Tensor>>&>(output_tensors),


We should be able to modify allgather to take const std::vector as well if necessary. Shall we create an issue to track that?

Yea, I agreed. PG's should accept const vectors& instead of vectors& given what we do is to update the content of the values within the vectors and not the vectors itself. Let me create an issue. #80241.

alanwaketan · 2022-06-24T20:28:00Z

Thanks Shen for approving this pull request. The CI failure doesn't seem to be related.

alanwaketan · 2022-06-24T20:28:13Z

@pytorchbot merge -f

pytorchmergebot · 2022-06-24T20:29:27Z

@pytorchbot successfully started a merge job. Check the current status here

github-actions · 2022-06-24T20:30:08Z

Hey @alanwaketan.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

alanwaketan · 2022-06-24T20:35:06Z

LGTM! I'd vote for let's some tests to cover both forward and backward (i.e., torch.distributed.nn.*). It does not have to be in this PR, can be a PR on top. Just wanna verify we didn't miss anything. :)

Sounds good. Opened #80242 to keep track of this.

Summary: This patch makes allgather as a custom op such that it's dispatcher passable. It's one part of the effort to route comm ops to the dispatcher such that tracing mechanisms that relies on the dispatcher can trace them, e.g., LazyTensor and AOTAutograd. Pull Request resolved: #79669 Approved by: https://github.com/mrshenli Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/3359af93909bb6009659e01e81eabb9c8dbb4b3a Test plan from GitHub: python test/distributed/test_c10d_nccl.py -k test_allgather_ops python test/distributed/test_c10d_gloo.py -k test_allgather_basics ...and other existing distributed tests. Reviewed By: atalman Differential Revision: D37455652 Pulled By: alanwaketan fbshipit-source-id: 058691c6f5a59807b585a22a93353e91313f8b6f

alanwaketan requested review from H-Huang, awgu, mingzhe09088, mrshenli, pritamdamania87, rohan-varma and zhaojuanmao as code owners June 16, 2022 01:47

alanwaketan mentioned this pull request Jun 16, 2022

[c10d] Make allreduce as a custom op #79582

Closed

facebook-github-bot added the cla signed label Jun 16, 2022

facebook-github-bot added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Jun 16, 2022

alanwaketan requested review from aazzolini, ezyang and wconstab and removed request for H-Huang, awgu, mingzhe09088, mrshenli, rohan-varma and zhaojuanmao June 16, 2022 01:50

This was referenced Jun 16, 2022

[c10d] Make reduce_scatter as a custom op #79683

Closed

[c10d] Make reduce as a custom op #79686

Closed

[c10d] Make gather as a custom op #79687

Closed

[c10d] Make scatter as a custom op #79688

Closed

[c10d] Make alltoall as a custom op #79691

Closed

alanwaketan requested a review from mrshenli June 16, 2022 20:56

This was referenced Jun 17, 2022

[c10d] Make barrier as a custom op #79777

Closed

[c10d] Make send/recv as custom ops #79779

Closed

alanwaketan requested a review from wanchaol June 22, 2022 22:34

mrshenli approved these changes Jun 24, 2022

View reviewed changes

alanwaketan mentioned this pull request Jun 24, 2022

Change c10d APIs in ProcessGroup to accept const std::vector<at::Tensor>& #80241

Open

pytorchmergebot added the Merged label Jun 24, 2022

pytorchmergebot closed this in 3359af9 Jun 24, 2022

alanwaketan added release notes: distributed (c10d) release notes category topic: new features topic category labels Jun 24, 2022

alanwaketan mentioned this pull request Jun 24, 2022

Write some torch.distributed.nn.* tests for the new dispatcher passable ops #80242

Open

facebook-github-bot deleted the gh/alanwaketan/35/head branch June 28, 2022 14:17

crcrpar mentioned this pull request Aug 4, 2022

all_gather supporting single tensor output #82639

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[c10d] Make allgather as a custom op#79669

[c10d] Make allgather as a custom op#79669
alanwaketan wants to merge 3 commits intogh/alanwaketan/35/basefrom
gh/alanwaketan/35/head

alanwaketan commented Jun 16, 2022 •

edited

Loading

Uh oh!

facebook-github-bot commented Jun 16, 2022 •

edited

Loading

🕵️ 1 new failure recognized by patterns

pull / linux-bionic-py3_7-clang8-xla / test (xla, 1, 1, linux.2xlarge) (1/1)

Uh oh!

mrshenli left a comment

Uh oh!

mrshenli Jun 24, 2022

Uh oh!

alanwaketan Jun 24, 2022

Uh oh!

alanwaketan commented Jun 24, 2022

Uh oh!

alanwaketan commented Jun 24, 2022

Uh oh!

pytorchmergebot commented Jun 24, 2022

Uh oh!

github-actions bot commented Jun 24, 2022

Uh oh!

alanwaketan commented Jun 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

alanwaketan commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Jun 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

❌ 1 New Failures

🕵️ 1 new failure recognized by patterns

pull / linux-bionic-py3_7-clang8-xla / test (xla, 1, 1, linux.2xlarge) (1/1)

Uh oh!

mrshenli left a comment

Choose a reason for hiding this comment

Uh oh!

mrshenli Jun 24, 2022

Choose a reason for hiding this comment

Uh oh!

alanwaketan Jun 24, 2022

Choose a reason for hiding this comment

Uh oh!

alanwaketan commented Jun 24, 2022

Uh oh!

alanwaketan commented Jun 24, 2022

Uh oh!

pytorchmergebot commented Jun 24, 2022

Uh oh!

github-actions bot commented Jun 24, 2022

Uh oh!

alanwaketan commented Jun 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alanwaketan commented Jun 16, 2022 •

edited

Loading

facebook-github-bot commented Jun 16, 2022 •

edited

Loading