Reconcile DSpark runtime (#480) + B2 rejection sampling/adaptive block sizing (#482)#502
Reconcile DSpark runtime (#480) + B2 rejection sampling/adaptive block sizing (#482)#502stephenlthorn wants to merge 10 commits into
Conversation
Implements Chen et al. (2023) / Leviathan et al. (2023) rejection sampling for the DSpark speculative decode path. At temp=0 (default), behavior is unchanged (greedy argmax matching). At temp>0, accepts draft tokens with probability min(1, target_prob/draft_prob) and samples correction tokens from the residual distribution. Key properties: - Distribution-exact: output is drawn from EXACTLY the target model's distribution regardless of draft quality - Token-identical to non-spec decode at temp=0 (greedy) - Numerically stable: all computations in log-probability space (handles 129K vocab without overflow) - Zero overhead when disabled: buffer only allocated when DS4_SPEC_TEMP is set Activation: DS4_SPEC_TEMP=0.6 DS4_SPEC_RNG_SEED=42 ./ds4 ... Based on the community analysis from ds4#468: @lobanov proved +30-48% speedup with component measurements; this wires the acceptance algorithm into the decode loop for end-to-end lossless spec decode. Co-Authored-By: Tang Feng (Audrey) <audreyt@audreyt.org> Co-Authored-By: lobanov <lobanov@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ARDD adversarial review found two critical bugs: 1. OFF-BY-ONE (CRITICAL): metal_graph_verify_suffix_tops row[i] is the target distribution AFTER processing drafts[i], predicting drafts[i+1]. B2 was using row[i] to verify drafts[i] — wrong distribution entirely. Fix: prepend s->logits (from previous target eval) as row 0, shift verify output by one. Now drafts[0] verifies against s->logits and drafts[j>0] verifies against verify_row[j-1]. 2. RNG RESEEDED PER CALL (HIGH): b2_rng was stack-local, reseeded from time(NULL) on every speculative eval call. At >1 tok/s, multiple calls within the same second got identical seeds, producing correlated random sequences. Fix: persist RNG state in ds4_session struct, seed once. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Conservative-then-aggressive strategy: start at block=2 (near-baseline), escalate to full block after seeing full commits (structured output detected), drop back on partial commits. Tracks previous cycle's acceptance via dspark_prev_accepted/drafted fields in session struct. Opt-in via DS4_DSPARK_ADAPTIVE=1 env var. Measured results (M5 Max 128GB, Q2 base + Q4K DSpark, clean bandwidth): JSON structured: +8.5% speedup (42.27 vs 38.95 tok/s) Markdown tables: +6.3% speedup (45.21 vs 42.54 tok/s) Code generation: -17% (needs higher acceptance — HyperDFlash path) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On-hardware validation (M4 Max 128GB, q2-imatrix base + Q4_K DSpark drafter)Correctness: PASS
The Generation tok/s by prompt type (
|
| Prompt | baseline | legacy_mtp | dspark_b2 | dspark_adaptive | dspark_b5 | Best DSpark vs baseline |
|---|---|---|---|---|---|---|
| json_repetitive | 28.40 | 31.44 | 31.54 | 37.44 | 38.10 | +34.2% (b5) |
| md_table | 31.53 | 31.57 | 30.55 | 33.75 | 31.91 | +7.0% (adaptive) |
| code_gen | 31.35 | 31.42 | 30.16 | 28.81 | 30.00 | -3.6% to 0% (wash) |
| prose | 31.31 | 31.29 | 26.81 | 21.68 | 21.21 | -31% to -32% (regression) |
This confirms @lobanov's original acceptance-rate problem (issue #468: "average acceptance 1.5-2 tokens, 5% slowdown") is resolved by this branch's combination of the prefix-checkpoint path (#480) and the B2 correction fix (#482) - we're seeing +34% on repetitive JSON with the same Q4_K drafter class on the same q2 base quant.
temp=0 identicality vs baseline
json/table/code: byte-identical across all 5 configs. Prose diverges on all 3 DSpark configs (legacy_mtp stays identical). Root cause: a near-tied target logit gets flipped by the drafter's slight numeric bias, and greedy decoding cascades the divergence from there. The worst_argmax_gap invariant still holds (every committed token is still argmax-correct against the target model at the moment it's verified) - so this is "target-argmax-replay lossless," not "byte-identical," on ties. Worth calling out explicitly in the README's DSpark section since it currently reads like byte-identical is the guarantee.
Findings
- block=5 regresses on prose (-32%): low acceptance means more rejected drafts to verify plus partial-commit replay cost. block=2 nearly halves the loss (-14%) but still trails baseline.
- Adaptive escalates to b5 under low acceptance and pays the same penalty as fixed b5 on prose. A ceiling (e.g. cap escalation at b3 unless recent acceptance is consistently full) would likely recover most of the structured-output win without the prose regression - flagging as a possible follow-up, happy to try it if useful.
- Prefill throughput is ~3% lower across the board with DSpark loaded (extra resident weights + verify overhead) - minor, not blocking.
- Single-sample-per-cell on an M4 Max; didn't control for thermal throttling. The b5 vs b2 prose gap (-32% vs -14%) should be re-run 2-3x before treating it as a stable number rather than noise.
DS4_SPEC_TEMP(B2 stochastic path) not yet exercised - all runs above are temp=0. That's a separate correctness surface (rejection sampling at temp>0) worth a dedicated follow-up pass.
Raw logs/output and a re-runnable bench script are on hand if useful for someone else to reproduce.
On-hardware validation on M4 Max (see PR antirez#502 comment) confirmed adaptive mode's single-step 2->full jump regresses low-acceptance workloads: one incidental full commit at block=2 escalated straight to block=5, and the next low-acceptance cycle then paid the full block=5 partial-commit replay cost before dropping back down (-31% on prose vs baseline). Stage the escalation instead: require 2 consecutive full commits before using the full block, one intermediate step at block=3. Drop back to block=2 immediately on any partial commit, same as before. Also document in the README that DSpark's temp=0 correctness guarantee is "every committed token is the target model's argmax on verify", not byte- identical output to a non-speculative run: near-tied logits can flip which side of the tie a drafter-influenced pass commits, which is expected and not a correctness bug, but was previously undocumented and looked like a stronger guarantee than the code actually provides.
|
Pushed a follow-up commit addressing the two items from the benchmark comment above:
Not yet re-benchmarked on real hardware — the person with the GPU/model loaded is queued to re-run the prose case (and ideally 2-3x to control for the thermal noise flagged earlier) against this staged version. Will report back with numbers once that's done. |
Re-benchmark: staged escalation does not fix the prose regressionFollow-up to the earlier validation comment. Re-ran only the prose case (3x, to rule out thermal noise) against the staged-escalation commit (7ad4ea2).
Correctness unaffected: Diagnosis: this was the wrong fix for the problemThe +0.8 t/s improvement is inside measurement noise, not a real fix. Staging the escalation only slows down how fast adaptive reaches block=5, it doesn't change the fact that block=5 (or even block=2) costs more than it saves once you're there:
RecommendationDon't present staged escalation as a prose fix - it isn't one, though it's still worth keeping as protection against future flaky single-commit escalation. Two actual paths forward, and I'd defer to whoever picks this up next on which:
Not picking one of these myself since it's a product tradeoff (peak structured-output speedup vs. worst-case prose safety) rather than a bug fix. Happy to implement whichever is preferred. |
Summary
This reconciles the two open DSpark PRs from issue #468 into one branch that builds cleanly and passes the model-free test suite:
DS4_DSPARK_ADAPTIVE)The two PRs share an earlier common scaffold but diverged afterward, so this isn't a plain merge - I cherry-picked both commit chains onto
main(skipping #482's skip-logits-replay commit + its own revert, which cancel out to a no-op) and hand-resolved one real conflict.Fix included
The conflict was in the partial-commit path of the speculative loop: audreyt's prefix-checkpoint fast-commit (skip replay when the batch verify already captured a KV row for each committed position) and machiabeli's B2 correction token (sampled fresh during rejection, never processed by the verify pass) interact badly - a correction token has no captured KV row, so routing it through the fast-commit path would silently commit against a stale/wrong cache row.
Fixed by gating the fast-commit path on
commit_has_correction: any commit that includes a B2 correction token always takes the frontier-restore + replay path, same as before #480's optimization existed. Also fixed a small leak ofb2_target_logitson the prefix-commit early return.Testing
On Apple Silicon (Metal), no GPU-heavy model runs yet on this machine:
make(Metal): clean, 0 warningsmake cpu: clean./ds4_test --dspark-binder --dspark-markov-bf16 --dspark-runtime --server: all pass--dspark-speculative-block,--mtp-verify-depth) self-skip withoutDS4_TEST_DSPARK/DS4_TEST_MTP- not yet run against a real DSpark GGUFI plan to run the real acceptance/speed benchmarks from #468 (block=2 vs block=5 vs adaptive, JSON/code/creative prompt mix) on a machine with the model loaded and will report back with numbers. Flagging this now as a reviewable integration point rather than waiting, since #480 and #482 are both stalled on being combined.
Thanks to @audreyt and @machiabeli for the actual implementation work - this PR is integration/reconciliation only, no new algorithmic contribution beyond the correction-token fix above.