Skip to content

Commit 17ad637

Browse files
authored
CollectiveX: one nccl-ep handle per group, and restore the low-latency rows (#2407)
1 parent f749f44 commit 17ad637

4 files changed

Lines changed: 261 additions & 32 deletions

File tree

experimental/CollectiveX/bench/ep_nccl.py

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,9 @@ def __init__(self, args, rank, world_size, local_rank, device):
114114
self._combine_cfg = CombineConfig(send_only=0)
115115
self._comm = None
116116
self._ep_group = None
117+
# Exactly ONE handle per group, rebound per problem shape — see _ensure_handle.
118+
self._handle = None
119+
self._bound = None
117120

118121
def buffer_cap(self, args):
119122
if self._ll:
@@ -238,15 +241,34 @@ def create_buffer(self, spec):
238241
self._ht_disp_counts_t = self._t(self._ht_disp_counts)
239242

240243
def _ensure_handle(self, p):
241-
"""Create (once, cached on the problem) the reusable per-step handle for p's routing.
242-
243-
create_handle is collective and, in HT, performs the metadata exchange that fixes the
244-
received-token count — so it must run in the same order on every rank. It first runs
245-
inside Pass 1's untimed warm (ladder order, identical across ranks); the timed passes
246-
then reuse the cached handle and never enter a collective here.
244+
"""Bind the group's single handle to p's routing, creating it on first use.
245+
246+
ONE handle per group, rebound per shape — never one handle per shape. `buffer_idx`, the
247+
LL double-buffer parity selector, is per-HANDLE state, but the buffers it selects are
248+
offsets into the per-GROUP rdma_buffer: two handles built from the same group config
249+
resolve to the SAME parity-0/parity-1 count+flag slots and advance their parity
250+
independently, so one handle's "next buffer, safe to clean" is the other's "current
251+
buffer, in flight". Interleaving handles therefore corrupts the signalling even when it
252+
does not hang outright, which makes every latency drawn from such a run suspect. Filed
253+
upstream as NVIDIA/nccl#2303; reproduced on a stock wheel by ladder [1] (one handle,
254+
clean) vs ladder [1, 2] (two handles, 64 dispatch + 6 combine receive timeouts -> 719).
255+
256+
`ncclEpInitHandle` takes no token count and `ncclEpUpdateHandle` is documented as a
257+
"per-step collective: prepare the handle for the given top-k routing decisions", so
258+
rebinding IS the intended lifecycle. This also matches the other three backends, which
259+
each allocate one object sized to the ladder maximum and vary the token count per call.
260+
261+
Both create_handle and update are collective, and HT additionally performs the metadata
262+
exchange that fixes the received-token count, so they must run in the same order on
263+
every rank. They only ever run on a shape CHANGE, and every timed component is preceded
264+
by an untimed warm() on its own problem — so the collective always lands in warm (or in
265+
the oracle passes), never inside a timed window. Re-entering with the already-bound
266+
problem returns immediately without a collective or a sync.
247267
"""
248268
cached = getattr(p, "_nccl", None)
249269
if cached is not None:
270+
if self._bound is not cached:
271+
self._rebind(cached)
250272
return cached
251273
stream = self._stream()
252274
topk_idx_t = self._t(p.topk_idx)
@@ -271,18 +293,45 @@ def _ensure_handle(self, p):
271293
expert_counters=self._t(h.recv_experts),
272294
recv_total_counter=self._t(h.recv_total),
273295
)
274-
h.handle = self._ep_group.create_handle(
275-
self._layout,
276-
topk_idx_t,
277-
layout_info=ht_layout_info,
278-
config=HandleConfig(),
279-
stream=stream,
296+
# LL takes layout_info only on dispatch (the API forbids it on create/update); HT needs
297+
# it here so this problem's counters receive its own metadata-exchange results.
298+
h.layout_info = ht_layout_info
299+
if self._handle is None:
300+
self._handle = self._ep_group.create_handle(
301+
self._layout,
302+
topk_idx_t,
303+
layout_info=ht_layout_info,
304+
config=HandleConfig(),
305+
stream=stream,
306+
)
307+
h.handle = self._handle
308+
torch.cuda.synchronize()
309+
if not self._ll:
310+
h.count = int(h.recv_total.item())
311+
self._bound = h
312+
else:
313+
h.handle = self._handle
314+
self._rebind(h)
315+
p._nccl = h
316+
return h
317+
318+
def _rebind(self, h):
319+
"""Point the single handle at h's routing (collective; untimed callers only).
320+
321+
Rebinding does not reallocate: `Handle.update` only swaps in the new top-k indices.
322+
HT re-reads its received-token count because the metadata exchange recomputes it for
323+
this routing; the value is deterministic per problem, so a later rebind to the same
324+
problem reproduces it.
325+
"""
326+
self._handle.update(
327+
h.topk_idx_t,
328+
layout_info=None if self._ll else h.layout_info,
329+
stream=self._stream(),
280330
)
281331
torch.cuda.synchronize()
282332
if not self._ll:
283333
h.count = int(h.recv_total.item())
284-
p._nccl = h
285-
return h
334+
self._bound = h
286335

287336
# ---- transport contract ------------------------------------------------------------------
288337

@@ -464,8 +513,11 @@ def finalize(self, rc):
464513
return rc
465514

466515
def _destroy_handles(self):
467-
# Per-problem handles are cached on the problem namespaces; the group keeps no registry,
468-
# so there is nothing to walk here. Handles are released when their problems are GC'd
469-
# (Handle.destroy runs in the binding's __del__); the group/comm destroy below reclaims
470-
# the device buffers. Kept as a seam in case bring-up needs explicit handle teardown.
471-
return
516+
# One handle for the whole group, so teardown is a single explicit destroy rather than
517+
# waiting on per-problem GC. The problem namespaces still hold a reference to it for
518+
# their dispatch/combine calls; dropping _bound first keeps a late rebind from touching
519+
# a destroyed handle. The group/comm destroy below reclaims the device buffers.
520+
self._bound = None
521+
if self._handle is not None:
522+
self._handle.destroy()
523+
self._handle = None

experimental/CollectiveX/configs/platform_config.json

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"scale_up_transport": "nvlink",
1111
"launcher": "single-slurm",
1212
"backends": {"deepep-v2": [8, 16], "uccl-ep": [8], "nccl-ep": [8]},
13-
"ll_backends": {"deepep-v2": [8], "uccl-ep": [8]},
13+
"ll_backends": {"deepep-v2": [8], "uccl-ep": [8], "nccl-ep": [8]},
1414
"fabric": {"nic": "ConnectX-7 2x200GbE", "switch": "Arista 7060DX5-64S (Tomahawk4, 25.6T)"},
1515
"operator": {
1616
"partition": "hpc-gpu-1",
@@ -34,7 +34,7 @@
3434
"scale_up_transport": "nvlink",
3535
"launcher": "single-slurm",
3636
"backends": {"deepep-v2": [8, 16], "uccl-ep": [8], "nccl-ep": [8]},
37-
"ll_backends": {"deepep-v2": [8], "uccl-ep": [8]},
37+
"ll_backends": {"deepep-v2": [8], "uccl-ep": [8], "nccl-ep": [8]},
3838
"fabric": {"nic": "ConnectX-7 400G", "switch": "NVIDIA Quantum-2 QM9790 (25.6T, InfiniBand)"},
3939
"operator": {
4040
"partition": "main",
@@ -54,7 +54,7 @@
5454
"scale_up_transport": "nvlink",
5555
"launcher": "single-slurm",
5656
"backends": {"deepep-v2": [8, 16], "uccl-ep": [8], "nccl-ep": [8]},
57-
"ll_backends": {"deepep-v2": [8], "uccl-ep": [8]},
57+
"ll_backends": {"deepep-v2": [8], "uccl-ep": [8], "nccl-ep": [8]},
5858
"fabric": {"nic": "ConnectX-7 400GbE", "switch": "Whitebox Tomahawk3 leaf + Tomahawk4 (RoCE)"},
5959
"operator": {
6060
"partition": "gpu-2",
@@ -77,6 +77,7 @@
7777
"scale_up_transport": "nvlink",
7878
"launcher": "single-slurm",
7979
"backends": {"deepep-v2": [8, 16], "nccl-ep": [8]},
80+
"ll_backends": {"nccl-ep": [8]},
8081
"fabric": {"nic": "ConnectX-8 2x400GbE", "switch": "NVIDIA Spectrum-X SN5600 (51.2T)"},
8182
"operator": {
8283
"partition": "batch_1",
@@ -101,6 +102,7 @@
101102
"scale_up_transport": "mnnvl",
102103
"launcher": "gb-nv",
103104
"backends": {"deepep-v2": [8, 16], "nccl-ep": [8, 16]},
105+
"ll_backends": {"nccl-ep": [8]},
104106
"fabric": {"nic": "MNNVL (scale-out not used)", "switch": "NVLink NVL72"},
105107
"operator": {
106108
"partition": "batch",
@@ -120,6 +122,7 @@
120122
"scale_up_transport": "mnnvl",
121123
"launcher": "gb-nv",
122124
"backends": {"deepep-v2": [8, 16], "nccl-ep": [8, 16]},
125+
"ll_backends": {"nccl-ep": [8]},
123126
"fabric": {"nic": "MNNVL (scale-out not used)", "switch": "NVLink NVL72"},
124127
"operator": {
125128
"partition": "batch_1",
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
#!/usr/bin/env python3
2+
"""Torch-free tests for the nccl-ep single-handle contract.
3+
4+
The invariant under test: ONE handle per group, rebound per problem shape. Two handles built
5+
from the same group config resolve to the same LL parity signal slots while advancing their
6+
parity independently, which corrupts signalling (NVIDIA/nccl#2303) -- so a regression that
7+
reintroduces per-shape handles must fail loudly here rather than on a cluster.
8+
9+
torch and nccl are stubbed so this runs without the benchmark image.
10+
"""
11+
from __future__ import annotations
12+
13+
import sys
14+
import types
15+
import unittest
16+
from pathlib import Path
17+
from unittest import mock
18+
19+
ROOT = Path(__file__).resolve().parents[1]
20+
21+
22+
def _stub_modules():
23+
"""Fake torch / nccl modules so `import ep_nccl` succeeds without the benchmark image."""
24+
torch = types.ModuleType("torch")
25+
torch.bfloat16 = "bfloat16"
26+
torch.int32 = "int32"
27+
torch.empty = lambda *a, **k: types.SimpleNamespace(shape=a[0] if a else ())
28+
torch.zeros = lambda *a, **k: types.SimpleNamespace(item=lambda: 7)
29+
torch.cuda = types.SimpleNamespace(synchronize=lambda: None)
30+
dist = types.ModuleType("torch.distributed")
31+
torch.distributed = dist
32+
33+
ep = types.ModuleType("nccl.ep")
34+
for name in (
35+
"Algorithm", "CombineConfig", "CombineInputs", "CombineOutputs", "DispatchConfig",
36+
"DispatchInputs", "DispatchOutputs", "GroupConfig", "HandleConfig", "Layout",
37+
"LayoutInfo", "Tensor",
38+
):
39+
setattr(ep, name, type(name, (), {"__init__": lambda self, *a, **k: None}))
40+
ep.Algorithm = types.SimpleNamespace(LOW_LATENCY="LL", HIGH_THROUGHPUT="HT")
41+
ep.Layout = types.SimpleNamespace(EXPERT_MAJOR="EM", FLAT="FLAT")
42+
core = types.ModuleType("nccl.core")
43+
pkg = types.ModuleType("nccl")
44+
pkg.ep, pkg.core = ep, core
45+
return {
46+
"torch": torch, "torch.distributed": dist,
47+
"nccl": pkg, "nccl.ep": ep, "nccl.core": core,
48+
}
49+
50+
51+
sys.path[:0] = [str(ROOT), str(ROOT / "bench")]
52+
53+
# Import ep_nccl against the stubs, then withdraw them: leaving a fake torch in sys.modules
54+
# makes the genuinely torch-dependent modules in this process (test_runtime, test_ll_oracle)
55+
# error instead of skipping. Dropping ep_nccl too keeps the stub-built module private to us.
56+
with mock.patch.dict(sys.modules, _stub_modules()):
57+
import ep_nccl # noqa: E402
58+
59+
sys.modules.pop("ep_nccl", None)
60+
61+
62+
class FakeHandle:
63+
"""Records every rebind so the tests can assert on the collective call pattern."""
64+
65+
def __init__(self):
66+
self.updates = []
67+
self.destroyed = False
68+
69+
def update(self, topk_idx, *, layout_info=None, stream=None):
70+
self.updates.append((topk_idx, layout_info))
71+
72+
def destroy(self):
73+
self.destroyed = True
74+
75+
76+
class FakeGroup:
77+
def __init__(self):
78+
self.created = 0
79+
self.handle = FakeHandle()
80+
81+
def create_handle(self, layout, topk_idx, *, layout_info=None, config=None, stream=None):
82+
self.created += 1
83+
return self.handle
84+
85+
86+
def backend(ll=True):
87+
"""An NCCLEPBackend with just the fields _ensure_handle touches (no __init__, no GPU)."""
88+
b = object.__new__(ep_nccl.NCCLEPBackend)
89+
b._ll = ll
90+
b._layout = "EM" if ll else "FLAT"
91+
b._handle = None
92+
b._bound = None
93+
b._ep_group = FakeGroup()
94+
b.device = "cuda:0"
95+
b.num_local_experts = 4
96+
b.args = types.SimpleNamespace(hidden=16)
97+
b._t = lambda x: x
98+
b._stream = lambda: 0
99+
return b
100+
101+
102+
def problem(T):
103+
return types.SimpleNamespace(
104+
T=T, dispatch_x=f"x{T}", topk_idx=f"idx{T}", topk_weights=f"w{T}"
105+
)
106+
107+
108+
class TestSingleHandle(unittest.TestCase):
109+
def test_one_handle_across_many_shapes(self):
110+
"""Nine ladder rungs must still produce exactly one create_handle."""
111+
b = backend()
112+
for T in (1, 2, 4, 8, 16, 32, 64, 128, 256):
113+
b._ensure_handle(problem(T))
114+
self.assertEqual(b._ep_group.created, 1)
115+
116+
def test_shape_change_rebinds_and_repeat_does_not(self):
117+
"""update() on a shape switch; no collective when the bound shape is re-entered."""
118+
b = backend()
119+
pa, pb = problem(1), problem(2)
120+
b._ensure_handle(pa)
121+
self.assertEqual(len(b._ep_group.handle.updates), 0) # first bind is the create
122+
123+
b._ensure_handle(pb)
124+
self.assertEqual(len(b._ep_group.handle.updates), 1)
125+
126+
# Re-entering the bound problem repeatedly -- the timed loop's steady state -- must not
127+
# enter a collective, otherwise every iteration gains a rank-synchronising step.
128+
for _ in range(8):
129+
b._ensure_handle(pb)
130+
self.assertEqual(len(b._ep_group.handle.updates), 1)
131+
132+
# Returning to an earlier shape rebinds again (its cached namespace is reused).
133+
b._ensure_handle(pa)
134+
self.assertEqual(len(b._ep_group.handle.updates), 2)
135+
136+
def test_every_problem_shares_the_one_handle(self):
137+
b = backend()
138+
handles = {id(b._ensure_handle(problem(T)).handle) for T in (1, 2, 4)}
139+
self.assertEqual(len(handles), 1)
140+
self.assertIs(b._ensure_handle(problem(1)).handle, b._handle)
141+
142+
def test_ll_never_passes_layout_info_on_rebind(self):
143+
"""The API forbids layout_info on create/update in LL mode."""
144+
b = backend(ll=True)
145+
b._ensure_handle(problem(1))
146+
b._ensure_handle(problem(2))
147+
self.assertEqual([info for _, info in b._ep_group.handle.updates], [None])
148+
149+
def test_ht_rebind_carries_that_problems_counters(self):
150+
"""HT re-runs the metadata exchange into the rebound problem's own counter tensors."""
151+
b = backend(ll=False)
152+
ha = b._ensure_handle(problem(1))
153+
hb = b._ensure_handle(problem(2))
154+
self.assertEqual(len(b._ep_group.handle.updates), 1)
155+
self.assertIs(b._ep_group.handle.updates[0][1], hb.layout_info)
156+
self.assertIsNot(ha.layout_info, hb.layout_info)
157+
self.assertEqual(hb.count, 7) # re-read after the exchange
158+
159+
def test_destroy_releases_the_handle_once(self):
160+
b = backend()
161+
b._ensure_handle(problem(1))
162+
handle = b._handle
163+
b._destroy_handles()
164+
self.assertTrue(handle.destroyed)
165+
self.assertIsNone(b._handle)
166+
self.assertIsNone(b._bound)
167+
b._destroy_handles() # idempotent
168+
169+
170+
if __name__ == "__main__":
171+
unittest.main()

experimental/CollectiveX/tests/test_matrix.py

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -213,14 +213,15 @@ def test_nccl_ep_rollout_shape(self):
213213
# both stay inside the 72-GPU scale-up domain (world <= scale_up_domain => LSA, no GIN),
214214
# so EP16 works over MNNVL where the RDMA-GIN path walls.
215215
# * AMD SKUs: no rows (NCCL EP is NVIDIA-only; AMD runs mori).
216-
# * LOW-LATENCY: no rows on ANY SKU. The wheel's LL count/flag protocol consumes stale
217-
# double-buffer signals (values carry no generation, so a signal from two calls earlier is
218-
# bit-identical at a repeating workload); a rank that slips one parity cycle gets lapped and
219-
# the pipeline wedges on dispatch/combine receive timeouts, ending in cudaErrorLaunchFailure.
220-
# Reported as NVIDIA/nccl#2303, fixed by NVIDIA/nccl#2306, unfixable here (we install the
221-
# published wheel). Observed first on GB, then reproduced on every x86 SKU — 5/5 over SSH and
222-
# 4/4 in sweep 30155842613 — so ll_backends carries no nccl-ep row anywhere until a fixed
223-
# wheel ships. Restore per-SKU rows only alongside a spec bump that contains the fix.
216+
# * LOW-LATENCY: EP8 on all six NVIDIA SKUs. These rows were dropped while every LL leg
217+
# wedged, on the reading that only a fixed wheel could restore them. That reading was
218+
# wrong about the cause: the wedge needed TWO LL handles live on one group. `buffer_idx`
219+
# is per-handle but its buffers are offsets into the per-group rdma_buffer, so same-config
220+
# handles alias one another's parity count/flag slots. The adapter now binds a single
221+
# handle and rebinds it per shape, which removes the aliasing without a wheel bump —
222+
# proven on a stock wheel by ladder [1] (one handle, clean) vs [1, 2] (two handles,
223+
# 64 dispatch + 6 combine receive timeouts). LL is decode-only and EP8-only: GB stays at
224+
# EP8 here even though normal mode runs EP16, because LL adds no EP16 row on any SKU.
224225
# BF16 only — no FP8 case (NCCL EP FP8 unsupported this release).
225226
document = matrix(backend="all")
226227
runnable = {
@@ -255,14 +256,16 @@ def test_nccl_ep_rollout_shape(self):
255256
},
256257
{"bf16"},
257258
)
258-
# Low-latency: no nccl-ep case on any SKU while the wheel carries the stale-signal wedge.
259+
# Low-latency: EP8 on every NVIDIA SKU, and EP8 only — a stray EP16 LL row would dispatch a
260+
# shape the mode does not define.
259261
ll = {
260262
(item["sku"], item["case"]["ep"])
261263
for item in document["requested_cases"]
262264
if item["case"]["backend"] == "nccl-ep"
263265
and item["case"]["mode"] == "low-latency"
266+
and item["disposition"] == "runnable"
264267
}
265-
self.assertEqual(ll, set())
268+
self.assertEqual(ll, {(sku, 8) for sku in rdma_skus | gb_skus})
266269

267270
def test_invalid_filters_fail_closed(self):
268271
for options in (

0 commit comments

Comments
 (0)