glm5next: keep rejected DFlash draft keys out of the C4 tail ring - #622
Draft
shieldstar wants to merge 1 commit into
Draft
shieldstar wants to merge 1 commit into
shieldstar wants to merge 1 commit into
Conversation
glm_kpool.py:_decode_update_kernel writes tail[slot][pos % 4] = key for every row of a speculative request, including draft rows the verifier later rejects. The 4-slot ring therefore ends each step holding keys of tokens that were never committed, and the next completed pool of the FP8 C4 index is built from them. selector_num_accepted_tokens is staged by Glm5NextModelState but pooled_indexer.py never consumed it. Track a per-slot DecodeTailRing: snapshot the ring at the start of each decode step, and on the next step restore that snapshot and replay only the accepted prefix of the saved rows before writing new ones. The accepted length is position-derived (this step's first position minus the previous step's); selector_num_accepted_tokens is consumed as a cross-check and positions win on disagreement, so resume and replay re-base the request correctly. The prefill tail kernel zeroes the slot's saved count so a fresh or resumed prefill never restores a stale ring. Cost: ~100 KB at max_num_seqs=16, K=7. Only speculative decoding of GLM-5.3 (DFlash) is affected; the MTP snapshot/restore path is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: shieldstar <53370897+shieldstar@users.noreply.github.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
glm_kpool.py:_decode_update_kernelwrotetail[slot][pos % 4] = keyfor every row of a speculative request, including DFlash draft rows the verifier then rejected. The 4-slot C4 tail ring therefore carried keys of never-committed tokens into the next completed pool of the FP8 index.Glm5NextModelStatealready stagesselector_num_accepted_tokens, butpooled_indexer.pynever consumed it.This PR adds per-slot
DecodeTailRingbookkeeping: each decode step snapshots the ring, and the next step restores that snapshot and replays only the accepted prefix of the previously saved rows before writing its own.Details
selector_num_accepted_tokensis consumed as a cross-check; on disagreement positions win, so resume and replay re-base the request correctly instead of trusting a stale count._prefill_tail_kernelzeroes the slot's saved count, so a fresh, resumed or short-tail prefill chunk never restores a stale ring.update_decode_poolstakestail_ring=andnum_accepted_tokens=(ones as fallback); the pooled indexer allocates the ring sized1 + num_speculative_tokensand passesmain_metadata.selector_num_accepted_tokens[:num_reqs].max_num_seqs=16, K=7.Found while auditing the mixed prefill/decode path for the corruption described in #616; it is a separate defect (that corruption reproduces with speculative decoding off), reported here on its own.
Testing
Kernel-level (GB10, Triton 3.7.1, the
dev/jovian-judgementcontainer):_decode_update_kernel,_prefill_pool_kerneland_prefill_tail_kernelJIT-compile and run. A two-step scenario (max_num_seqs=16, K=7, two 8-row speculative requests at positions 100 and 200 plus a 300-row prefill; step 2 restarts them at 103 and 205 withnum_accepted_tokens=[3, 5]) produces tails, ring state and completed pools bit-identical to a from-scratch run that only ever saw the accepted rows; the prefill slot's saved count is 0. The check script is small and can be attached if wanted.An earlier revision of this patch failed to compile at engine warm-up (
Loop carried variable saved changed type, atl.static_rangeindex reused as a runtime loop variable); that is fixed and is why the kernels are now exercised on the GPU rather than onlypy_compiled.Serving-level, four-node DGX Spark TP4, image built from
dev/jovian-judgement@2e67b303plus this branch and #618, GLM-5.3-Flash-NVFP4-Spark with the DFlash2 draft (k=7), fp8 KV,--max-num-batched-tokens 1024:Engine start, FULL cudagraph capture and the DFlash2 draft warm-up all pass with the new kernels (3.5 min boot, KV pool 5,455,114 tokens at 1M context).
llm-inference-benchcoding-peak c1 smoke: 85.0 tok/s median, 91.0 max, 0 CJK runs; the same as the unpatched build (83-86), and since that number is acceptance-bound it shows DFlash acceptance on coding prompts is unchanged.llm-inference-benchestonia profile, c4 x 30, temperature 1.0: 26/30 (2 wall-clock timeouts mid-reasoning, one "United Kingdom", one correct answer written in Chinese). The unpatched build on the same chunk size scored 24-28/30 across three runs; the residual misses are the concurrent-prefill issue in GLM-5.3-Flash TP4 on 4x DGX Spark: decode corrupted by concurrent chunked prefill; rate scales with prefill tokens per step (4096 -> 1024 chunks: estonia 12/30 -> 28/30) #616 and are not affected by this change.Spec-decode counters over the estonia run: 193,491 accepted tokens over 80,796 draft steps (2.39 per step), no acceptance collapse.
Four concurrent 90k-token needle prompts, 3 rounds: 10/12 clean, 1 runaway, 1 wrong (all of the GLM-5.3-Flash TP4 on 4x DGX Spark: decode corrupted by concurrent chunked prefill; rate scales with prefill tokens per step (4096 -> 1024 chunks: estonia 12/30 -> 28/30) #616 class).
Corrected 2026-09-03: this line first read 11/12. Our needle harness scored a stream clean if the code appeared anywhere in the output, so a degenerate run that quoted the code mid-babble counted as a pass. Re-scored strictly (the visible answer is the code and the stream terminated on its own) the same log is 10/12. The comparison in this PR is unaffected, since the baseline was measured the same way, but the absolute number was wrong and is fixed here. Details in GLM-5.3-Flash TP4 on 4x DGX Spark: decode corrupted by concurrent chunked prefill; rate scales with prefill tokens per step (4096 -> 1024 chunks: estonia 12/30 -> 28/30) #616.
Not measured: a direct before/after on index-pool fidelity for generated tokens, which is what this fix changes; the semantics check above is the evidence for that part.
Duplicate check
gh pr list --state open --search "tail ring OR glm_kpool OR C4 tail OR rejected draft"returns nothing related; #505 (parallel C4 prefill pool writes) touches the prefill pool path, not the decode tail ring, and does not consumeselector_num_accepted_tokenseither.Conventions
ruff checkandruff format --check(0.14.0, repo pyproject) pass on both files; every touched function has a Google-style docstring; the commit is DCO signed.AI assistance
The audit that found the defect, the kernel change and the compile/semantics check were produced with AI assistance (Claude); the submitter reviewed every changed line and ran the tests listed above on our cluster.
Assumptions to check in review
selector_num_accepted_tokensequals bonus + accepted rows of the previous verify step; it is only used as a cross-check, so a mismatch degrades to the position-derived value rather than corrupting state.prev_first + accepted.