TL/UCP: enable host-side SRA allreduce pipelining

[wfaderhold21]

Overlap the reduce-scatter and allgather phases across fragments on the host path so fragment i's allgather (pure network) runs concurrently with fragment i+1's reduce-scatter (network + CPU reduction). Previously the host branch of the SRA-knomial pipeline heuristic disabled pipelining (n_frags=1), leaving the NIC idle during every reduce step and the CPU idle during the entire allgather.

The host else-branch of get_pipeline_params() now sets conservative defaults: threshold 256KB, 512KB fragments, nfrags floor 2, pdepth 2, parallel order. Env override UCC_TL_UCP_ALLREDUCE_SRA_KN_PIPELINE still takes precedence. Correctness is unchanged: the CUDA in-place path already runs the identical fragmented schedule, and per-frag scratch is sized from max_frag_count (smaller frags -> smaller scratch).

[svcnbu-swx-hpcx]

🤖 CI Triage Agent — ASAN · commit dbea8e26

TL;DR: The gtest test_schedule.pipelined_depth_16_exact_lifecycle failed because the PR's new depth-16 pipeline tests expect UCC_SCHEDULE_PIPELINED_MAX_FRAGS to be 16, but the header still defines it as 4, so ucc_schedule_pipelined_init clamps the depth to 4 and only 4 fragments/8 subscriptions are created instead of the expected 16/32. Bump UCC_SCHEDULE_PIPELINED_MAX_FRAGS to 16 (and size frags[] accordingly).

Full analysis

Summary: ASAN gtest run failed 1 test — test_schedule.pipelined_depth_16_exact_lifecycle — while the build itself succeeded.

Root cause: UCC_SCHEDULE_PIPELINED_MAX_FRAGS is still 4 in src/schedule/ucc_schedule_pipelined.h (line 14), and the frags[] array is sized to it (line 95). ucc_schedule_pipelined_init clamps any requested n_frags down to this max (ucc_schedule_pipelined.c lines 238-244). The PR's new depth-16 tests request 16 fragments and assert g_fault_frag_inits == 16 and g_fault_subscribe_attempt == 32 (test_schedule.cc lines 1102-1112), but the clamp produces only 4 fragments (8 subscribe attempts), so the EXPECT_EQ(16, ...) assertion fails. The test/source are inconsistent: the tests were written for MAX_FRAGS=16 but the constant wasn't raised.

Implicated commit: d56c68d — "TL/UCP: enable host-side SRA allreduce pipelining" (Ferrol Aderholdt) on branch topic/ar-sra-host-pipelining; the depth-16 tests were added without bumping the max-frags constant.

File: src/schedule/ucc_schedule_pipelined.h:14 (#define UCC_SCHEDULE_PIPELINED_MAX_FRAGS 4); array at :95; clamp at src/schedule/ucc_schedule_pipelined.c:238-244; failing assertions at test/gtest/core/test_schedule.cc:1111-1112.

Suggested fix: Change #define UCC_SCHEDULE_PIPELINED_MAX_FRAGS 4 to 16 in src/schedule/ucc_schedule_pipelined.h so the frags[] array can hold 16 entries and the init no longer clamps a depth-16 request. (Alternatively, if 16 is not intended, adjust the new depth-16 tests to match the real max — but the PR's purpose of "host-side SRA pipelining" and the pipelined_init_depth_16_success test comment "matches UCC_SCHEDULE_PIPELINED_MAX_FRAGS" indicate the constant should become 16.) Verify no static allocations depending on the old value of 4 overflow after the bump.

Related: PR #1354; prior schedule-pipeline fixes c8314d3 (#1262), 4f67436 (#1051).

[svcnbu-swx-hpcx]

🤖 CI Triage Agent — Build & Test · commit dbea8e26

TL;DR: Two new test_schedule gtests (pipelined_real_subscription_failure_first, pipelined_depth_16_exact_lifecycle) fail because the fault-injection harness installs process-global callbacks/counters in fault_reset() but never tears them down, so their exact-count assertions get corrupted by other tests in the same binary; fix by clearing the fault hooks and resetting counters in a gtest TearDown().

Full analysis

Summary: The gtest job (shard 0) failed: 2 of 7759 tests failed — test_schedule.pipelined_real_subscription_failure_first and test_schedule.pipelined_depth_16_exact_lifecycle. Build and all other stages passed.

Root cause: These tests use a fault-injection harness that registers process-global hooks and counters — ucc_event_manager_set_subscribe_fault_cb(fault_subscribe_cb) and ucc_schedule_pipelined_set_lock_observer(fault_lock_observe) (test/gtest/core/test_schedule.cc:1025-1026), plus static globals g_fault_subscribe_attempt, g_fault_subscribe_fail_at, g_fault_lock_inits/destroys, etc. (lines 927-936). They are set at the start of each fault test in fault_reset() but are never uninstalled/reset in a TearDown. Once installed, fault_subscribe_cb stays active for the rest of the single gtest process and fires for every subsequent ucc_event_manager_subscribe() call — including from unrelated test_schedule tests — incrementing the shared g_fault_subscribe_attempt counter (and potentially returning a spurious UCC_ERR_NO_MEMORY when a stale g_fault_subscribe_fail_at matches). The two failing tests are exactly the ones with the tightest exact-equality oracles (EXPECT_EQ(fail_at+1, g_fault_subscribe_attempt) == 1, and EXPECT_EQ(32, g_fault_subscribe_attempt)), so cross-test contamination / gtest's non-source execution order makes them fail while the looser fault tests pass. The ucc_schedule_pipelined_init/_finalize unwind logic itself (src/schedule/ucc_schedule_pipelined.c) is internally consistent with the expected counts, confirming the defect is the leaked global test state rather than the production code path.

Implicated commit: d56c68d1 (Ferrol Aderholdt, "TL/UCP: enable host-side SRA allreduce pipelining") which added this harness, refined by 67ae2ddd (ferrol aderholdt, "REVIEW: remove unnecessary comments") — both on PR #1354.

File: test/gtest/core/test_schedule.cc:1013-1027 (fault_reset installs hooks; no matching teardown) and the assertions at lines 1058, 1072-1076, 1111-1119.

Suggested fix: Make the fault harness self-contained per test. Add a TearDown() (or explicit cleanup at the end of each fault test / fault_expect_subscription_unwind) that unregisters the global hooks and resets counters, e.g.:

ucc_event_manager_set_subscribe_fault_cb(NULL);
ucc_schedule_pipelined_set_lock_observer(NULL);
g_fault_subscribe_fail_at = -1;
g_fault_subscribe_attempt = 0;

so no fault callback survives into other tests and each test starts from a clean, deterministic global state. Alternatively, gate fault_subscribe_cb so it is a no-op unless a fault test is actively running. This removes the order-dependence and makes the exact-count assertions reliable.

Related: PR #1354 (topic/ar-sra-host-pipelining); prior pipelined-schedule fixes for context — #1262 (task restart in ppln schedule) and #1051 (n_deps overflow in pipelined schedule).