[c10d] Make send/recv as custom ops by alanwaketan · Pull Request #79779 · pytorch/pytorch

alanwaketan · 2022-06-17T08:25:39Z

Stack from ghstack (oldest at bottom):

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

Summary:
This patch makes send/recv as custom ops 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_send_recv
...and other existing distributed tests.

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. [ghstack-poisoned]

facebook-github-bot · 2022-06-17T08:25:47Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/79779
📄 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

✅ No Failures (0 Pending)

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

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚

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 send/recv as custom ops 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_send_recv ...and other existing distributed tests. ghstack-source-id: d0536fb Pull Request resolved: #79779

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. [ghstack-poisoned]

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. ghstack-source-id: c627df1 Pull Request resolved: #79779

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. [ghstack-poisoned]

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. ghstack-source-id: b5cad84 Pull Request resolved: #79779

mrshenli · 2022-06-24T15:05:28Z

torch/csrc/distributed/c10d/Ops.cpp

 }

+c10::intrusive_ptr<ProcessGroup::Work> send(
+    at::TensorList tensors,


same comment on TensorList vs std::vector<Tensor>

I think it's a convention to use TensorList to represent const std::vectorat::Tensor&.

I only use const std::vectorat::Tensor& if there is a const std::vectorstd::vectorat::Tensor& in the signature to keep consistency on that function. Let me know which way you like.

mrshenli · 2022-06-24T15:05:38Z

torch/csrc/distributed/c10d/Ops.cpp

      BarrierOptions{device_ids, std::chrono::milliseconds(timeout)});
 }

+c10::intrusive_ptr<ProcessGroup::Work> send(


trailing underscore?

Send is not a in-place op. So there is no needs to have a trailing _.

wanchaol

lgtm, have some questions.

wanchaol · 2022-06-25T00:12:42Z

torch/csrc/distributed/c10d/Ops.cpp

+    int64_t dstRank,
+    int64_t tag) {
+  static auto op = c10::Dispatcher::singleton()
+                       .findSchemaOrThrow("c10d::send", "")


hmm would we not find the schema by any chance?

Why would you have such concern? Do you happen to know another op called "c10d::send" as well?

wanchaol · 2022-06-25T00:14:08Z

torch/csrc/distributed/c10d/Ops.cpp

+  return op.call(tensors, process_group, dstRank, tag);
+}
+
+c10::intrusive_ptr<ProcessGroup::Work> recv(


just noticed we have this recv and recv_ two separate APIs (above), what's the difference between these two and should we merge?

The above recv_ is the implementation of the function schema. This one is the C++ API for users to call the op. recv_ follows aten ops's conventions while recv follows c10d's conventions.

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. [ghstack-poisoned]

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. ghstack-source-id: 07f0d10 Pull Request resolved: #79779

alanwaketan · 2022-06-27T08:15:46Z

@pytorchbot merge --green

alanwaketan · 2022-06-27T08:16:23Z

Thanks for approving this pull request, Shen and Wanchao.

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. [ghstack-poisoned]

Summary: This patch makes send/recv as custom ops 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_send_recv ...and other existing distributed tests. ghstack-source-id: b1c39c2 Pull Request resolved: #79779

alanwaketan · 2022-06-27T17:44:37Z

@pytorchbot merge --green

pytorchmergebot · 2022-06-27T17:45:47Z

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

github-actions · 2022-06-27T22:08:01Z

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.

Summary: This patch makes send/recv as custom ops 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: #79779 Approved by: https://github.com/mrshenli, https://github.com/wanchaol Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/79c2dfcd8ef3ab2ca14a942109242ae7440dd6b1 Test plan from GitHub: python test/distributed/test_c10d_nccl.py -k test_send_recv ...and other existing distributed tests. Reviewed By: b0noI Differential Revision: D37509682 Pulled By: alanwaketan fbshipit-source-id: a3a698f85b64b256618b8af95dd29aade4b50d74

alanwaketan requested review from H-Huang, awgu, mingzhe09088, mrshenli, pritamdamania87, rohan-varma and zhaojuanmao as code owners June 17, 2022 08:25

This was referenced Jun 17, 2022

[c10d] Make allreduce as a custom op #79582

Closed

[c10d] Make allgather as a custom op #79669

Closed

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

This was referenced Jun 17, 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

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

alanwaketan mentioned this pull request Jun 17, 2022

[c10d] Make scatter as a custom op #79688

Closed

This was referenced Jun 17, 2022

[c10d] Make alltoall as a custom op #79691

Closed

[c10d] Make barrier as a custom op #79777

Closed

alanwaketan removed request for H-Huang, awgu, mingzhe09088, rohan-varma and zhaojuanmao June 17, 2022 08:26

alanwaketan requested a review from wanchaol June 22, 2022 23:45

alanwaketan requested review from aazzolini, ezyang and wconstab June 22, 2022 23:45

mrshenli added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 24, 2022

mrshenli reviewed Jun 24, 2022

View reviewed changes

mrshenli approved these changes Jun 25, 2022

View reviewed changes

wanchaol approved these changes Jun 25, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

pytorchmergebot added the Merged label Jun 27, 2022

pytorchmergebot closed this in 79c2dfc Jun 27, 2022

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

facebook-github-bot deleted the gh/alanwaketan/42/head branch July 1, 2022 14:18

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

🔗 Helpful links

✅ No Failures (0 Pending)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wanchaol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanwaketan commented Jun 27, 2022

Uh oh!

alanwaketan commented Jun 27, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

alanwaketan commented Jun 27, 2022

Uh oh!

pytorchmergebot commented Jun 27, 2022

Uh oh!

github-actions bot commented Jun 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alanwaketan commented Jun 17, 2022 •

edited

Loading

facebook-github-bot commented Jun 17, 2022 •

edited

Loading