ras: include ncclTimeout in the client error breakdown#2263
Open
EylonKrause wants to merge 1 commit into
Open
Conversation
rasClientBreakDownErrors iterated error codes with i < ncclInProgress, which stops at enum 6 and never visits ncclTimeout (enum 8, which sorts after ncclInProgress=7). The headline rank count (computed as nRanks minus ncclSuccess/ncclInProgress) does include timeout ranks, so on a timeout the summary said N ranks errored while the per-rank breakdown listed fewer. Loop to ncclNumResults skipping ncclInProgress, and add the missing ncclTimeout case to ncclErrorToString (was falling through to Unexpected). Signed-off-by: EylonKrause <eylon1909@gmail.com>
Author
|
Disclosure: this contribution was authored with an AI coding assistant (Claude) and reviewed before submission. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
rasClientBreakDownErrorsenumerates the most-frequent NCCL error codes among a communicator's ranks withfor (int i = ncclUnhandledCudaError; i < ncclInProgress; i++). That bound stops atncclRemoteError(6) and never visitsncclTimeout(8), which sorts afterncclInProgress(7) inncclResult_t. But the headline count is computed asnRanks - (ncclErrors[ncclSuccess] + ncclErrors[ncclInProgress]), which does include timeout ranks. So when one or more ranks reportncclTimeout, the RAS report states "Asynchronous error on N ranks" while the per-rank breakdown lists fewer than N — the timeout ranks are attributed to no rank/GPU/node. Separately,ncclErrorToStringhas noncclTimeoutcase and falls through to "Unexpected error".Related Issues
None.
Changes & Impact
src/ras/client_support.cc: loop toncclNumResultswhile skippingncclInProgress, soncclTimeoutis included in the breakdown; and add the missingncclTimeoutcase toncclErrorToString. Diagnostic / serviceability output only; no data-path effect. Non-breaking.Performance Impact
None.
Testing
make src.build;all_reduce_perfregression:Out of bounds values : 0 OK.ncclTimeout = 8 > ncclInProgress = 7) versus the loop bound; the header/breakdown disagreement appears only when a rank reportsncclTimeout.