Ensure NCCL error handling code is disabled for NCCL versions < 2.4 #25634
Conversation
pytorchbot
added
oncall: distributed
pietern
left a comment
pietern
left a comment
There was a problem hiding this comment.
How does this impact the watchdog thread? Will it effectively be a big nop now?
LGTM otherwise.
Ensured we don't start this thread when error checking is disabled. |
mrshenli
left a comment
mrshenli
left a comment
There was a problem hiding this comment.
LGTM! Thanks! Let's make sure all CI pass before landing.
There was a problem hiding this comment.
This caused build failure:
Sep 05 20:06:57 CMakeFiles/ProcessGroupNCCLErrorsTest.dir/ProcessGroupNCCLErrorsTest.cpp.o: In function `ProcessGroupNCCLErrorsTest_testNCCLErrorsNonBlocking_Test::TestBody()':
Sep 05 20:06:57 ProcessGroupNCCLErrorsTest.cpp:(.text+0x4f1): undefined reference to `torch::cuda::nccl::version()'
There was a problem hiding this comment.
These files don't link against all of libtorch. It's better to use the macros here.
|
Is this PR just waiting on a conflict resolution at this point? |
|
@jithunnair-amd A few CI builds are still failing, I need to fix them. |
…25634) Summary: Pull Request resolved: #25634 ncclCommAbort() and ncclGetAsyncError() were two APIs added in NCCL 2.4 to detect errors in NCCL communicators. These were used as part of ProcesGroupNCCL and we also enforced that only NCCL versions 2.4+ were supported. Although, there is still legitimate use for older NCCL versions and hence we should still support those. For that purpose, in this change I've ensured we disable NCCL error checking for versions < 2.4. Test Plan: 1) Test with 2.4.8 2) Test with 2.2.13 3) unit tests. Differential Revision: D17178988 fbshipit-source-id: 757d4cf7c85dbfab804341b8b96fb480c4dda00b
|
@pritamdamania87 Will this PR be merged soon? |
|
@jithunnair-amd Apologize for the delay on this, I was tied up with a few other PRs. I spent some time this week trying to fix the build issue but haven't made much progress on that front. Will give it another shot soon when I have some free cycles. |
|
Abandoning in favor of #27124 (which was exported using ghexport). |
6 participants
Summary:
ncclCommAbort() and ncclGetAsyncError() were two APIs added in NCCL
2.4 to detect errors in NCCL communicators. These were used as part of
ProcesGroupNCCL and we also enforced that only NCCL versions 2.4+ were
supported. Although, there is still legitimate use for older NCCL versions and
hence we should still support those.
For that purpose, in this change I've ensured we disable NCCL error checking
for versions < 2.4.
Test Plan:
Differential Revision: D17178988