Repository navigation
Conversation
One row for 2 nodes x 16 ranks, ReduceScatter [136M, 168M): Ring/Simple with 32 channels instead of the default 64. It runs 7% to 19% faster there in float, bfloat16 and half. All other rows are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
aduforest
marked this pull request as ready for review
November 3, 2026 21:27
aduforest
marked this pull request as draft
November 3, 2026 21:29
Member
Author
|
@argus review |
aduforest
marked this pull request as ready for review
November 3, 2026 21:29
Contributor
There was a problem hiding this comment.
ARGUS
Findings
1 more finding(s), not posted inline
projects/rccl/tuner/rccl_tuner_gfx950.csv:8(Low) No test probes the 2-node/16-rank reducescatter row (AppliesEmbeddedConfigToCostTable uses 1 node/8 ranks); a follow-up asserting non-comment row count == numConfigs per embedded CSV in csv-tuner-test.cc would catch a dropped row.
Bottom line: Looks approval ready.
Comment @argus review to re-run, or @argus re-review after pushing.
Member
Author
|
Folded into #14722, which now contains the combined two-node tuning and boundary/fallback tests. The original benchmark evidence and limitations are preserved and linked there. Closing this PR as superseded; no changes were merged into develop. |
Contributor
pr-rocm-systems-14898-oci-rccl-pr-buildBuilds
FailuresNo failing tests in this run. 189 tests skipped. pr-rocm-systems-14898-oci-rccl-pr-smokeBuilds
Failures
15 tests skipped. |
Contributor
pr-rocm-systems-14898-ruby64-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
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
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.
Motivation
Improve ReduceScatter performance on MI350X, 2 nodes × 8 GPUs (16 ranks). Using 32 instead of 64 channels reduces median time by 6.0%-16.9% in the verified float cases.
Technical Details
Adds this row to
projects/rccl/tuner/rccl_tuner_gfx950.csv:Uses Ring/Simple with 32 channels for
136 MiB <= size < 168 MiB, only on two nodes with 16 ranks. Exactly 168 MiB and settings outside this scope are unchanged. Smaller sizes are excluded because the benefit was inconsistent.Test Plan
Verified float logs from job 113968: six rounds around the changed range, three wider 1-512 MiB rounds, and 12 execution-mode combinations. Existing/new settings and repeated controls ran in the same allocation, with order rotated. Both in-place and out-of-place operations were checked.
A separate 88-run follow-up repeated questionable controls and checked both rule boundaries.
Test Result
#wrong=0, and 1.06x-1.20x speedup across all 120 tested float cases inside the range.#wrong=0; all 24 mode/size boundary checks passed.Measurements and reproduction details
Median out-of-place float times, DEFAULT policy, registration 0, no graph:
The last sample is 256 bytes below 168 MiB. These samples use Ring/Simple, with 64 channels before and 32 after.
The 12 tested modes span DEFAULT/EFFICIENCY/ZERO policies, registration modes and graph/non-graph runs—not every possible combination.
Follow-up job 115790 used another two-node allocation, six rounds and all six orders of existing/repeated-existing/new settings. It tested EFFICIENCY/R2 without graphs, ZERO/R2 without graphs, and EFFICIENCY/R1 and R2 with graphs. Initial/final hardware checks passed. Reported settings matched at every control size. Probes at 256 bytes below, at and above both boundaries confirmed the rule's scope.
Original outside-range and repeated-control differences reached 11.3%; some 8%-9% differences had stable matching controls. Follow-up control variation peaked at 2.1%. The remaining 4.4% slowdown occurred at 2 MiB with EFFICIENCY/R2 without graphs.
Command template; select one value per placeholder and add
-G 1for graphs:For the wider sweep, replace the size arguments with
-b 1M -e 512M -f 2.Both CSVs used RCCL 2.32.3 (
091930df78f8d9402e53034fda2adbd53e3e3aec), ROCm 10.1.0 and Open MPI 5.0.10. The rccl-tests binary reports2.19.6-HEAD:7ee494a5d4, SHA-256cafd89d0ba72aedbf84e49bc233b990d94931dfd555f661295297363247dcc91. These are same-build CSV comparisons, not PR/base builds. Full logs are retained, not attached.Issue Tracking
JIRA ID: AICOMRCCL-3447
Submission Checklist