Skip to content

TL/UCP: select exact allgather radix schedules - #1328

Open
jeffnvidia wants to merge 6 commits into
openucx:masterfrom
jeffnvidia:jmahou/ucc-mixed-radix-selector
Open

TL/UCP: select exact allgather radix schedules#1328
jeffnvidia wants to merge 6 commits into
openucx:masterfrom
jeffnvidia:jmahou/ucc-mixed-radix-selector

Conversation

@jeffnvidia

@jeffnvidia jeffnvidia commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

What

Add opt-in, message-size-aware exact-schedule selection for TL/UCP K-nomial
AllGather. This PR is stacked on the mixed-radix execution capability in #1327.

UCC_TL_UCP_ALLGATHER_KN_MIXED_RADICES has three states:

  • unset or empty: preserve the legacy fixed-radix path;
  • auto: select an exact fixed or mixed schedule;
  • a list such as 8,6: use that authoritative exact schedule.

Policy

Automatic mode uses the total AllGather receive payload:

  • below 1 GiB, enumerate exact factorizations from radices 2--9, minimize the
    number of phases, then minimize total fanout sum(radix - 1);
  • at or above 1 GiB, retain the conservative R2-oriented exact factorization.

Ties use a deterministic priority order: powers of two first (8,4,2), then
the remaining radices in descending order. Uniform selected schedules are
normalized to the existing fixed-radix path. If no supported exact
factorization exists, selection falls back to the legacy path.

Examples:

Team Message range Schedule
60 small 4,5,3
64 small fixed R8
72 small 8,9
96 small 4,4,6
128 small 8,4,4
96 large 2,2,2,2,2,3

Performance evidence

The legacy topology heuristic selects fixed R4 at 64 ranks and does not use
message size. Results below are intentionally anonymized and normalized to
that measured legacy-auto execution (1.00).

Environment Message range Current auto Normalized speedup vs legacy auto
System A below 1 GiB R8 1.159x
System A 1--8 GiB R2 1.167x
System B below 1 GiB R8 1.056x
System B 1--8 GiB R2 1.001x

On System B below 1 GiB, current auto was within 0.14% on average / 1.04%
worst case
of the measured R2/R4/R8 winner. At 1--8 GiB, those candidates
converged and current auto stayed within 0.10% on average / 0.23% worst
case
. This provides portability/non-regression evidence rather than claiming
a universal large-message R2 gain.

Additional observations:

  • Exact mixed schedules avoid the fixed-radix remainder penalty: at 96 ranks,
    representative exact schedules reached 1.81x--1.99x the best measured
    fixed-radix endpoint across three anonymous environments.
  • For small messages, fewer phases consistently win. With phase count fixed,
    a 72-rank exhaustive sweep ranked every permutation of the balanced 3,4,6
    factors above more uneven three-phase factorizations. At 32 ranks in another
    environment, the balanced 2,4,4 family similarly beat 2,2,8.
  • Ordering the same factor multiset was usually a secondary effect and showed
    no portable largest-first, largest-last, or PPN-first rule. The selector
    therefore uses a topology-independent cost and stable ordering.
  • Large-message results remain runtime- and platform-dependent. Explicit
    schedules remain available when a deployment has stronger local evidence.

These relative results demonstrate selector behavior and should not be used as
absolute system-performance comparisons.

Validation

  • Selection tests cover an infeasible minimum phase count at 60 ranks, fixed
    R8, 8,6, 8,9, balanced 4,4,6 and 8,4,4, the large-message R2
    schedule, odd factors, and unsupported exact shapes.
  • The validated tree passed a debug build, install/config validation, focused
    schedule tests, exhaustive selector-oracle coverage, and CUDA AllGather
    checks.

@dpressle

Copy link
Copy Markdown
Collaborator

/build

@dpressle

Copy link
Copy Markdown
Collaborator

@ci-triage-agent

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC%2Fucc-test-mpi · commit 2d72f80f

TL;DR: The ngc_pytorch build stage failed compiling allgather_knomial.c because PR #1328 calls ucc_array_size(candidates) — a function that doesn't exist in UCC — and -Werror turns the implicit-declaration warning into a hard error. Replace it with the existing UCX helper ucs_array_length() (or ucs_static_array_size()), whichever matches the candidates array type.

Full analysis

Summary: Build stage "Setup Image x86_64/ngc_pytorch/0" failed; make install aborted on a compile error in the TL/UCP allgather knomial component.

Root cause: src/components/tl/ucp/allgather/allgather_knomial.c:434 calls ucc_array_size(candidates), but no such symbol is declared anywhere (it's absent from ucc_math.h/ucc_datastruct.h; UCC uses UCX's ucs_array_* API). GCC emits -Werror=implicit-function-declaration (and -Werror=nested-externs), and because UCC is built with -Wall -Werror, the warning is fatal, killing the whole make -j16 install. This code path is new to PR #1328 (it does not exist on main), so it was never compiled before.

Implicated commit: [REDACTED:Hex High Entropy String] (the build under test), part of PR #1328 "TL/UCP: select exact allgather radix schedules". The touched file was last landed on main via e59b8083 (Sergey Lebedev, #1176), but the offending ucc_array_size call is added by #1328.

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

Suggested fix: Replace ucc_array_size(candidates) with the correct array-length helper for however candidates is declared:

  • If candidates is a UCX ucs_array_t, use ucs_array_length(candidates).
  • If it's a plain C fixed-size array, use ucc_static_array_size(candidates) / ucs_static_array_size(candidates).

