CollectiveX: wire-basis bandwidth, nccl-ep LL hold, routed HT window, AsyncLL, DSv4-Pro workload - #2786
CollectiveX: wire-basis bandwidth, nccl-ep LL hold, routed HT window, AsyncLL, DSv4-Pro workload#2786Oseltamivir wants to merge 15 commits into
Conversation
…te definitions DeepEP/UCCL/NCCL low-latency kernels move one copy per (token, expert) assignment; normal mode and MoRI LL move rank-deduplicated (token, dest-rank) copies. byte_provenance always carried the smaller deduplicated numerator, so any bandwidth divided from it on a token-expert backend was a lower bound published as the wire rate (34.1% low on nccl-ep LL EP8 at T=128: 77.4 MB vs the 117.4 MB the kernels move) and not comparable across backends. Rows now carry wire_byte_provenance beside byte_provenance: identical for token-rank receives, per-assignment for token-expert ones, basis declared by the existing logical_copies.wire discriminator. bandwidth.py divides from the wire basis (falling back to the deduplicated one for pre-wire artifacts, which only ever understates).
…nce ships The T<=128 ladder clamp reduces exposure to the un-fenced shared-memory combine race (DeepEP #642's fence, absent at our pin and at NVIDIA master); the source has said all along it is not a safety boundary — lower rungs are less likely to hit the race, not immune. Green clamped rows are therefore not publication-valid, and describing T=256 as a capacity limit was wrong. Rows held in the registry, normal mode unaffected.
…d HT dispatch ncclEpUpdateHandle is documented as a per-step collective and production routing changes every MoE layer, so a serving step pays it before every HT dispatch at the handle's full token capacity. Excluding it (as NVIDIA's ep_bench does) made HT dispatch the one window that omitted its routing work while deepep-v2, uccl-ep, MoRI and FlashInfer all carry theirs per call. kernel_generation bumps to nccl-ep-ht-routed — the per-row discriminator the earlier NCCL measurement changes lacked.
…on deploys SGLang's moriep dispatcher maps low-latency to AsyncLL split-phase (dispatch_send/recv + combine_send/recv, block_num 64 rdma 32 warps 8) and asserts that kernel type; IntraNodeLL is not selected by any engine, so the previous rows measured an off-production path (kernel_generation intranode-ll discriminates them). The timed windows now run AsyncLL send+recv back-to-back and the adapter fails closed on wheels whose AsyncLL lacks the recv API — single-call AsyncLL returns before any payload lands. Pending on-metal validation on mi355x.
…d and the routed HT window
deepseek-v4-pro: hidden 7168, top-k 6, 384 routed experts — the dsv4 shape the rest of InferenceX benchmarks (operatorx testlists, HF-config-derived). The workload name is part of every case_id, so this is a clean identity break in the durable store: no v3 row can collide with a v4-pro row. Ladders and seed unchanged.
|
On-metal validation, both green: MoRI AsyncLL @ mi355x (run 33319409233, LL EP8 bf16+fp8): nccl-ep HT routed window @ h200 (run 33319407695, normal EP8 bf16 decode+prefill): App-side companion: SemiAnalysisAI/InferenceX-app#953 (reader divides rates from the wire basis; matrix flips nccl-ep LL cells to held). Note both validations ran under the deepseek-v3 workload (dispatched before the v4-pro commit); the first post-merge sweep populates the deepseek-v4-pro identities. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked whether the workload swap to topk=6 (from the old topk=8) invalidates the hardcoded COMBINE_REL_TOL = 8 * 2.0 ** -8 correctness-gate tolerance in ep_harness.py — it doesn't: the bound derives from topk (fewer stores with topk=6 only tightens the true error, so the topk=8-sized constant remains a valid, if slightly loose, upper bound).
Extended reasoning...
Confirmed findings already exist as inline comments for this PR (README.md still describing MoRI's old IntraNodeLL kernel, and methodology.md's byte-accounting section left describing only the old rank-deduplicated basis), so a full Overview/Security/Scrutiny/Other-factors writeup is not needed here per the findings-present path. I independently examined one additional candidate — whether the deepseek-v4-pro workload swap (topk 8 to 6) breaks the hardcoded COMBINE_REL_TOL tolerance in ep_harness.py, which was sized against topk=8 — and confirmed via the code comments and sweep.json that the bound is a valid (if now slightly loose) upper bound for topk=6, since the tree-summation/sequential accumulation error scales with topk and fewer stores only tightens the true error. This is not a correctness bug, so I'm recording it as ruled out rather than adding a new inline finding.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
experimental/CollectiveX/docs/methodology.md— The "Logical payload bandwidth is: logical_payload_bytes / measured_latency_seconds" section (methodology.md's authoritative English guide for the byte math) was not updated by this diff and still describes only the old rank-deduplicated basis, still citing MoRI's now-renamedIntraNodeLLas the dedup exemplar, with no mention ofwire_byte_provenanceor the fact that bandwidth.py now divides GB/s from the wire basis by default. A reader following this doc believes GB/s uses rank-deduplicated bytes, but the numbers now use per-assignment wire bytes for LL rows, understating nothing per se but making the doc actively wrong about which basis produced the published rate. …Extended reasoning...
…Fix: rewrite this section to describe the wire basis as the default numerator,
byte_provenanceas the pre-wire fallback, and drop/replace the staleIntraNodeLLreference.bandwidth.py's
_wire_bytes()(added by this diff) now preferswire_byte_provenanceand divides GB/s from per-assignment bytes for token-expert LL rows (e.g. nccl-ep LL), a materially different, larger numerator than thelogical_payload_bytesthis methodology.md section still describes as the sole basis. The PR description claims methodology.md was rewritten to describe the new wire-basis accounting, but this specific section (lines 449-464, under## Measurement) has zero diff hunks touching it -- confirmed viagit diff ... -- methodology.mdshowing no changes near these lines. A reader relying on this section (the repo's own authoritative English guide per AGENTS.md) to interpret bandwidth.py's GB/s figures, or to sanity-checklogical_copies.wire, gets a description of a basis the code no longer uses by default, and the doc still cites the retiredIntraNodeLLkernel as the MoRI LL example even…Verification: nit (documentation defect; no runtime/behavior failure). The claim is factually accurate. The four diff hunks in methodology.md are at @@ -19, @@ -87/88, @@ -210/211, and @@ -385/390 — none touch lines 449-471. That "Logical payload bandwidth" section is unchanged and still states the numerator is the rank-deduplicated logical count: line 449-451 "Logical payload bandwidth is:… | Severity: nit…
…th AsyncLL (review)
…sis (review) The Measurement section still described the rank-deduplicated count as the sole GB/s numerator and cited the retired IntraNodeLL as the dedup exemplar; bandwidth.py and the app now divide from wire_byte_provenance. The section now names both accountings, which rows differ, the 34% understatement that motivated the change, and the fallback behavior for pre-wire artifacts.
|
Both review findings are now addressed on the branch: the README MoRI row was synced to AsyncLL in 5d3b540, and 97944b4 rewrites methodology's "Logical payload bandwidth" section (the untouched lines ~449-464 the review flagged) around the wire basis — both accountings named, which rows differ, the 34% understatement, and the pre-wire fallback. All five test modules green after the main merge. Dispatching a full all-SKU sweep from this branch now; results will be posted here. |
The full v4-pro sweep's two b300 EP16 legs each drew one of these nodes and failed closed at the network-profile gate (rdma-port-5=inactive = the 5th selector, mlx5_4). Idle-pool survey: exactly these two nodes have mlx5_4 port state 1: DOWN; the other 12 idle nodes are 4: ACTIVE — a sick node pair (likely one leaf/cable pair), not a rail-wide outage. SRE handback owed.
… regression is resolved All gb300 deepep-v2 LL shards die at the first CUDA op after Buffer init with cudaErrorUnknown (context poisoned during nvshmem symmetric-heap init over MNNVL), on every rank, both degrees and precisions. Discrimination: gb300 normal mode green at n2/n4; gb200 LL green with the same image/pin/workload; a main-branch control with the old workload fails identically — so neither this PR nor the v4-pro shape. CI-shaped 2-node hand probe (job 27430 on the pool) reproduces: bootstrap and buffer creation complete, then every rank's next CUDA call returns unknown error. Driver is 580.159.03 (installed in the KV-fix window ~2026-08-26..28, after the last green gb300 LL run). Platform escalation owed; rows held so sweeps don't burn four known-red shards each.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e9ef099. Configure here.
…(mori >= #505) AsyncLL under the DSv4-Pro workload (topk 6) dies on every shipped mi35x-mori image: ROCm/mori#505 (AsyncLL slot assignment double-allocates when top-k does not divide warpSize, fixed upstream 2026-07-31) postdates them all. On-metal probe of this date-stamped nightly (job 41563, pure mori.ops): AsyncLL topk6@384 and topk8@256 both pass, split-phase recv API present. mi355x only — the -tw SKUs keep their image until separately validated on their docker path.
Full-SKU validation ledger (v4-pro workload, all fixes applied)Rerun 33356406487 ( Fixes validated on-metal in this rerun:
Both review findings were addressed earlier (README AsyncLL sync in 5d3b540; methodology wire-basis section rewrite in 97944b4). |
…un-breaks gb300 LL The gb300 LL cudaErrorUnknown was ours, not the platform's. 2x2 on the pool: stock in-image deep_ep (built on nvidia-nvshmem-cu13==3.4.5) runs LL clean on the exact r01 nodes and driver (580.159.03) where the CI stack dies; the CI venv's nvidia-nvshmem-cu12==3.3.9 r12 host library survives sm90/sm100 but poisons the CUDA context during symmetric-heap init over MNNVL on sm103 — buffer creation returns, the next CUDA call on every rank fails. The earlier driver-window attribution was a coincidence of timing (pin bump and driver install overlapped; hold commit e9ef099 superseded). The wheel spec now lives in common.sh (COLLX_DEEPEP_V2_NVSHMEM_SPEC), is installed by prepare_backend, and keys the venv cache so every NVIDIA deepep venv rebuilds once against cu13. gb300 ll_backends restored.
Root cause, finally isolated by holding everything else constant: a venv identical to prepare_backend's recipe (same DeepEP pin, same nccl-cu13 2.30.4, same nvshmem-cu13 3.4.5) is green with torch==2.11.0+cu130 and red with torch==2.10.0+cu130 on the same gb300 r01 nodes (jobs 27760 vs 27430/27757). torch 2.10.0's bundled CUDA-13 userland poisons the CUDA context during nvshmem symmetric-heap init over MNNVL on sm103 + driver 580.159.03; sm90 and sm100 are unaffected. Falsified along the way, in order: the v4-pro workload (main control), the driver alone (stock image green on the failing nodes — the user's r03 canary forced that 2x2), the nvshmem cu12 wheel (cu13 rebuild still red), the DeepEP pin (old-pin venv red). 2.11.0 is what the cu130 image ships. The torch spec joins the nvshmem spec in the venv cache key, so every deepep venv rebuilds once.
The DeepEP build compiles every object with the right -gencode (setup.py 'Arch list: 10.3', all compile lines sm_103), but the final nvcc -dlink step receives NO gencode and falls back to nvcc's CUDA-13 default, sm_75 — the .so's single linked device image is sm_75 and its kernels can never load on the target GPU. On gb300/sm103 that surfaced as cudaErrorUnknown at the first legacy-kernel launch (LL and layout paths; ElasticBuffer JIT paths were unaffected, which is why normal mode stayed green). Proven by instrumented runs of prepare_backend's own build (job 27792: compile lines sm_103, step 9/9 -dlink bare, product sm_75) after hand builds of the identical recipe produced sm_103 and ran LL clean on the failing nodes. Fix: NVCC_PREPEND_FLAGS carries -gencode=arch=compute_<arch>,code=sm_<arch> into every nvcc invocation including the dlink. COLLX_DEEPEP_V2_BUILD_GEN (dlarch1) joins the cache key so the sm_75 venvs with valid .ready markers rebuild instead of being reused. The torch 2.10->2.11 bump (previous commit) stays: 2.11 matches the image and the 2.10-vs-2.11 A/B was run against differently-linked binaries, so its verdict is voided rather than reversed; the pin question can be revisited upstream of this fix if anyone cares to re-test.
gb300 low-latency: root-caused, fixed, fleet-validatedRoot cause: DeepEP builds with Theories falsified on the way, each by one experiment: the v4-pro workload (main control failed identically), the driver (stock green on failing nodes, same 580.159.03), the nvshmem cu12 wheel (cu13 rebuild still red), the DeepEP pin (old-pin venv red too). The interim torch 2.10→2.11 verdict was confounded (hand builds took a different link path that preserved the arch) and is voided; the 2.11 pin stays because it matches the image. Fix (commits on this branch): Validation:
gb300 |
1 participant
Five measurement-integrity fixes, one commit each:
wire_byte_provenance): DeepEP/UCCL/NCCL LL kernels move one copy per (token, expert);byte_provenancealways carried the rank-deduplicated numerator, so LL "payload bandwidth" was a lower bound published as the wire rate (34.1% low on nccl-ep LL EP8 @t=128: 77.4 vs 117.4 MB) and not cross-backend comparable. Rows now carry both bases;bandwidth.pydivides from the wire basis (fallback only understates). App-side consumption fix goes to InferenceX-app (packages/db/src/collectivex/reader.tsmaps onlybyte_provenancetoday).ll_backendsuntil a fenced wheel ships. Methodology stops describing T=256 as a capacity limit.ncclEpUpdateHandlein the timed dispatch: it is documented as a per-step collective and production routing changes every MoE layer; excluding it made HT the one window omitting its routing work while deepep-v2/uccl-ep/MoRI/FlashInfer all carry theirs per call.kernel_generation→nccl-ep-ht-routed(the per-row discriminator earlier NCCL measurement changes lacked).kernel_generationintranode-lldiscriminates them). Fails closed on wheels lacking the recv API — single-call AsyncLL returns before payload lands.deepseek-v4-pro: hidden 7168, top-k 6, 384 routed experts (the dsv4 shape the rest of InferenceX benchmarks). Workload name is in every case_id — clean identity break in the durable store.Tests: all five CollectiveX test modules green; matrix regenerates 308 cases, all v4-pro, nccl LL absent, mori LL present. On-metal validation of (3) and (4) in flight on this branch: nccl-ep normal @h200 run 33319407695, mori low-latency @mi355x run 33319409233 — results will be posted here.
Note
Medium Risk
Changes alter published latency/bandwidth contracts, disable nccl-ep LL fleet-wide, and change DeepEP build pins—high impact for benchmark comparability but scoped to experimental CollectiveX harness/runtime.
Overview
CollectiveX tightens measurement integrity across EP backends: artifacts now record
wire_byte_provenancealongside rank-deduplicatedbyte_provenance, andbandwidth.pydivides GB/s from the wire basis so low-latency kernels that move one copy per (token, expert) are not understated (~34% on nccl-ep LL). Docs and report headers switch from “logical payload” to wire-basis semantics, with fallback only as a lower bound.NCCL EP: high-throughput dispatch includes
ncclEpUpdateHandlein the timed window (kernel_generationnccl-ep-ht-routed); low-latency is removed fromll_backendsfleet-wide until a fenced wheel ships (documented race, not a safe T≤128 clamp). MoRI low-latency now benchmarksAsyncLLwith split-phase send+recv (production/SGLang path), notIntraNodeLL, with fail-closed if recv APIs are missing.The sweep workload becomes
deepseek-v4-pro(top-k 6, 384 routed experts). DeepEP V2 build/cache pins cu13 NVSHMEM, torch 2.11, and NVCC dlink arch viaNVCC_PREPEND_FLAGS; mi355x image and b300 node excludes update inplatform_config.json. Tests add wire-basis preference for bandwidth fits.Reviewed by Cursor Bugbot for commit b8b7467. Bugbot is set up for automated code reviews on this repo. Configure here.