Fix DSpark greedy-identity break: replay accepted tokens via single-token decode - #659
Open
nexus-cw wants to merge 1 commit into
Open
Fix DSpark greedy-identity break: replay accepted tokens via single-token decode#659nexus-cw wants to merge 1 commit into
nexus-cw wants to merge 1 commit into
Conversation
…ingle-token decode DSpark speculative decode did not preserve byte-identical greedy (--temp 0) output vs. non-speculative decode on longer generations. Root cause: the compressed-KV 'compressor frontier' state for every accepted token (including the always-accepted bonus token) was computed via the verify batch's own multi-token GEMM (ds4_gpu_matmul_f16_pair_tensor), a numerically distinct code path from ordinary single-token decode's fused projection+store kernel (ds4_gpu_matmul_f16_pair_compressor_store_tensor) -- standard FP16/BF16 GEMM non-associativity between batched and single-vector kernels means the committed KV state for accepted tokens subtly diverged from what pure decode would produce for the same tokens, and this drift eventually flips an argmax tie on longer generations. Fix: ds4_session_eval_dspark_speculative_argmax had a fast path for full accepts (commit_drafts == draft_n) that committed the batch-verify's KV/compressor-frontier state directly. The partial-accept path already had a rollback+replay mechanism (roll back to the pre-verify snapshot, then re-decode each accepted token one at a time via the same single-token decode kernel plain greedy decode uses). This removes the full-accept fast path entirely so every accept -- full or partial -- falls through to that same rollback+replay path, guaranteeing bit-identical post-accept KV state and logits vs. plain decode for the same tokens. Zero effect when no drafter is configured. Verified byte-identical drafter-vs-no-drafter output (resident IQ2XXS + the DSpark support GGUF, --temp 0) on two prompts at both 400 and 800 generated tokens, including the reproducer that previously diverged mid-generation. Cost: this adds a single-token replay per accepted token even on full accepts, worsening an already-net-loss drafter throughput case on this hardware/pairing (~15-20% slower generation vs. no-drafter, up from ~5%) -- correctness first, honestly measured; see the linked issue for details. make clean && make cuda-spark: clean rebuild, zero warnings. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
nexus-cw
added a commit
to nexus-cw/ds4
that referenced
this pull request
Aug 2, 2026
Issue antirez#658, PR antirez#659 (nexus-cw:dspark-greedy-identity -> main). Fix verified end-to-end on upstream main directly (the DSpark drafter GGUF loads natively there); pre-fix reproduction on unpatched upstream main also confirmed before filing, so the bug report is not fork-specific.
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.
Summary
Fixes #658: DSpark speculative decode did not preserve byte-identical greedy
(
--temp 0) output vs. non-speculative decode on longer generations.Root cause (see linked issue for full detail)
ds4_session_eval_dspark_speculative_argmax's accept-commit logic had a fastpath for full accepts (
commit_drafts == draft_n) that committed the verifybatch's own batched-GEMM-derived compressor-frontier KV state
(
ds4_gpu_matmul_f16_pair_tensor) directly. That state is numericallydistinct (standard FP16/BF16 GEMM non-associativity) from what ordinary
single-token decode's fused kernel
(
ds4_gpu_matmul_f16_pair_compressor_store_tensor) would have produced forthe same token, so it silently drifted from a pure-decode trajectory until it
eventually flipped an argmax tie on longer generations.
The partial-accept path already had the correct mechanism: roll the
KV/compressor-frontier state back to the pre-verify snapshot, then re-decode
each accepted token one at a time through the same single-token kernel plain
decode uses (
metal_graph_eval_token_raw_swa). It just wasn't used for fullaccepts.
Fix
Minimal diff: removes the full-accept fast path entirely. Every accept (full
or partial) now falls through to the existing rollback+replay path
unconditionally, so post-accept KV state and logits are always re-derived
through the same code path plain greedy decode uses -- bit-identical by
construction, not by numerical luck.
No new code, no new flags, no dialect/quantization changes. Zero effect when
no drafter is configured (
draft_nis never reached in that case).Identity-test evidence
Resident IQ2XXS quant of DeepSeek-V4-Flash + a DSpark support GGUF (
--mtp <support.gguf> --dspark),--cuda --temp 0, drafter-enabled vs no-drafter,byte-for-byte
diff, built from this branch (based on currentmain):Both prompts were confirmed to diverge on unpatched
mainbefore this fix(the second one is the original reproducer from the linked issue, extended
past its divergence point here). No divergence observed post-fix in either
case.
Overhead (stated plainly)
This fix makes every accepted token -- including full accepts, which
previously skipped the replay -- pay for one extra fused single-token
forward pass. On our hardware/pairing (NVIDIA GB10, resident IQ2XXS +
this drafter), that increases drafter-enabled generation slowdown vs.
no-drafter from about -5% to roughly -14% to -17% across a few prompts
(single runs, not a full statistical sweep). This is the honest cost of
correctness here: the alternative (a narrower fix that re-derives only the
compressor-frontier projection instead of the full forward pass) is possible
in principle but a larger, riskier change, and wasn't attempted in this PR --
happy to discuss as a follow-up if there's interest.
Testing
make cuda-spark: clean rebuild, zero warnings, on this branch (based oncurrent
main).🤖 Generated with Claude Code