[Release/1.7] Enable NCCL A2A on OSS #48857
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mingzhe09088
requested review from
mrshenli,
pietern,
pritamdamania87,
rohan-varma and
zhaojuanmao
as code owners
facebook-github-bot
added
cla signed
oncall: distributed
💊 CI failures summary and remediationsAs of commit 445963c (more details on the Dr. CI page):
XLA failureJob pytorch_xla_linux_bionic_py3_6_clang9_test is failing. Please create an issue with title prefixed by Extra GitHub checks: 1 failed
ci.pytorch.org: 1 failedThis 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. This comment has been revised 10 times. |
Summary: Pull Request resolved: pytorch#45899 Use function polymorphism to avoid repeated casts I.e. instead of using `NCCL_CHECK(from_nccl_result(` add variant of the function that takes `ncclResult_t` as input argument Add non-pointer variant of `to_nccl_comm` to avoid `*to_nccl_comm(&comm)` pattern Test Plan: Imported from OSS Reviewed By: walterddr Differential Revision: D24138012 Pulled By: malfet fbshipit-source-id: 7f62a03e108cbe455910e86e894afdd1c27e8ff1
Summary: Pull Request resolved: pytorch#45900 Use `torch:cuda::nccl:all2all` from `ProcesGroupNCCL.cpp` Fixes pytorch#42517 Here is a NCCL dependency graph: ``` libnccl.a --> libtorch_cuda.so ---> libtorch_python.so | ^ | | --------> libc10d.a ----------------- ``` When static library is linked into a dynamic library or an executable, linker is removes all unused/duplicate symbols from that library, unless `-whole-archive` option is used. Before pytorch#42514 all nccl call made from `ProcessGroupNCCL.cpp` were also made from `torch/csrc/cuda/nccl.cpp`, which is compiled as part of `libtorch_cuda.so` But adding `ncclSend`|`ncclRecv` to ProcesGroupNCCL.cpp forced linker to embed those into `libtorch_python.so`, which also resulted in linking other dependent symbols into the library. This PR adds `nccl[Send|Recv]` call to `torch_cuda.so` by implementing `all2all` in `torch_cuda` and thus avoids double linking the static library. More involved, but prone solution, would be to use wrappers exported in `torch::cuda::nccl` namespace, instead of making direct NCCL API calls. Test Plan: Imported from OSS Reviewed By: mingzhe09088 Differential Revision: D24138011 Pulled By: malfet fbshipit-source-id: 33305197fc7d8707b7fd3a66b543f7733b9241a1
mingzhe09088
force-pushed
the
release/1.7
branch
from
44b0e00 to
445963c
Compare
Contributor
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Pull Request resolved: #45900
Use
torch:cuda::nccl:all2allfromProcesGroupNCCL.cppFixes #42517
Here is a NCCL dependency graph:
When static library is linked into a dynamic library or an executable, linker is removes all unused/duplicate symbols from that library, unless
-whole-archiveoption is used. Before #42514 all nccl call made fromProcessGroupNCCL.cppwere also made fromtorch/csrc/cuda/nccl.cpp, which is compiled as part oflibtorch_cuda.soBut adding
ncclSend|ncclRecvto ProcesGroupNCCL.cpp forced linker to embed those intolibtorch_python.so, which also resulted in linking other dependent symbols into the library.This PR adds
nccl[Send|Recv]call totorch_cuda.soby implementingall2allintorch_cudaand thus avoids double linking the static library.More involved, but prone solution, would be to use wrappers exported in
torch::cuda::ncclnamespace, instead of making direct NCCL API calls.Test Plan: Imported from OSS
Reviewed By: mingzhe09088
Differential Revision: D24138011
Pulled By: malfet
fbshipit-source-id: 33305197fc7d8707b7fd3a66b543f7733b9241a1
Fixes #{issue number}