Then rebuild locally with -Werror to confirm the implicit-declaration and nested-externs warnings are gone. (Alternatively, if a ucc_array_size macro is genuinely intended, add it to src/utils/ucc_math.h, but reusing the existing UCX helper is preferred.)

Related: PR #1328 ; prior context PR #1176 (#1176).

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 7d992e62-d176-42e8-8c27-9d14379a7bc7 in the triage console for the audit trail.

@openucx openucx deleted a comment from svcnbu-swx-hpcx Jul 13, 2026
@jeffnvidia
jeffnvidia force-pushed the jmahou/ucc-mixed-radix-selector branch from 2d72f80 to 508e8d4 Compare July 13, 2026 12:59
@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 508e8d4e

TL;DR: The codestyle lint failed because commit TL/UCP: remove stale allgather radix initialization has a title 51 characters long, exceeding the 50-char limit; shorten that commit's title to ≤50 characters and force-push.

Full analysis

Summary: The "Lint (codestyle)" GitHub Actions job failed on its commit-title check — one commit title exceeds the 50-character maximum.

Root cause: The commit-title validator in .github/workflows/codestyle.yaml rejects any non-merge commit title longer than 50 characters (if [ ${#msg} -gt 50 ]). The commit titled TL/UCP: remove stale allgather radix initialization is 51 characters long, so check_title returned 1, setting ok=0 and exiting with code 1. The log shows: Commit title is too long: 51 / Bad commit title: 'TL/UCP: remove stale allgather radix initialization'. This is a lint policy failure, not a build/test error.

Implicated commit: The offending commit is the one titled TL/UCP: remove stale allgather radix initialization (part of PR #1328, branch jmahou/ucc-mixed-radix-selector). Exact SHA not shown in log; the PR HEAD is [REDACTED:Hex High Entropy String].

File: .github/workflows/codestyle.yaml (the check_title length check, [ ${#msg} -gt 50 ]) — the failure is in the commit message, not a source file.

Suggested fix: Reword that commit's title to ≤50 characters, e.g. TL/UCP: remove stale allgather radix init (41 chars) or TL/UCP: drop stale allgather radix init (39 chars). Rebase to amend the message (git rebase -i), then force-push the branch to re-run the check.

Related: none

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 582ac137-fce0-451d-8cb0-b9fcbc497836 in the triage console for the audit trail.

@jeffnvidia
jeffnvidia force-pushed the jmahou/ucc-mixed-radix-selector branch 2 times, most recently from 81f4268 to afa8d2f Compare July 15, 2026 09:07
@jeffnvidia
jeffnvidia marked this pull request as ready for review July 15, 2026 09:31
@jeffnvidia
jeffnvidia force-pushed the jmahou/ucc-mixed-radix-selector branch 3 times, most recently from afa8d2f to 6e8663d Compare July 15, 2026 12:54
@jeffnvidia

Copy link
Copy Markdown
Contributor Author

Maintainer note: this stacked PR inherits the intentional minimal tl_ucp.c
configuration entry from #1327. The selector-only diff remains unchanged at
294 additions / 17 deletions, and there are no formatter-suppression directives.

Any clang-format/codestyle complaint about the surrounding legacy configuration
table is the same issue described on #1327: formatting the one new entry expands
to the entire existing array. Please accept/waive the minimal form, or advise if
a separate formatting-only change is preferred.

@jeffnvidia
jeffnvidia force-pushed the jmahou/ucc-mixed-radix-selector branch 13 times, most recently from 058defb to 0603a54 Compare July 16, 2026 11:20
@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit bf947bec

TL;DR: The codestyle "commit title" check failed because one commit title in PR #1328 exceeds the 50-character limit (it's 57 chars). Shorten that commit's title to ≤50 characters and force-push.

Full analysis

Summary: The codestyle job's commit-title lint step exited with code 1 because a commit message title is too long.

Root cause: The commit-title check enforces ${#msg} -gt 50 → fail (unless the title starts with Merge). The offending commit title TL/UCP: make automatic radix selection message-size aware is 57 characters, so the check printed Commit title is too long: 57 / Bad commit title and set ok=0, causing exit 1. This is a lint policy violation in the PR's commit history, not a code/build/test bug or a hang (the whole job ran in ~6 seconds with continuous output).

Implicated commit: The commit in PR #1328 (branch jmahou/ucc-mixed-radix-selector) titled TL/UCP: make automatic radix selection message-size aware (author: the PR author, jmahou). Note the checked-out HEAD is bf947bec "TL/UCP: simplify radix selector fallback", but the too-long title belongs to an earlier commit in the PR range.

File: .github/workflows/codestyle.yaml (the check_title function; the ${#msg} -gt 50 rule) — the fix is in git history, not this file.

Suggested fix: Reword the offending commit's title to ≤50 characters while keeping a valid header prefix (TL/UCP:). For example:

  • TL/UCP: message-size aware radix selection (42 chars), or
  • TL/UCP: size-aware auto radix selection (39 chars).

Use an interactive rebase to edit that commit, then force-push:

git rebase -i remotes/origin/<base>
# mark the "message-size aware" commit as 'reword', save a shorter title
git push --force-with-lease

All other titles passed, so no further changes are needed.

Related: none found.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit c968ff0d

TL;DR: The codestyle job's commit-title linter failed because one commit title exceeds the 50-character limit; shorten the title of the commit TL/UCP: make automatic radix selection message-size aware (57 chars) to ≤50 characters.

Full analysis

Summary: The "Run set -eE" commit-title check step in the codestyle workflow exited 1 because a PR commit title is longer than the allowed 50 characters.

Root cause: The linter's check_title() rejects titles longer than 50 chars (unless they start with Merge). The commit title TL/UCP: make automatic radix selection message-size aware is 57 characters, so the script printed Commit title is too long: 57 / Bad commit title, set ok=0, and exited with code 1. This is a lint policy violation, not an infrastructure/timeout issue — the whole job ran in ~8 seconds with continuous output.

Implicated commit: The PR commit titled TL/UCP: make automatic radix selection message-size aware (author: PR #1328 owner, jmahou). Full SHA not shown in log (only titles are printed by the linter).

File: .github/workflows/codestyle.yaml — the check_title() if [ ${#msg} -gt 50 ] rule (the offending data is the commit message itself, not a repo source file).

Suggested fix: Reword and amend the offending commit title to ≤50 characters, e.g. TL/UCP: message-size-aware radix selection (42 chars) or TL/UCP: size-aware auto radix selection (39 chars). Since this is one commit in a stacked series, use an interactive rebase (git rebase -i, mark that commit reword), then force-push the branch. No code change is required.

Related: PR #1328 (jmahou/ucc-mixed-radix-selector). No related issues found/searched — the diagnosis is fully determined by the log.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 4a8a3814

TL;DR: The codestyle lint job failed its commit-title-length check because one commit in PR #1328 has a 57-character title (limit is 50). Shorten that commit's title to ≤50 characters.

Full analysis

Summary: The codestyle job's commit-title validation step exited with code 1 after flagging one commit title as too long.

Root cause: The lint script rejects non-merge commit titles longer than 50 characters. The commit titled TL/UCP: make automatic radix selection message-size aware is 57 characters, producing Commit title is too long: 57 / Bad commit title and setting ok=0, which triggers exit 1. All other title checks passed (headers and format are fine); only length failed.

Implicated commit: The PR commit titled TL/UCP: make automatic radix selection message-size aware (author jmahou, branch jmahou/ucc-mixed-radix-selector). Full SHA not shown in log (only %h was printed); not the checked-out HEAD 4a8a3814.

File: .github/workflows/codestyle.yaml — the check_title() function, if [ ${#msg} -gt 50 ] length gate.

Suggested fix: Reword the offending commit title to ≤50 characters, e.g. TL/UCP: message-size-aware radix selection (42 chars) or TL/UCP: size-aware auto radix selection (39 chars), then rebase/force-push the branch (git rebase -i → reword that commit). No source-code change is needed; this is purely a commit-message policy violation.

Related: none

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit d75697ef

TL;DR: The codestyle lint failed because one commit title in PR #1328 exceeds the 50-character limit; shorten TL/UCP: make automatic radix selection message-size aware (57 chars) to ≤50 characters and force-push.

Full analysis

Summary: The codestyle GitHub Actions job failed its commit-title check because a non-merge commit title is longer than 50 characters.

Root cause: The check-title script in .github/workflows/codestyle.yaml rejects any non-Merge commit whose subject is longer than 50 characters. The commit titled TL/UCP: make automatic radix selection message-size aware is 57 characters, so check_title printed Commit title is too long: 57, set ok=0, and the script exited with code 1. All other titles in the branch passed.

Implicated commit: The offending commit is the one titled TL/UCP: make automatic radix selection message-size aware on branch jmahou/ucc-mixed-radix-selector (author jmahou; exact SHA not printed in the log — it uses %h short hashes that weren't echoed alongside titles). HEAD of the PR is [REDACTED:Hex High Entropy String].

File: .github/workflows/codestyle.yaml — the check_title() function, if [ ${#msg} -gt 50 ] guard.

Suggested fix: Reword that commit's subject to ≤50 characters and force-push the rebased branch. For example:

  • TL/UCP: message-size aware auto radix (37 chars), or
  • TL/UCP: size-aware auto radix selection (39 chars).

Use an interactive rebase (git rebase -i, mark the commit reword), then git push --force-with-lease. No source-code change is needed; this is purely a commit-message policy violation.

Related: none

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id f938eab0-b0ff-4bfa-9c53-a8e7cb3af303 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit b75264f4

TL;DR: The codestyle job failed its commit-title lint because one commit in PR #1328 has a 57-character title, exceeding the 50-character limit; shorten that commit's title to ≤50 chars.

Full analysis

Summary: The "Lint (codestyle)" GitHub Actions job failed at the commit-title check step with exit code 1.

Root cause: The commit-title linter in .github/workflows/codestyle.yaml enforces a 50-character max on non-Merge commit titles. The commit titled TL/UCP: make automatic radix selection message-size aware is 57 characters long, so the check printed Commit title is too long: 57 / Bad commit title, set ok=0, and exited 1. All other commit titles passed. This is a lint policy violation, not a build/test hang — the job ran to completion in ~7 seconds.

Implicated commit: The commit with title TL/UCP: make automatic radix selection message-size aware (one of the commits in PR #1328 / branch jmahou/ucc-mixed-radix-selector; author not shown in log, likely jmahou). HEAD is [REDACTED:Hex High Entropy String].

File: .github/workflows/codestyle.yaml (the check_title/50-char length gate; msg length check if [ ${#msg} -gt 50 ])

Suggested fix: Reword that commit's title to 50 characters or fewer while keeping a valid TL/UCP: header, e.g.:

  • TL/UCP: make auto radix message-size aware (43 chars), or
  • TL/UCP: radix auto-select by message size (42 chars)

Apply via interactive rebase (git rebase -i, reword that commit) and force-push the branch. No source-code change is required; this is purely a commit-message policy failure.

Related: none

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id bc6aa252-4165-4d2f-b2f9-77388b70d3f2 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 35531a8d

TL;DR: The codestyle job's commit-title linter failed because one commit title in PR #1328 exceeds the 50-character limit (57 chars); shorten that commit's subject line to ≤50 characters.

Full analysis

Summary: The codestyle GitHub Actions job's commit-title check exited with code 1 because a commit in the PR branch has a title longer than the allowed 50 characters.

Root cause: The check script in .github/workflows/codestyle.yaml enforces if [ ${#msg} -gt 50 ] for non-merge commit titles. The commit titled TL/UCP: make automatic radix selection message-size aware is 57 characters long, so the script printed Commit title is too long: 57 / Bad commit title, set ok=0, and exited 1. This is a lint policy violation, not a build/test failure or a timeout — the whole job ran in ~6 seconds with no gaps.

Implicated commit: The offending commit is the one titled TL/UCP: make automatic radix selection message-size aware (author: jmahou, branch jmahou/ucc-mixed-radix-selector). The tip commit checked out was 35531a8d "TL/UCP: make mixed-radix mode explicit".

File: .github/workflows/codestyle.yaml (the check_title function; length check if [ ${#msg} -gt 50 ]). The problem is in commit data, not the workflow file.

Suggested fix: Reword that commit's title to ≤50 characters and force-push. For example, interactively rebase (git rebase -i remotes/origin/<base>), mark that commit reword, and change the subject to something like TL/UCP: msg-size-aware auto radix (33 chars) or TL/UCP: size-aware auto radix selection (39 chars). All other commit titles in the range passed, so only that one needs editing.

Related: none

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 4f2877ce

TL;DR: The codestyle commit-title lint failed because one commit title exceeds the 50-character limit; shorten TL/UCP: make automatic radix selection message-size aware (57 chars) to ≤ 50 characters and force-push.

Full analysis

Summary: GitHub Actions Lint (codestyle) job failed at the commit-title check step with exit code 1.

Root cause: The codestyle workflow's check_title function rejects any commit title longer than 50 characters (unless it's a merge commit). The commit titled TL/UCP: make automatic radix selection message-size aware is 57 characters, triggering Commit title is too long: 57Bad commit titleok=0exit 1. All other title/header/dot checks passed; this single overlong title is the only failure.

Implicated commit: The offending commit on branch jmahou/ucc-mixed-radix-selector titled TL/UCP: make automatic radix selection message-size aware (author not shown in log; part of PR #1328). The HEAD checked out was [REDACTED:Hex High Entropy String].

File: .github/workflows/codestyle.yaml (the set -eE / check_title step; the 50-char rule is at if [ ${#msg} -gt 50 ]).

Suggested fix: Reword the offending commit title to 50 characters or fewer while keeping a valid header prefix. For example, TL/UCP: message-size aware auto radix selection (47 chars) or TL/UCP: size-aware automatic radix selection (44 chars). Use an interactive rebase (git rebase -i, reword that commit) and force-push the branch. This is a genuine style violation, not an infra/flake issue — no workflow change needed.

Related: PR #1328 (openucx/ucc); no related issues found.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id bed25fdb-b001-4130-add9-ff36e89733c1 in the triage console for the audit trail.

@jeffnvidia
jeffnvidia force-pushed the jmahou/ucc-mixed-radix-selector branch from 4f2877c to 43a2ecf Compare July 19, 2026 11:12
@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 43a2ecf4

TL;DR: The codestyle lint job failed because one commit title in PR #1328 exceeds the 50-character limit; shorten it to pass. No code bug — it's a commit-message policy violation.

Full analysis

Summary: The "Lint (codestyle)" GitHub Actions job failed at the commit-title check step with exit code 1.

Root cause: The commit-title linter in .github/workflows/codestyle.yaml enforces a 50-character max on non-merge commit titles. The commit TL/UCP: make automatic radix selection message-size aware is 57 characters long, triggering Commit title is too long: 57 / Bad commit title, which sets ok=0 and exit 1. All other titles passed; this single title is the sole failure.

Implicated commit: The offending commit is the one titled TL/UCP: make automatic radix selection message-size aware on branch jmahou/ucc-mixed-radix-selector (PR #1328). (Short SHA not printed in this log; it's the 6th commit checked, between TL/UCP: simplify radix selector fallback and TL/UCP: simplify automatic radix selection.)

File: .github/workflows/codestyle.yaml — the check_title() function, guard if [ ${#msg} -gt 50 ].

Suggested fix: Reword that commit title to ≤ 50 characters (keeping a valid TL/UCP: header and no trailing period), then force-push. For example:

  • TL/UCP: message-size aware auto radix (37 chars), or
  • TL/UCP: size-aware auto radix selection (39 chars).
    Use an interactive rebase (git rebase -i, mark that commit reword) to edit only that title, then git push --force-with-lease. This is a policy check, not a source-code defect — no application file needs changing.

Related: none

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit f9edd8d4

TL;DR: The codestyle "Lint" job failed because one commit title in PR #1328 exceeds the 50-character limit; shorten TL/UCP: make automatic radix selection message-size aware (57 chars) to ≤50 characters.

Full analysis

Summary: The codestyle commit-title check failed — a commit title is longer than the allowed 50 characters.

Root cause: The workflow's check_title function rejects any non-Merge commit title longer than 50 characters. The commit titled TL/UCP: make automatic radix selection message-size aware is 57 characters, so the check printed Commit title is too long: 57 / Bad commit title, set ok=0, and exited with code 1. This is a lint policy violation, not a code or infrastructure error.

Implicated commit: The offending commit in PR #1328 (branch jmahou/ucc-mixed-radix-selector) with title TL/UCP: make automatic radix selection message-size aware. Author not shown in the log (only titles are printed); it is one of the PR's own commits.

File: .github/workflows/codestyle.yaml (the check_title logic, if [ ${#msg} -gt 50 ]) — the violation is in the commit message, not a source file.

Suggested fix: Reword the commit title to ≤50 characters and force-push. For example: TL/UCP: message-size aware auto radix (37) or TL/UCP: msg-size aware radix selection (38). Rebase interactively (git rebase -i), reword that commit, and push to re-trigger the check. All other 34 commit titles passed.

Related: none

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 56b71d77

TL;DR: The codestyle job failed because one commit title in PR #1328 is 57 characters — over the 50-char limit — and the fix is to reword/amend that commit title to ≤50 characters.

Full analysis

Summary: The codestyle commit-title lint step exited with code 1 because a commit title exceeded the 50-character maximum.

Root cause: The workflow's check_title() function rejects any non-Merge commit title longer than 50 characters. The commit titled 'TL/UCP: make automatic radix selection message-size aware' is 57 characters, triggering Commit title is too long: 57Bad commit titleok=0exit 1. This is a genuine lint violation, not an infra flake (the whole job ran in ~2 seconds with no gaps/hangs).

Implicated commit: The offending commit in PR #1328 with title TL/UCP: make automatic radix selection message-size aware (author not shown in log; it's one of the branch jmahou/ucc-mixed-radix-selector commits). Head commit of the run is 56b71d77.

File: .github/workflows/codestyle.yaml — the check_title() if [ ${#msg} -gt 50 ] check (the rule); the actual defect is the commit message itself, not the source tree.

Suggested fix: Reword the offending commit title to ≤50 characters and force-push. For example:

  • TL/UCP: make radix selection msg-size aware (44 chars), or
  • TL/UCP: size-aware auto radix selection (39 chars).

Since this is mid-history in a large series, rebase interactively and reword that commit:

git rebase -i remotes/origin/<base>
# mark the "make automatic radix selection message-size aware" commit as `reword`
git push --force-with-lease

All other 36 titles passed, so only that one needs changing.

Related: none found.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 0ccdcff8

TL;DR: The codestyle job's commit-title linter failed because one commit title in PR #1328 exceeds the 50-character limit; shorten the offending commit title to ≤50 characters.

Full analysis

Summary: The codestyle GitHub Actions job failed at the commit-title check step (exit code 1) because a commit message title is too long.

Root cause: The CI script in codestyle.yaml enforces a 50-character maximum on non-merge commit titles. The commit title TL/UCP: make automatic radix selection message-size aware is 57 characters, so check_title returned 1 and set ok=0, causing exit 1. All other titles passed. This is a pure lint policy failure, not a build/test error:

Commit title is too long: 57
Bad commit title: 'TL/UCP: make automatic radix selection message-size aware'
##[error]Process completed with exit code 1.

Implicated commit: The commit in PR #1328 with title TL/UCP: make automatic radix selection message-size aware (author: jmahou, per branch jmahou/ucc-mixed-radix-selector). SHA not printed in this log; identify it via git log --oneline on the branch.

File: .github/workflows/codestyle.yaml (the check_title shell step; length check if [ ${#msg} -gt 50 ]).

Suggested fix: Reword that single commit title to 50 characters or fewer and force-push. For example: TL/UCP: message-size aware auto radix select (44 chars) or TL/UCP: size-aware auto radix selection (39 chars). Use interactive rebase (git rebase -i) to reword that commit, then git push --force-with-lease. No source-code change is needed.

Related: none

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 78b3ef46

TL;DR: The codestyle "commit title" check failed because one commit in PR #1328 has a 57-character title, exceeding the 50-char limit; shorten that commit's subject line to ≤50 chars and force-push.

Full analysis

Summary: The codestyle GitHub Actions job failed at the commit-title lint step (check_title) with exit code 1.

Root cause: The check enforces a maximum commit-title length of 50 characters (if [ ${#msg} -gt 50 ] in .github/workflows/codestyle.yaml). The commit titled TL/UCP: make automatic radix selection message-size aware is 57 characters long, so the script printed Commit title is too long: 57 / Bad commit title, set ok=0, and exited 1. Every other commit passed both the header-prefix and length checks — this single overlong title is the only violation.

Implicated commit: The offending commit is the one with subject TL/UCP: make automatic radix selection message-size aware on branch jmahou/ucc-mixed-radix-selector (author not shown in log; it is one of the PR #1328 commits, HEAD = [REDACTED:Hex High Entropy String]).

File: .github/workflows/codestyle.yaml (the check_title step, msg > 50 guard) — this is the check, not a bug; the fix is in the commit message itself.

Suggested fix: Rewrite the overlong commit title to ≤50 characters, e.g. TL/UCP: message-size-aware auto radix or TL/UCP: size-aware automatic radix select (both begin with an allowed TL/ header, contain no trailing dot). Use an interactive rebase to reword just that commit:

git rebase -i remotes/origin/<base>
# mark the "message-size aware" commit as "reword", shorten to <=50 chars
git push --force-with-lease

No source-code change is required; this is purely a commit-message policy violation.

Related: none found.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id d3c60724-8ecc-47cd-9ece-ed21225b30de in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (codestyle) · commit 31eeeeb0

TL;DR: The Lint (codestyle) job failed because one commit title in PR #1328 exceeds the 50-character limit; shorten the offending commit message and the check will pass.

Full analysis

Summary: The commit-title lint step (.github/workflows/codestyle.yaml) exited 1 because a commit title is 57 characters, over the 50-char maximum.

Root cause: The codestyle check iterates over every commit in the PR range and enforces [ ${#msg} -gt 50 ]. The commit titled TL/UCP: make automatic radix selection message-size aware is 57 characters, so check_title returned 1 and set ok=0, causing the final exit 1. Every other title passed (correct header prefix, no trailing dot, ≤50 chars). This is purely a commit-message-length violation, not a code/build problem.

Implicated commit: The commit whose title is TL/UCP: make automatic radix selection message-size aware (in branch jmahou/ucc-mixed-radix-selector, PR #1328). Author not shown in log, but it is authored by the PR owner (jmahou).

File: .github/workflows/codestyle.yaml — the commit-title check (if [ ${#msg} -gt 50 ]); the violation is in the commit message itself, not in a source file.

Suggested fix: Reword that commit's title to ≤50 characters, e.g. TL/UCP: message-size-aware auto radix (37) or TL/UCP: size-aware automatic radix select (41). Since the branch has many commits, use an interactive rebase (git rebase -i, mark that commit reword), fix the title, then force-push. The lint will then pass.

Related: none found in logs.

Note: the log contains a redacted git basic-auth header (AUTHORIZATION: basic ***) emitted by actions/checkout — that's expected/masked, no action needed.

@jeffnvidia
jeffnvidia force-pushed the jmahou/ucc-mixed-radix-selector branch from 31eeeeb to 2b73955 Compare July 19, 2026 14:26
Add explicit per-phase radix schedules to TL/UCP K-nomial AllGather while preserving the fixed-radix and AllGatherV paths. Parse and validate exact schedules through the existing team configuration.
Cover schedule parsing, legacy fixed-radix behavior, and exact mixed-radix peer and segment layouts.
@jeffnvidia
jeffnvidia force-pushed the jmahou/ucc-mixed-radix-selector branch from 2b73955 to 469d39e Compare July 19, 2026 17:24
Add opt-in automatic exact-schedule selection. Small messages minimize phase count and total fanout, with powers of two prioritized in the generated order. Large messages retain an R2-oriented factorization, and explicit schedules remain authoritative.
Cover minimum-phase fallback, fanout and order selection, radix 9, fixed-radix normalization, the large-message boundary, odd factors, and unsupported teams.
@jeffnvidia
jeffnvidia force-pushed the jmahou/ucc-mixed-radix-selector branch from 9c1d899 to ecf55f9 Compare July 20, 2026 11:10
@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLinter · commit 27f1f1a7

TL;DR: The Linter (clang-tidy) job failed with exit code 125 because allgather_knomial.c:250 assigns radix = p->radix; but that value is never used afterward — a clang-analyzer-deadcode.DeadStores warning treated as an error. Remove the dead assignment (or use radix where intended).

Full analysis

Summary: clang-tidy static analysis failed on src/components/tl/ucp/allgather/allgather_knomial.c:250 — "Value stored to 'radix' is never read" — and since warnings are treated as errors, the linter step exited with code 125.

Root cause: In ucc_tl_ucp_allgather_knomial_start, after calling ucc_kn_ag_pattern_init(size, rank, radix, ...), line 250 does radix = p->radix; inside the UCC_COLL_TYPE_ALLGATHER branch. That reassigned radix is never read again on any subsequent code path in that branch (the following code uses p, offset, rbuf, etc., not radix). clang-analyzer-deadcode.DeadStores flags it, and the job runs run-clang-tidy with warnings-as-errors, so 1 error → exit 125.

Implicated commit: 27f1f1a7 — jeffnvidia, "TL/UCP: address radix selector review" (this is the PR HEAD [REDACTED:Hex High Entropy String]). The mixed-radix selector work in 5e88a39f/ec516448 introduced this pattern-init flow.

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

Suggested fix: Remove the dead store on line 250 (radix = p->radix;) since nothing reads radix afterward in that branch. If the intent was to reflect the selected radix back for later use (e.g. logging/subsequent phases), add a NOLINT only if that read genuinely happens; otherwise just delete the line. Minimal patch:

    if (ct == UCC_COLL_TYPE_ALLGATHER) {
        ucc_kn_ag_pattern_init(size, rank, radix, p->radices, p->n_iters,
                               args->dst.info.count, p);
-       radix  = p->radix;
        offset = ucc_buffer_block_offset(args->dst.info.count, size, rank) *

Verify radix isn't needed further down the ALLGATHER branch before removing (from the read, it is not).

Related: none found (the mixed-radix selector series: commits 5e88a39f, ec516448).

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 2d0402eb-9e3c-43e9-ad4f-60164f7d3147 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLinter-NVIDIA · commit 27f1f1a7

TL;DR: The Linter-NVIDIA (clang-tidy) job failed because allgather_knomial.c:250 stores p->radix into radix, which is never read again in the ALLGATHER branch — a dead store flagged as an error. Remove the redundant assignment (or use the value) to fix it.

Full analysis

Summary: clang-tidy-17 reported Value stored to 'radix' is never read in allgather/allgather_knomial.c, and since warnings are treated as errors the linter step exited with code 125.

Root cause: In ucc_tl_ucp_allgather_knomial_start (ALLGATHER branch), line 250 radix = p->radix; reassigns radix after ucc_kn_ag_pattern_init, but the subsequent code in that branch only uses size, rank, and pradix is never read again before the function returns via ucc_progress_queue_enqueue. The clang-analyzer-deadcode.DeadStores check flags this dead store, and the workflow counts any error: line as a failure (nerrors != 0exit 125).

Implicated commit: 27f1f1a7 — jeffnvidia, "TL/UCP: address radix selector review" (PR #1328, branch jmahou/ucc-mixed-radix-selector).

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

Suggested fix: Remove the now-dead radix = p->radix; assignment on line 250 (the ALLGATHER branch does not use radix afterward). If the intent was for radix to reflect the pattern's radix for later use, move the assignment to where it's actually consumed, or drop it. Alternatively, if it must be kept for symmetry, annotate with // NOLINT(clang-analyzer-deadcode.DeadStores), but removing it is cleaner. Verify the ALLGATHERV/else branches still receive the original radix argument (they do — they use it before any reassignment).

Related: PR #1328 (this change); prior radix-schedule commits 5e88a39f and ec516448 on the same file.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (ROCm) · commit 27f1f1a7

TL;DR: The ROCm Lint (clang-tidy) job failed because line 250 of allgather_knomial.c stores radix = p->radix; whose value is never subsequently read — a dead store that clang-tidy treats as an error. Remove that assignment (or use the value) to fix it.

Full analysis

Summary: Lint (ROCm) job exited 125 — clang-tidy clang-analyzer-deadcode.DeadStores error, treated as error, in allgather_knomial.c.

Root cause: In ucc_tl_ucp_allgather_knomial_start, the ALLGATHER branch assigns radix = p->radix; at line 250, but radix is never read again after this point in that branch (it's only consumed on line 248 before the reassignment, and in the other ALLGATHERV/else branches). This is a dead store. The Lint job counts any error: in the clang-tidy log and exits 125 (if [ $nerrors -ne 0 ]; then exit 125). This assignment was introduced/left over by the mixed-radix selector work in the PR under test.

Implicated commit: 27f1f1a7 (jeffnvidia, "TL/UCP: address radix selector review") — the head commit of this PR; the dead store lives in code added by the 5e88a39f/ec516448 mixed-radix allgather series.

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

Suggested fix: Remove the now-redundant radix = p->radix; line (line 250), since the value isn't used afterward in the ALLGATHER branch. If it was intended to be consumed downstream (e.g. inside the loop/other logic that used to read radix), then update that code to actually use the reassigned value. Simplest correct fix given current code is to delete line 250.

Related: PR #1328 (this build); prior mixed-radix commits 5e88a39f, ec516448.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLinter-NVIDIA · commit 3b4b5007

TL;DR: The clang-tidy (CUDA) setup step failed during the CUDA-repo apt-get update because the LLVM apt repository (apt.llvm.org) became unreachable over IPv6 mid-run, leaving apt with no Release file for llvm-toolchain-noble-17. This is a transient network/infrastructure flake unrelated to the PR's code — retry the job.

Full analysis

Summary: The clang-tidy (CUDA) job failed at the "install CUDA" step's apt-get update with exit code 100.

Root cause: At 16:13:06 apt tried to refresh the LLVM repo and failed: Cannot initiate the connection to apt.llvm.org:80 (2a04:4e42:26::561). - connect (101: Network is unreachable), followed by E: The repository 'http://apt.llvm.org/noble llvm-toolchain-noble-17 Release' no longer has a Release file. Note that the same repo was reached successfully only ~90 seconds earlier (16:11:52, over IPv4 151.101.162.49). The later refresh attempted the IPv6 address 2a04:4e42:26::561 and the runner had no working IPv6 route, so apt-get update exited non-zero (exit 100) and the -e shell aborted the step. This is a transient connectivity flake in the CI environment, not a defect in the PR's mixed-radix-selector code — the failure occurs in the environment-setup step, before any source is compiled or clang-tidy is run.

Implicated commit: none — the failure is environmental, not caused by commit [REDACTED:Hex High Entropy String].

File: .github/workflows/clang-tidy-nvidia.yaml (the apt-get update in the CUDA-install step)

Suggested fix: Re-run the job first; it will very likely pass. To make this step resilient to the recurring LLVM-repo/IPv6 flakiness, harden the workflow:

  • Force apt over IPv4 for these steps: echo 'Acquire::ForceIPv4 "true";' | sudo tee /etc/apt/apt.conf.d/99force-ipv4, or
  • Make the CUDA-install apt-get update retry/tolerate the transient LLVM-repo failure (e.g. sudo apt-get update -o Acquire::Retries=3 and consider -o APT::Update::Error-Mode=any handling), and/or add a retry wrapper around the setup steps.

Related: none found in this repository.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 871a2dfb-7ad6-4700-95e7-5447da60dfc0 in the triage console for the audit trail.

@dpressle

Copy link
Copy Markdown
Collaborator

/build

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-mpi · commit 97b051fc

TL;DR: The ucc-test-mpi bulk run crashed with SIGABRT — a ucc_assert_always(0) in ucc_dummy_progress (src/schedule/ucc_schedule.c:101) fired because a CL/HIER+ucp triggered collective was progressed without ever having its real progress callback installed. This is a genuine crash, not a timeout; fix the CL/HIER triggered-post init to assign task->progress.

Full analysis

Summary: Stage "Run UCC MPI tests (bulk)" aborted (exit 134) during the CL/HIER+ucp, triggered mode (-T), -c alltoall,alltoallv,allreduce,barrier test.

Root cause: ucc_context_progress invoked a task's progress callback that was still the default ucc_dummy_progress, which unconditionally calls ucc_assert_always(0) ("this function should never be called", src/schedule/ucc_schedule.c:98-102). ucc_coll_task_init installs ucc_dummy_progress as the placeholder (line 121) expecting each collective to override it; a CL/HIER triggered-post path put a task on the progress queue without assigning a real progress function. The trailing exit code 143 is Jenkins/slurm teardown after the abort — a symptom, not the cause (the crash and cancellation occur within ~4 seconds, no hang/idle gap).

Implicated commit: unknown (not in this build's log; PR #1328 at commit 97b051f is the change under test — inspect its CL/HIER / triggered-post changes for a missing task->progress assignment). Most recent schedule-triggered fix on record: c11c0ab "CORE: Fix asym mem for triggered tasks (#1026)" by nsarka.

File: src/schedule/ucc_schedule.c:101 (assertion) — root cause is the CL/HIER ucp triggered collective init that leaves task->progress == ucc_dummy_progress (see default set at src/schedule/ucc_schedule.c:121).

Suggested fix: In the CL/HIER (ucp) triggered-post code path exercised by PR #1328, ensure every task added to a schedule/queued for ucc_context_progress has its progress callback set to the real implementation before being posted. Reproduce locally with:
UCC_CLS=basic,hier UCC_CL_HIER_TUNE=inf UCC_CL_HIER_TLS=ucp UCC_TL_NCCL_TUNE=0 UCX_TLS=^cuda_ipc ucc_test_mpi -T --triggered 0 -c alltoall,alltoallv,allreduce,barrier and add an assert/guard in the triggered-post setup to catch tasks left with ucc_dummy_progress.

Related: none found via issue/PR search (queries for "ucc_dummy_progress assertion" and "triggered post progress schedule" returned no matches).

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-mpi · commit 3b4b5007

TL;DR: The CL/HIER+ucp allreduce MPI test aborted with ucc_schedule.c:101 Assertion 0 failed inside ucc_dummy_progress, meaning an allreduce task was scheduled without a real progress callback installed — a defect in the PR's new mixed-radix allreduce selector/init path.

Full analysis

Summary: Jenkins stage "Run UCC MPI tests (bulk)" (node 198) crashed with SIGABRT (exit 134 → pipeline 143) during the CL/HIER+ucp run of alltoall,alltoallv,allreduce,barrier.

Root cause: The abort comes from ucc_dummy_progress() calling ucc_assert_always(0) (src/schedule/ucc_schedule.c:101), reached via ucc_context_progress → the task's progress pointer. ucc_coll_task_init initializes task->progress = ucc_dummy_progress as a placeholder that must be overwritten by the real algorithm init (e.g. ucc_tl_ucp_allreduce_knomial_init_common sets task->super.progress = ucc_tl_ucp_allreduce_knomial_progress at allreduce_knomial.c:236). The task that crashed still had the dummy progress function, i.e. it was enqueued for progress without a valid algorithm handler. Given the branch (jmahou/ucc-mixed-radix-selector) adds a mixed-radix allreduce selector, the new selection path picks/creates an allreduce task whose init returns before assigning task->super.progress (or maps to an algorithm index that has no progress function), and the task is then posted and progressed. This is a code-logic bug introduced by the PR, not a timeout or infra issue (the run showed continuous activity right up to the abort — no hang).

Implicated commit: The PR head under test, [REDACTED:Hex High Entropy String] (branch jmahou/ucc-mixed-radix-selector, PR #1328). The crash-site files (ucc_schedule.c, allreduce_knomial.c) are unchanged on the base branch, so the regression is in this PR's new selector/init code.

File: Crash site: src/schedule/ucc_schedule.c:101 (ucc_dummy_progress). Root fix location: the PR's new mixed-radix allreduce init path in src/components/tl/ucp/allreduce/ — ensure it sets task->super.progress (and post/finalize) like ucc_tl_ucp_allreduce_knomial_init_common (allreduce_knomial.c:234-237).

Suggested fix: In the new mixed-radix allreduce init function, verify every code path assigns task->super.progress, task->super.post, and task->super.finalize before the task is returned/enqueued (mirror lines 234-237 of allreduce_knomial.c). Also confirm the new algorithm entry in ucc_tl_ucp_allreduce_algs[] and the selector string map to an init that installs a real progress callback for all radix/size cases (including the alltoall,alltoallv,allreduce,barrier CL/HIER+ucp mix that triggered it). Add an assertion/guard in init that progress != ucc_dummy_progress before enqueue to catch this earlier.

Related: none found (issue/PR search for "mixed radix selector allreduce" returned no matches in this repo).

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 0fe3d869-8a93-4ae2-a9f1-e4c43909d42b in the triage console for the audit trail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants