Skip to content

cuda: COLI_CUDA_RESID=1 — expert-group results stay on device (#431 PR-C0) - #434

Merged
JustVugg merged 5 commits into
JustVugg:devfrom
ZacharyZcR:feat/cuda-resident-accum
Jul 21, 2026
Merged

cuda: COLI_CUDA_RESID=1 — expert-group results stay on device (#431 PR-C0)#434
JustVugg merged 5 commits into
JustVugg:devfrom
ZacharyZcR:feat/cuda-resident-accum

Conversation

@ZacharyZcR

@ZacharyZcR ZacharyZcR commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Third increment of #431. Stacked on #432 (shares the branch history — review the top commit).

What

Decode-time (S=1) expert groups without the host round-trip. Today each VRAM-tier group costs: host gather-memcpy, H2D, kernels, D2H of every expert row, a per-device sync, and a host weighted accumulation — per device, per layer. Under COLI_CUDA_RESID=1:

  • issue — input row P2P from the layer's home device → broadcast → grouped-W4 kernels → per-expert weighting + fixed-order reduce on the group's device → partial sum peer-pushed into a per-issue slot on the home device → event. Fully async: zero host bytes, zero syncs.
  • take — after moe() returns, the home legacy stream waits every issue event and reduces the slots in issue order (deterministic, no atomics), joining the residual exactly where the routed upload used to.

The CPU tier overlaps with the issued GPU work as before — this subsumes the overlap #342 was after, with the take phase collapsed into a stream dependency. A failed take would silently drop routed experts (a wrong answer, not a slow one) → fatal by design; a failed issue falls back to the synchronous path per device. Buffers reserve at the 64-expert cap so a re-issue never reallocs under a still-queued stream.

Measured — all three operating points, wall-clock parity everywhere

6× RTX 5090, ROUTER=1 TC_W4A16=1 TEMP=0 DRAFT=0, same binary, flag-only:

A/B hit RESID=0 RESID=1
96-token cold process 94.1% 1.30 tok/s 1.29 tok/s
256-token long run 96.8% 1.89 tok/s 1.94 tok/s
champion domain (full pin, disk-0) 100% 6.64 tok/s 6.54 tok/s
256-token greedy text byte-identical (despite the accumulation-order change)

The matured-cache warm A/B this PR was gated on has now run — and it closed the question honestly: parity even at hit 100%. The profiler's summed API time was a mirage; with COLI_CUDA_PIPE=2 the choreography already overlaps the critical path (full analysis in #431). This PR is therefore not a speed change and does not claim one.

Why merge it, then

  • It is the dataflow a single-graph decode requires: a CUDA Graph cannot contain a host-side accumulation. Every future graph-capture attempt needs exactly this issue/take split.
  • It subsumes the CPU/GPU overlap cuda: COLI_GROUP_ASYNC=1 — overlap the CPU expert rows with the GPU groups at decode (opt-in, +6-8%) #342 was after, with less machinery (a stream dependency instead of a worker handshake).
  • Zero host bytes and zero per-device syncs per layer is the structurally right shape regardless of today's bottleneck — the current parity says the wall is elsewhere (disk + CPU tier), not that the shape is wrong.
  • Off by default (COLI_CUDA_RESID=1 opt-in), byte-identical output, and a fail-fast take path — no behavior change unless asked for.

make check 77/77; CUDA build zero warnings; oracle from #432 unaffected.

@ZacharyZcR

Copy link
Copy Markdown
Contributor Author

Un-draft gate result: champion-regime A/B (1,024-token, usage-snapshot-controlled, late segment at hit 99.1%) measured 5.07 vs 5.11 tok/s — parity, gate not cleared. Staying draft. Full analysis and the roadmap pivot in #431 (comment) — the choreography this PR removes turns out to sit off the critical path on the 6-GPU pipe2 topology at every reachable operating point. The dataflow remains the graph-correct one; parking it until something puts the GPU side back on the critical path.

…gg#431 PR-C0)

Decode-time (S=1) expert groups without the host round-trip. Today each
VRAM-tier group costs: host gather-memcpy of the input row, H2D, grouped
kernels, D2H of every expert row, a per-device stream sync, and a host
weighted accumulation — per device, per layer. Under this flag:

  issue  input row P2P from the layer's home device -> broadcast ->
         grouped-W4 kernels -> per-expert weighting and a fixed-order
         reduce ON the group's device -> partial sum peer-pushed into a
         per-issue slot on the home device -> event. Fully async: zero
         host bytes, zero syncs.
  take   after moe() returns, the home legacy stream waits every issue
         event and reduces the slots in issue order (deterministic, no
         atomics), then the accumulator joins the residual exactly where
         the routed upload used to.

The CPU tier overlaps with the issued GPU work through moe() as before —
this subsumes the overlap JustVugg#342 was after, with the take phase collapsed
into a stream dependency instead of a collection pass. A failed take
would silently drop routed experts (a wrong answer, not a slow one), so
it is fatal by design; a failed issue falls back to the synchronous path
per device. Buffers reserve at the 64-expert cap so re-issue never
reallocs under a still-queued stream.

Measured (6x RTX 5090, ROUTER=1 TC_W4A16=1, TEMP=0 DRAFT=0): parity at
every operating point reachable in a cold-start A/B (96-token: 74.0 vs
74.3 s; 256-token warm segment t=128..256: 2.91 vs 2.89 tok/s) — at hit
94-97% the choreography this removes hides behind ~350-700 MB/token of
disk misses. 256-token greedy text is byte-identical across the flag
despite the accumulation-order change. The champion regime (hit 99-100%,
where the removed syncs sit on the critical path) needs a matured-cache
A/B and is the acceptance gate for un-drafting; either way this is the
dataflow a single-graph decode (JustVugg#431 PR-C) requires — a graph cannot
contain a host-side accumulation.
@ZacharyZcR
ZacharyZcR force-pushed the feat/cuda-resident-accum branch from 87c8369 to 0f8cbe2 Compare July 20, 2026 16:54
@ZacharyZcR

Copy link
Copy Markdown
Contributor Author

Rebased onto current dev after the merge wave. Two things needed resolving:

  1. Symbol collision with the now-merged cuda: COLI_GROUP_ASYNC=1 — overlap the CPU expert rows with the GPU groups at decode (opt-in, +6-8%) #342: that PR's overlap path took the names coli_cuda_expert_group_issue/_take with different semantics (take returns a host pointer; mine reduces on-device and returns nothing). Mine are renamed to coli_cuda_expert_group_resident_issue/_resident_take, which reads better anyway.
  2. Both async paths coexist in moe(): cuda: COLI_GROUP_ASYNC=1 — overlap the CPU expert rows with the GPU groups at decode (opt-in, +6-8%) #342's COLI_GROUP_ASYNC block runs first, then the COLI_CUDA_RESID block, then the synchronous fallback under cuda: COLI_GROUP_ASYNC=1 — overlap the CPU expert rows with the GPU groups at decode (opt-in, +6-8%) #342's if(!async_done) guard. Each flag is independently opt-in and default-off, so no behaviour changes unless one is set.

make check 105/105, zero warnings. Still draft — its un-draft gate (a champion-regime win) has not cleared: with the disk residue gone after #445, the choreography this removes measured parity at every operating point I can reach (analysis in #431). Happy to keep it parked, close it, or land it as dataflow groundwork — your call. The #451 grouped-kernel plumbing it was partly built for is already merged.

…Windows CI)

The rebase that renamed my PR-C0 entries to *_resident_* left two stubs in
backend_loader.c with the new function name but the OLD field name in the
availability guard, so the resident_issue stub kept the plain name and
collided with JustVugg#342's coli_cuda_expert_group_issue on the MinGW CUDA-DLL
build (conflicting types — JustVugg#342's take returns a host pointer, mine reduces
on-device). Name and guard field now agree on the _resident_ variant in both
stubs. MinGW syntax check clean; make check 105/105.
@ZacharyZcR
ZacharyZcR marked this pull request as ready for review July 21, 2026 02:30
@JustVugg

Copy link
Copy Markdown
Owner

To merge this I need validation data I can't produce myself (no such GPU here). The bar every backend holds: (1) correctness first — a teacher-forcing token-exact check on a tiny model (SNAP=<tiny> TF=1 ./coli ... → N/32 vs the CPU oracle), or at minimum a clearly-coherent chat transcript; (2) perf — before/after tok/s on the same prompt + your GPU/driver/runtime version. Rebase on current dev (CI is green), get CI passing, post that data, and I merge. Correctness matters more than speed — #298 was a GPU path that looked right and produced garbage.

For COLI_CUDA_RESID=1 (expert-group results stay on device): the claim is a placement change, so a byte-identical proof (sha256 of a greedy completion with RESID on vs off, same as #474 did) plus the tok/s delta would let me merge fast. Thanks @ZacharyZcR.

@ZacharyZcR

Copy link
Copy Markdown
Contributor Author

Validation data as requested — rebased onto current dev (through the AMD/HIP single-source refactor; the resident path's peer-copy/event-wait API surface is now mapped in backend_gpu_compat.h, and the HIP syntax check passes).

Byte-identical proof — 6× RTX 5090, driver 610.43.02, CUDA 13.3, GLM-5.2 int4, full champion config (PIPE=2 TC_W4A16=1 ROUTER=1 NUMA=1 PIN_GB=all TEMP=0 DRAFT=0), 256-token greedy, flag-only A/B on the same binary:

sha256(completion, RESID=0) = 98d48995cc45a6a2a05dc7deffa5ff710d4bc3defb279aa41bc0f9108aec5328
sha256(completion, RESID=1) = 98d48995cc45a6a2a05dc7deffa5ff710d4bc3defb279aa41bc0f9108aec5328

diff of the two completions: identical.

Perf, same runs (expert hit 100%, i.e. nothing hiding behind disk):

RESID=0 RESID=1
decode 256 tok 42.36 s (6.04 tok/s) 42.08 s (6.08 tok/s)

Parity, as the PR body states — this is a structural change (the dataflow a graph-captured decode needs), not a speed claim.

On the red CI: the Engine/linux/macos jobs fail on anything that merges with current dev — 12 stray built test binaries were committed in #368's conflict resolution, so make skips building tests/test_e8_kernel and the runner hits EACCES. Fix in #500; a re-run after it merges should come back green (CUDA/HIP/Windows/Python/Web all pass already).

@JustVugg
JustVugg merged commit 31d644c into JustVugg:dev Jul 21, 2026
6 of 9 checks passed
@JustVugg

Copy link
Copy Markdown
Owner

Merged. The flag-only A/B on the same binary with identical sha256 is the right way to prove a dataflow change, and calling the perf parity honestly instead of dressing it as a win is why this went in without further questions. You were also right about the red CI — and to be precise about blame: those 12 stray binaries were committed by my conflict resolution on #368 (a careless git add -A in the rebase worktree), not by the PR itself. #500 has removed them and pattern-ignored the path. Ready for C1 whenever you are.

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.

2 participants