TL/UCP: add exact mixed-radix allgather schedules

[jeffnvidia]

What

Add exact per-phase radix schedules to TL/UCP K-nomial AllGather. A schedule
such as 8x6 uses radix 8 in the first phase and radix 6 in the second; its
product must equal the team size.

Set an explicit schedule with, for example:

UCC_TL_UCP_ALLGATHER_KN_RADIX=8x6

The same option accepts a fixed radix or auto. The existing fixed-radix path
and AllGatherV behavior remain unchanged. Invalid schedules are rejected, and
existing algorithm tuning still decides whether K-nomial AllGather is selected.

Why

For non-power team sizes, a fixed radix can produce a truncated final phase or
proxy/extra ranks. An exact mixed schedule covers the complete team with an
explicit radix for every phase.

Across three tested environments at 96 ranks, a representative exact schedule
improved the large-message endpoint by 1.81x--1.99x over the best measured
fixed radix by avoiding the fixed-radix remainder path.

Performance evidence

Results are intentionally anonymized and normalized to the best measured
fixed-radix execution for each environment. Each row uses one exact schedule
at the endpoint; it is not a pointwise best-schedule envelope.

Environment	Team	Exact schedule	Fixed baseline	Normalized speedup
System A	96 ranks	4x4x6	R4	1.98x
System B	96 ranks	2x2x2x2x2x3	R4	1.99x
System C	96 ranks	8x3x4	R3	1.81x

These relative results demonstrate the remainder-path mechanism; they are not
intended as absolute system-performance comparisons.

Validation

• Capability debug build and focused schedule tests passed.
• 16-rank CUDA AllGather covered fixed R2/R4/R8 and explicit 8x2, 2x8,
  and 4x4; all cases passed with zero failures.
• Tests cover parsing and bounded formatting, legacy fixed-radix behavior, and
  exact mixed peer sets, segment sizes, and offsets for every rank.

Automatic schedule selection is proposed separately in #1328.

[jeffnvidia]

Maintainer note on the tl_ucp.c diff and codestyle:

I intentionally reduced this file to the functional seven-line
ALLGATHER_KN_MIXED_RADICES entry and removed the unrelated formatting churn.
With clang-format 21.1.8, touching that entry expands formatting to the entire
legacy configuration-array initializer, changing the file from 7 additions / 0
deletions to 105 additions / 49 deletions. I did not add formatter-suppression
directives.

The minimal diff may therefore make the codestyle check fail. Could maintainers
please accept/waive that result, or advise if a separate formatting-only change
is preferred?

[greptile-apps]

Greptile Summary

The PR adds exact mixed-radix schedules for TL/UCP K-nomial AllGather while retaining the fixed-radix and AllGatherV paths.

• Adds parsing, formatting, cloning, and range lookup for ordered radix schedules.
• Extends K-nomial pattern progression to use a per-phase radix.
• Validates that mixed-schedule products equal the team size.
• Adds focused parser and peer-layout tests.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up scope.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/components/tl/ucp/allgather/allgather_knomial.c	Selects and validates message-range radix schedules and initializes mixed-radix AllGather tasks; no follow-up-eligible issue was identified.
src/coll_patterns/sra_knomial.h	Initializes exact mixed-radix AllGather patterns while preserving the legacy single-radix path.
src/coll_patterns/recursive_knomial.h	Advances the active radix between mixed-schedule phases.
src/utils/ucc_parser.c	Adds parsing, printing, cloning, and release support for ranged radix schedules.
src/utils/ucc_datastruct.c	Adds deep-copy and destruction support for radix-schedule range entries.
test/gtest/coll/test_knomial_schedule.cc	Covers legacy fixed-radix behavior and exact mixed-radix peer and segment layouts.
test/gtest/utils/test_parser.cc	Covers valid, invalid, ranged, formatted, overlong, and cloned schedule configurations.

_{Reviews (4): Last reviewed commit: "TL/UCP: address follow-up mixed-radix re..." | Re-trigger Greptile}

[Sergei-Lebedev]

/build

[Sergei-Lebedev]

i would maybe change semantic here, instead of exact decompostion consider it as allowed factors in decomposition so it generalises to any team size in a run.

[jeffnvidia]

don't we want the option for the user to provide the exact order ? (between 8x6 and 6x8 for example) I'm thinking that maybe the user would for example want to start with the first radix being the number of ranks in one node.

In the second PR, I provide an auto tuner that selects the radices and the order anyways but if the user chooses to select, I was thinking we ggive him "full control"

[svcnbu-swx-hpcx]

🤖 CI Triage Agent — Linter · commit d17161bf

TL;DR: The Linter (clang-tidy) job failed because allgather_knomial.c:250 contains a dead store — radix = p->radix; is assigned but never read in the ALLGATHER branch — which clang-tidy's clang-analyzer-deadcode.DeadStores check flags as an error (exit 125). Remove the dead assignment.

Full analysis

Summary: clang-tidy failed with 1 warning-treated-as-error: "Value stored to 'radix' is never read" in allgather_knomial.c.

