Repository navigation
Conversation
… spec verify Speculative target-verify on GDN layers keeps one full [HV, V, K] SSM snapshot per draft token so the commit can pick the accepted one. This adds an opt-in mode that keeps each draft step's (u, k, g) tuple instead (HV * (V + K + 1) floats per step, ~64x smaller at K = V = 128) and replays the accepted prefix at commit with the verify kernel's exact rounding, so the committed state and every output stay bit-identical to the snapshot verify. The KV budget reserves the tuple ring's real size, so the freed memory becomes KV cache instead of sitting idle. - verify kernel: optional (u, k, g) stores; tiles exactly like the dense verify - commit: _recon_kernel replay (inline-PTX rounding on CUDA, explicit fma on ROCm); the mamba-track replay runs before the in-place accepted-state replay so both start from the pre-verify state - pool: fp32 tuple ring replaces intermediate_ssm; conv windows unchanged - gating: --speculative-eagle-topk 1 (EAGLE/MTP, DFLASH, DSPARK), Triton verify kernel, GDN models; validated after the speculative hook and asserted again in the pool and at verify time - tests: test_gdn_lossless_verify.py checks exact equality against the snapshot verify (fp32/bf16 pools, PP stable rows, track ordering, bounds, PTX vs portable replay)
|
Hi @hnyls2002 @yizhang2077 @Qiaolin-Yu and others, could one of you trigger CI on this PR when you have a moment? It adds an opt-in |
|
Hi @hnyls2002 @yizhang2077 @Qiaolin-Yu, @ShangmingCai, @HaiShaw, @BBuf and others, could one of you trigger CI on this PR when you have a moment? It adds an opt-in |
- kv_cache_configurator: charge the default verify scratch from stage_per_req, so a fused draft's widened state entry is still priced (main switched the budget to it in sgl-project#43326). - kv_cache_configurator: reject --enable-linear-lossless-verify when building the unified pools; the guard lived in UnifiedHybridReqToTokenPool, which main removed. - test: import the verify kernel from ops/attention/linear and stub the commit hook's state_pool and layer_router. - ruff format the moved verify kernel.
This comment has been minimized.
This comment has been minimized.
BBuf
left a comment
There was a problem hiding this comment.
Please address the [P1] items, then re-ping.
Agent-assisted pre-review, findings verified against 24b47a2ce7ac.
| intermediate_state_indices=intermediate_state_indices, | ||
| cache_steps=cache_steps, | ||
| retrieve_parent_token=retrieve_parent_token, | ||
| u_states_buffer=u_states_buffer, |
There was a problem hiding this comment.
[P1] These keywords are passed even when lossless verify is disabled. On XPU this function is rebound to the platform wrapper, which accepts neither these arguments nor **kwargs, so ordinary GDN speculative verification now raises TypeError. Preserve the flag-off signature and gate tuple forwarding on backend support.
| ) | ||
| decode = cfg.linear_attn_decode_backend or cfg.linear_attn_backend | ||
| verify = cfg.linear_attn_verify_backend | ||
| if verify not in (None, "triton") or (verify is None and decode == "flashinfer"): |
There was a problem hiding this comment.
[P1] Checking the backend name does not exclude Ascend: this configuration passes validation, but the registry selects the Ascend verify and commit implementations. The pool removes intermediate_ssm while the Ascend commit still consumes it and accesses its shape, so the first commit fails. Reject unsupported platforms before allocating the tuple-only pool.
Preserve the flag-off XPU verify signature and reject lossless verification on unsupported platforms.
|
@hsthe29 I pushed Automated pre-review for |
This comment has been minimized.
This comment has been minimized.
BBuf
left a comment
There was a problem hiding this comment.
Please address the [P1] items, then re-ping.
Agent-assisted pre-review, findings verified against d1097b76955e.
| from sglang.test.test_utils import CustomTestCase | ||
|
|
||
| register_cuda_ci(est_time=40, stage="base-b", runner_config="1-gpu-large") | ||
| register_amd_ci(est_time=60, suite="stage-b-test-1-gpu-large-amd") |
There was a problem hiding this comment.
[P1] This AMD lane defaults to MI300, so the (4, 16) case never reaches the gfx950-specific verify tiling. The flag admits MI35x, but the description says gfx950 is untested and the ROCm evidence comes from a v0.5.21 backport. Please register/run these parity checks on MI35x at this head, covering both N=1 and N>1, or gate that architecture out until validated.
|
/tag-and-rerun-ci |
|
/rerun-test test/registered/attention/unittests/gdn/test_gdn_lossless_verify.py Targeted validation for |
|
🚀 |
|
@BBuf Thanks for the review, and for pushing d1097b7 to preserve the flag-off XPU verify signature and reject unsupported platforms. I've renamed the PR to follow the title convention. For the remaining P1 (running the parity checks on MI35x at this head, covering both N=1 and N>1), I don't have access to the MI35x machine over the weekend, so I'll pick it up on Monday. Sorry for the wait, and thanks for your patience. |
|
/rerun-failed-ci |
2 similar comments
|
/rerun-failed-ci |
|
/rerun-failed-ci |
Keep the lossless (u, k, g) arguments next to main's new ring_indices and replayssm_request_ring. Take main's rewritten _init_unified_pools without the unified-memory guard: validate_linear_lossless_verify already rejects --enable-unified-memory, and the guard broke test_unified_tri_pool, whose mocked mamba args do not carry the flag.
…nal slot Add the flag's row to server_arguments.mdx, and append the field to POSITIONAL_FIELD_ORDER instead of inserting it next to the ReplaySSM flags, so existing positional ServerArgs calls keep their slots.
…and a batch gfx950 picks its own verify tiles for the (4, 16) heads: (BV=8, 4 warps) for one request and (BV=16, 2 warps) for a batch. Register the test on the MI35x suite and add a single-request sweep, since every other case runs a batch of five.
Preview approval requiredThis pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit |
Could you take another look? |
|
[BBuf Review Agent] @hsthe29 The docs, positional-order and unified-pool fixes are in place, and the MI35x tests cover both gfx950 tile configurations. I’m fine with the reported parity results at this head. |
|
/rerun-test test/registered/kernels/ops/attention/test_fused_verify_triton_gdn.py |
|
🚀 |
Motivation
How it works today. On hybrid GDN models (Qwen3.5 / 3.6 / 3.8, Qwen3-Next), speculative target-verify runs the recurrent kernel with
disable_state_update=Trueand writes a full[HV, V, K]SSM snapshot for every draft position intointermediate_ssm. After acceptance,scatter_mamba_states_after_mtp_verifycopies two snapshots back into the persistent pool: the last accepted position, and, with theextra_bufferradix strategy, the position that crossed a mamba-track boundary. Every linear-chain algorithm goes through this path: EAGLE/MTP, DFLASH and DSPARK.The problem. That buffer is large, and it grows with the draft width. On Qwen3.6-35B-A3B one snapshot is 60 MiB (30 GDN layers × 32 heads × 128 × 128 × fp32), so each request needs
60 MiB × D._handle_max_mamba_cachereserves this for every request slot before the KV pool is sized, so all of it comes out of KV capacity:intermediate_ssmIn the first setup the KV pool ends up at 498,880 tokens. That is not enough for 25 concurrent 26k-token requests, so 7 of them sit in the queue.
--enable-linear-replayssm-specalready gets rid of these snapshots, but it changes how verify is computed: a frozen checkpoint plus a compact ring, a causal-masked window, and compensated materialization. Its results are therefore not bit-identical to the default verify. On Qwen3.6-35B-A3B with MTP (measured on v0.5.21), greedy outputs diverged from the default path after 23–143 tokens on 8 of 8 prompts. Many deployments can live with that. Some cannot: evals, A/B rollouts and regression testing all need the outputs to match the default path exactly.The idea. Each recurrent step is
The verify kernel already computes
u_t, the l2-normalizedk_tandg_t. Andh_0is still in the pool, because verify does not update state. So instead of keeping everyh_t, we can keep(u_t, k_t, g_t)and rebuild the acceptedh_tat commit time. That costsHV * (V + K + 1)floats per step instead ofHV * V * K, which is 63.8× less at K = V = 128.The rebuild has to round exactly the way the verify kernel does. Inside the kernel the decayed state
h * exp(g)is used twice, by the delta reduction and by the update. So the compiler keeps it as a separately rounded multiply and folds the update into a single FMA:fma(k, u, round(h * exp(g))). The replay writes out the same two instructions. A naive replay written as one expression lets the compiler contract a different multiply and drifts by 1–2 ULP. The tests include that case, to show they would catch it.Modifications
Everything sits behind a new flag,
--enable-linear-lossless-verify, off by default. With the flag off, no code path and no memory reservation changes.Verify kernel (
fused_sigmoid_gating_recurrent.py,gdn_triton.py)u_states_buffer/k_states_buffer/g_states_bufferarguments and aCACHE_U_TUPLESconstexpr. For each draft step, the kernel storesu(after the beta scaling), the l2-normalizedk, andg, at the row given byintermediate_state_indices.kandgdo not depend on the V tile, so only thei_v == 0program writes them.--speculative-adaptive.target_verify=True, which includes the SM90BV=4tile). Verify outputs are therefore identical by construction, not just in testing.Verify call site (
gdn_backend.py)intermediate_ssm.**kwargs: it would silently ignore the buffers, and the commit would replay zeros.retrieve_parent_token) reaches the tuple path.Commit (
hybrid_linear_attn_backend.py,mamba_state_scatter_triton.py)_recon_kernelreplays all GDN layers of a batch in one launch, starting fromssm_states[slot]and stopping at the accepted step.mul.rn.f32/fma.rn.f32inline PTX.fregister constraint. HIP builds instead use a plain multiply feedingtl.fma, which the compiler cannot contract.fused_mamba_state_scatter_with_maskuses.scatter_mamba_states_after_mtp_verifygets askip_ssmargument, so the SSM half is left to the replay.SGLANG_ENABLE_PP_SPEC) work: the replay reads the same rows verify wrote to (src_indices_raw).Pool and memory accounting (
memory_pool.py,kv_cache_configurator.py,disaggregation/decode.py,unified_memory_pool.py)MambaPool.SpeculativeStategetsintermediate_ssm_u/_k/_g, shaped[layers, spec_slots + 1, D, HV, V | K | -]. In this modeintermediate_ssmisNone.--mamba-ssm-dtypeis, so bf16 state pools stay exact too.spec_intermediate_bytes_per_reqcomputes the scratch size from the same shapesMambaPoolallocates: the tuple ring plus the deduplicated conv window._handle_max_mamba_cachereserves exactly that in all three sizing branches, including the joint solve formax_mamba_cache_size. The freed memory becomes KV cache instead of an idle reservation. With the flag off, the existing formula is used unchanged.HybridReqToTokenPooland the PD decode pool. The unified memory pool rejects it.Gating (
attention_hook.py,pipeline.py)The replay walks one linear chain, so the flag requires
--speculative-eagle-topk 1. This is enforced in three places:validate_linear_lossless_verifyruns right afterhandle_speculative_decoding. It cannot live inhandle_linear_attn_backend, because that runs earlier, before DFLASH and DSPARK pin topk to 1.--enable-unified-memory, and anySGLANG_RAGGED_VERIFY_MODEother thanstatic.MambaPoolrejects topk ≠ 1 and KDA layers, and the KV cache configurator rejects non-GDN models.forward_extendasserts that no tree metadata reaches the tuple path.Tests:
test/registered/attention/unittests/gdn/test_gdn_lossless_verify.py, registered for CUDA and AMD CI. Details below.Not changed: the default verify path, KDA, tree verify, and ReplaySSM.
Accuracy Tests
Unit tests (
test_gdn_lossless_verify.py, 8 tests). They drive the realMambaPool, the real verify kernel and the real commit entry point (HybridLinearAttnBackend.update_mamba_state_after_mtp_verify), and requiretorch.equalagainst the snapshot verify for:On H100 these 8 pass, and so do the existing GDN tests:
test_triton.py,test_gdn_replayssm_spec_fold.py,test_fused_verify_triton_gdn.py,test_gdn_noncontiguous_stride.py.Server-level, greedy. Stock
main(d75d5b3) against this branch with--enable-linear-lossless-verify, same flags otherwise (--max-running-requests 8 --max-mamba-cache-size 40 --kv-cache-dtype bf16 --page-size 64), 1× H100. Each workload runs twice per server, with a cache flush in between, so the server's own run-to-run determinism is measured too:cached_tokens = 512). This is the workload that catches a wrong track state.Every cell matched stock on both runs. When the same 8 prompts are fired from 8 threads instead, outputs occasionally differ in 2 of 8. Stock differs from itself in exactly the same way between runs, because arrival order changes how requests are batched; it is not specific to this change.
ROCm. The same kernels and commit logic, backported to v0.5.21 and run on an MI325X (gfx942, ROCm 7.15, Triton 3.8):
--enable-deterministic-inference --attention-backend tritonandSGLANG_USE_AITER=0, all four workloads matched stock on both runs.test_gdn_lossless_verify.pypasses at this PR's head on an MI350X (gfx950, ROCm 10, Triton 3.8). Every check is bit-exact for one request and for a batch, which run gfx950's own verify tiles (BV=8 / 4 warps and BV=16 / 2 warps). Server-level runs on gfx950 are not done yet.Speed Tests and Profiling
End to end. Qwen3.6-35B-A3B-FP8, MTP 3/1/4, 1× H100, with
--mem-fraction-static 0.8 --kv-cache-dtype bf16 --page-size 64 --max-running-requests 25 --max-mamba-cache-size 150 --enable-hierarchical-cache.bench_servingon the random dataset, 25 requests × (26k in / 2k out), concurrency 25:max_total_num_tokensAccept length is within run-to-run noise: it ranged from 3.45 to 3.51 across repeated runs of both configurations. That is expected, since the outputs are bit-identical.
Memory with other algorithms (8 requests, 40 mamba slots, so the absolute numbers are not comparable with the table above):
max_total_num_tokensPer-step kernel cost. Microbenchmark on the Qwen3.6-35B-A3B GDN geometry (30 layers, 16/32 heads, 128 × 128), H100, CUDA events, about 25% of rows crossing a track boundary. The verify kernel gets cheaper, because it writes
HV * (V + K + 1)instead ofHV * V * Kfloats per step. The replay is cheaper than the scatter at D=4 and costs a little more at D=8 and D=16, where it walks a longer chain. Net, every configuration is faster per verify step:Checklist
--help; there is no docs page that lists the linear-attention flags.Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): ⏳ Run #42155370412
Latest PR Test (Extra): ❌ Run #42155370523
Latest PR Test (AMD ROCm 10): ⏳ Run #42155370457
Tip
Base CI only covers the core cases, and its workload may not exercise your change. Use
/rerun-testto run the tests your change touches, or copy this prompt for an agent: