Repository navigation
[AMD][Bugfix] Mamba radix cache: avoid the degenerate single-token (M=1) recompute that corrupts the first token - #29633
ntgiang71096 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a retreat mechanism in the Mamba radix cache to prevent single-token (M=1) MoE forwards, which can produce NaNs on gfx950 GPUs. It also adds CPU-based unit tests and GPU-based integration tests to verify this logic. The reviewer identified a critical performance regression where perfect cache hits (tail == 0) would trigger an unnecessary retreat and recomputation. To resolve this, the reviewer suggested updating the retreat condition to only trigger when the tail is exactly 1, and provided corresponding updates for both the CPU and GPU test suites.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Hi @merrymercy @hnyls2002 this is ready for review whenever you have a moment. Could you kick off CI (/tag-and-rerun-ci)? Thank you! |
|
/tag-and-rerun-ci |
…=1) recompute that corrupts the first token On a hybrid GDN+MoE model with the mamba radix cache (--mamba-scheduler-strategy extra_buffer), a cache hit on a prompt of length == 1 mod 64 restores the nearest GDN checkpoint and recomputes exactly one token (extend_input_len == 1). On gfx950 that M=1 MoE forward selects tile block_m=16, whose aiter ck_moe_stage1 GEMM returns NaN for a finite fp8 input -> flat logits -> a wrong, silent first generated token. The GDN/mamba recurrence is bit-exact correct; the corruption is the M=1 MoE tile (the root fix is upstream aiter, tracked separately). Fix: in MambaRadixCache._match_post_processor, when a hit would leave a recompute tail < 2, retreat the matched prefix to the previous GDN checkpoint (tail becomes chunk+1 >= 2), moving the KV prefix and the COW restore source to the same boundary in lock-step; fall back to a fresh compute if there is no earlier checkpoint. Routing-only and correctness-safe: tail >= 2, fresh, and decode are unchanged by construction. Verified on AMD MI350X (gfx950) / Qwen3.6-35B-A3B-FP8 / TP1: tail=1 corruption removed (extend_input_len >= 2), tail=2 still hits the cache, GSM8K unchanged. Tests: - test/srt/mem_cache/test_tail1_retreat_logic_cpu.py (CPU unit test of the retreat decision) - test/registered/amd/test_qwen35_radix_tail1_fidelity_mi35x.py (E2E structural gate)
…l==0 A perfect cache hit leaves tail==0 (nothing to recompute, no M=1 MoE forward), so it must not be retreated; the previous `tail < 2` condition would have forced an unnecessary recompute of up to a full chunk on perfect hits -- a regression. Restrict the retreat to `0 < tail < MIN_SAFE_GDN_EXTEND` (i.e. exactly tail==1). Mirror the same guard in the CPU unit test and add tail==0 no-retreat coverage to both the CPU and E2E tests.
Wrap the edge cases in a test_ function driven by a shared CASES list, so pytest collects them. Previously all logic lived under __main__, so pytest saw 'collected 0 items' (exit 5) and failed build-test. Logic unchanged; the manual __main__ runner is kept.
18ad6e6 to
5a0f0d7
Compare
run_suite.py strictly requires every file under test/registered/ to have a register_*_ci call; the CPU logic test had none, failing build-test's 'Run unit tests' (run_suite --hw cpu). Register it via register_cpu_ci(suite=base-a-test-cpu), mirroring test_int8_checkpoint_store.py.
|
Hi @hanming-lu @hnyls2002 — sorry to ping directly, but this one's green now and should be a quick review if you have a moment. Small (+44, routing-only) correctness fix in mamba_radix_cache.py (_match_post_processor): on a radix cache hit where the prompt length is ≡ 1 (mod 64), the 1-token recompute (extend_input_len == 1) issues an M=1 MoE that returns NaN on gfx950 → silent wrong first token. The fix just retreats one checkpoint so the recompute is never M=1. The GDN recurrence itself is bit-exact (verified), and it's validated 24/24 on MI350X. Tests added (CPU decision-logic + MI35x fidelity), and build-test + base-a-test-cpu are green. cc @fzyzcjy — you wrote _match_post_processor, so you'd catch anything off faster than anyone 🙂 It's the companion to the extra_buffer enablement in #30359 (just merged). Thanks! |
|
Thanks @ntgiang71096! The file this PR patches, python/sglang/srt/mem_cache/mamba_radix_cache.py, was removed from main in #40313 ("Remove swa and mamba radix cache"), so the patch no longer applies. The underlying M=1 gfx950 MoE tile issue may still be relevant against the new unified-cache implementation — if so, please reopen against the new tree and we can take another look. |
Motivation
On a hybrid Gated-DeltaNet + MoE model (e.g. Qwen3.5/3.6) with the mamba radix prefix cache
(
--mamba-scheduler-strategy extra_buffer), a cache hit on a prompt whose total length is≡ 1 (mod 64) produces a wrong, silent first generated token.
Mechanism. The mamba radix cache stores the GDN recurrent state only at periodic checkpoints
(every
chunk = 64tokens), so checkpoints land at multiples of 64. On a hit, the engine restores thenearest checkpoint and recomputes the remaining "tail" — i.e. the number of tokens past that
checkpoint (
tail = extend_input_len, the recompute length). When the prompt length is≡ 1 (mod 64),the nearest checkpoint sits exactly one token back, so the tail is 1 (
extend_input_len == 1).Root cause (localized; the corrupting kernel is upstream aiter). That single-token (M=1) recompute
issues an M=1 MoE forward, which on gfx950 selects MoE tile
block_m=16; the aiterck_moe_stage1GEMM returns NaN at that tile for a finite fp8 input (the
32/64/128tiles are correct) → flatlogits → garbage first token. The GDN/mamba recurrence is bit-exact correct (verified). The
precise reason inside CK is still under investigation — it reproduces only under the live server's
warmed state, not in an isolated replay (see Related issues) — but the trigger and the fix are
verified. This PR is the correctness-safe sglang-side mitigation: it removes the
tail == 1trigger so the engine never issues the M=1 MoE, robust even on an un-fixed aiter.
The defect is the
extra_buffermamba-radix path. Everytail == 1cache hit is corrupted —24/24 (100%) of templated re-sends across six diverse prompts — while a
no_bufferbaseline(which does not cache mamba state) never reaches the path and never corrupts.
The fact of corruption is reliable; the specific garbage text is not bit-stable run-to-run,
even at temperature 0. The aiter MoE reduction is itself non-deterministic, and the corrupted
state has near-tied (flat) top logits, so the wrong argmax flips between forwards; a clean state stays
stable because its logit gap is large. The clean, low-noise probe is therefore the first-token
argmax (quantified under Accuracy Tests).
Why it's silent / impact. Standard evals grade a final answer after a long generation that
re-grounds on the correct prompt, and the trigger is rare (~1.5% of prompt lengths), so the bug stays
latent. It bites hardest where the first token is load-bearing and there is no recovery step:
≡ 1 (mod 64)(
65, 129, 193, …) returns a different first token than the original call (breaks response caches,"regenerate = same answer", eval reproducibility).
max_tokens=1): the garbage token is theanswer — no recovery.
corrupted (measured — see Accuracy Tests).
{/[/a tool name): garbage → parse failure(plausible consequence of the measured first-token corruption; not separately harness-tested).
A concrete, replicable example (AMD MI350X, Qwen3.6-35B-A3B-FP8, TP1, temp 0, thinking off).
This is the entire request — a normal 193-token prompt (
= 3×64 + 1, so≡ 1 mod 64):Send it once (cache miss), then send the identical request again. The second call is a cache
hit reporting
cached_tokens = 192(the nearest checkpoint,3×64) — so 192 of 193 tokens arerestored and exactly one token (the tail) is recomputed:
cached_tokensParis✓niestety, nie mogę odpowiedzieć…·\/\/\/\/…·svp, please provide the question…·ú— 4/4 wrong, each differentSame request, same temp 0 — the only difference is that the second call was served from the cache. The
corruption is reliable (every re-send wrong) yet the garbage differs each time (temp-0 greedy over a
flat-logit state + the non-deterministic MoE — not a sampling effect). With longer generations the
recurrent-state corruption often carries forward through the whole response, not just the first token.
Modifications
One file,
MambaRadixCache._match_post_processor(
python/sglang/srt/mem_cache/mamba_radix_cache.py). Before the copy-on-write (COW) state-restorelogic: when a hit would leave a recompute tail
< 2, retreat the matched prefix to the nearestearlier checkpoint ancestor (tail becomes
chunk + 1 ≥ 2); if there is no earlier checkpoint, fallback to a fresh compute (prefix 0). The KV prefix (
value[:best_value_len]) and the COW restore source(
last_node.mamba_value) move to the same boundary in lock-step, so KV and mamba state stayconsistent.
Design notes:
cow_mamba, not onenable_mamba_extra_buffer— i.e. the same condition that guardsthe COW state-restore later in this method. Any strategy that performs the mamba COW restore is
covered (e.g. int8); a strategy that doesn't set
cow_mamba(e.g.no_buffer, which doesn't cachemamba state) never enters the guard, so it is inert there. Correctness-safe in all cases.
full_untruncated_fill_ids, e.g. a PD-disaggregateddecode match before fill_ids is populated). Treating it as 0 would spuriously retreat every decode
hit and discard real cache hits.
(last_node, best_value_len)— the KVslice and the COW source — so they can never desync.
ck_moe_stage1block_m=16GEMM, a ROCm-only Composable Kernel). The trigger (tail == 1) isbackend-independent — it comes from the generic
extra_buffercache — and whether other backends'MoE kernels also mishandle the degenerate M=1 shape is untested (we do not claim it is safe
there). The retreat is therefore applied unconditionally on the mamba-COW path, not arch-gated:
it is a routing-only change, always correct on any backend (worst case ≤
chunkextra recomputetokens on ~1.5% of full re-sends), and avoids a degenerate single-token shape regardless of vendor.
If maintainers prefer to limit the change to the only confirmed-affected backend, gating the retreat
to ROCm is a one-line change.
Accuracy Tests
Primary gate — deterministic, by construction. The fix changes only a routing decision (which
checkpoint to restore from), based on lengths. The test asserts the structural property: a
L ≡ 1 mod 64re-send no longer produces a 1-token extend (extend_input_len ≥ 2, or a freshcached=0), with atail = 2control that must still hit the cache (guards againstover-triggering). Verified on current
mainatL=705(705 = 11×64 + 1→ tail=1, was garbage) andL=706(= 11×64 + 2→ tail=2, correct).Why the fix is correct (decisive isolation). Build
L ∈ {193,194,195,196}sharing the samecheckpoint@192 (
cached=192for all):tail=1(193) is corrupt;tail=2/3/4(194/195/196) arecorrect. So the stored checkpoint is correct — only the single-token recompute is wrong, and for
tail ≥ 2the later recomputed tokens re-ground. The clean correctness probe (per Motivation) is thefirst-token argmax: on tail=1 hits it matched fresh 0/40 against a 40/40 fresh-vs-fresh
floor before the fix; the retreat removes the path.
No regression. For
tail ≥ 2, fresh, and decode the code path is unchanged by construction(the retreat only fires on
tail < 2), so those cases are unaffected. Corroborating: GSM8K (n=40) =0.95 unchanged, radix cache hits 38/40; cache-HIT vs fresh next-token distributions are within the
MoE non-determinism floor (Jensen-Shannon over top-20 logprobs, temp 0).
Reproduce:
Scope of what was executed: the only backend exercised is AMD MI350X (gfx950 / ROCm / aiter) —
validated on Qwen3.6-35B-A3B-FP8, TP1, current
main(d282bdd8f1); the fail/pass split is verifiedat
L=705/L=706. A broader tail sweep (tail=1 fails; tail 2..63 and ≥64 pass) was run on an earliercontainer image; its results are consistent with the current-main
L=705/L=706verification but thefull sweep was not re-run on current main. The 397B / TP8 lane is not yet executed, and no
non-AMD backend was tested (the fix is applied generically — see the Backends / scope note above).
Speed Tests and Profiling
Negligible and bounded to the rare trigger:
≤ chunk(≤64) extra recompute tokens only on a fullre-send whose length is
≡ 1 mod chunk(~1/64 of lengths); a single-node prompt at that length fallsback to a full recompute for that one request. Zero effect on
tail ≥ 2hits or normal serving.No kernel changes.
Checklist
test_tail1_retreat_logic_cpu.py— CPU retreat-logic;test_qwen35_radix_tail1_fidelity_mi35x.py— E2E structural gate)Related issues
The kernel root cause is filed upstream as ROCm/aiter#4032 — the
ck_moe_stage1block_m=16GEMMreturns NaN at M=1 on gfx950 (clean fp8 input → NaN output). Confirmed in the live server: clamping
block_size_M ≥ 32inaiter/fused_moe.pydrives the tail=1 corruption to 0%, identifying the tileas the cause. The why-inside-CK is still open (it reproduces only under the live server's warmed state,
not in an isolated replay), which is why this sglang-side trigger removal is the verifiable, shippable
change.
CI States
Latest PR Test (Base): ❌ Run #29419886515
Latest PR Test (Extra): ❌ Run #29419886372