Root cause: In ucc_tl_ucp_allgather_knomial_start, the ALLGATHER branch reassigns radix = p->radix; (line 250) after ucc_kn_ag_pattern_init, but nothing in that branch subsequently reads radix. radix is only consumed in the ALLGATHERV/else branches, which use the value set at initialization (line 234) and don't depend on line 250. This makes line 250 a dead store, which clang-analyzer-deadcode.DeadStores treats as an error under -warnings-as-errors, so the run-clang-tidy step counts an error: and exits 125.

Implicated commit: d17161b — "TL/UCP: address mixed-radix review" by jeffnvidia (the exact commit under test); the mixed-radix pattern work in ec51644 is the origin of the pattern-init reshuffle.

File: src/components/tl/ucp/allgather/allgather_knomial.c:250

Suggested fix: Delete the dead assignment radix = p->radix; on line 250. If the intent was for later code in the ALLGATHER branch to use the pattern-selected radix, then actually use it (e.g. pass it where needed); otherwise removing the line clears the clang-tidy error. If retaining it for symmetry/readability is desired, annotate with // NOLINT(clang-analyzer-deadcode.DeadStores), but simple removal is cleanest.

Related: PR #1327 (this change); prior mixed-radix PRs touching this file (#1176, #1072). No existing issue found for this specific warning.

[svcnbu-swx-hpcx]

🤖 CI Triage Agent — Lint (ROCm) · commit d17161bf

TL;DR: The Lint (ROCm) job failed because clang-tidy (run with -warnings-as-errors) flagged a dead store at allgather_knomial.c:250 — radix = p->radix; is assigned but never read afterward. Remove that unused assignment (or use the variable) to fix the lint error.

Full analysis

Summary: clang-tidy clang-analyzer-deadcode.DeadStores error caused the Lint job to exit with code 125 (the build itself succeeded).

Root cause: In ucc_tl_ucp_allgather_knomial_start, the UCC_COLL_TYPE_ALLGATHER branch assigns radix = p->radix; at line 250, but radix is not read again in that branch (only offset/rbuf are used, then the branch ends). clang-analyzer reports "Value stored to 'radix' is never read," and because the CI step counts any error: line and exits 125, the lint gate fails. This dead store was introduced by the new mixed-radix allgather logic, where ucc_kn_ag_pattern_init computes the effective radix into p->radix.

Implicated commit: ec516448 (jeffnvidia, "TL/UCP: add exact mixed-radix allgather"), on the current head d17161bf (jeffnvidia, "TL/UCP: address mixed-radix review").

File: src/components/tl/ucp/allgather/allgather_knomial.c:250

Suggested fix: Remove the now-unused radix = p->radix; line in the ALLGATHER branch. If the intent was to make the effective (possibly clamped) radix visible for later use, then it must actually be used — but since the subsequent code in that branch doesn't reference radix, the clean fix is to delete line 250. If radix is unused after ucc_kn_ag_pattern_init in all branches, also confirm the local isn't left otherwise unused.

Related: PR #1327 (this PR, jmahou/ucc-mixed-radix-clean). No prior matching issue found.

[svcnbu-swx-hpcx]

🤖 CI Triage Agent — Linter-NVIDIA · commit d17161bf

TL;DR: The Linter-NVIDIA clang-tidy job failed because allgather_knomial.c:250 stores p->radix into the local radix variable that is never read again, tripping clang-analyzer-deadcode.DeadStores (treated as error). Remove the dead radix = p->radix; assignment.

Full analysis

Summary: clang-tidy-17 static analysis failed with 1 warning treated as error: dead store to radix in allgather_knomial.c, causing the job to exit 125.

Root cause: In ucc_tl_ucp_allgather_knomial_start(), the UCC_COLL_TYPE_ALLGATHER branch calls ucc_kn_ag_pattern_init(...) and then executes radix = p->radix; (line 250). The local radix is not used anywhere after this assignment in that branch (the code uses p members, offset, and rbuf), so the store is dead. clang-analyzer flags it as clang-analyzer-deadcode.DeadStores, and the linter workflow treats warnings as errors (1 warning treated as error), producing exit code 125. This was introduced by the mixed-radix refactor on this branch, where the pattern-init API changed and left the reassignment vestigial.

Implicated commit: d17161b — jeffnvidia, "TL/UCP: address mixed-radix review" (HEAD of PR #1327; the dead-store pattern originates in the mixed-radix series starting ec51644).

File: src/components/tl/ucp/allgather/allgather_knomial.c:250

Suggested fix: Delete the dead assignment on line 250 (radix = p->radix;). Since radix is already initialized at line 234 from task->allgather_kn.p.radix and is only read in the ALLGATHERV/AGX branches (lines 271, 296, 300), removing line 250 clears the warning without changing behavior. If the intent was to refresh radix after ucc_kn_ag_pattern_init mutates p->radix, verify whether any later ALLGATHER-branch code actually needs it — currently none does — otherwise the removal is correct.

Related: PR #1327 (jmahou/ucc-mixed-radix-clean); prior mixed-radix commit ec51644.