Skip to content

spec: add DSpark speculative decoding - #25173

Merged
ggerganov merged 17 commits into
ggml-org:masterfrom
wjinxu:dspark-upstream
Jul 28, 2026
Merged

spec: add DSpark speculative decoding#25173
ggerganov merged 17 commits into
ggml-org:masterfrom
wjinxu:dspark-upstream

Conversation

@wjinxu

@wjinxu wjinxu commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

This PR adds DSpark speculative decoding, layered on the merged DFlash drafter. DSpark (DeepSeek + PKU, 2026 — "Confidence-Scheduled Speculative Decoding with Semi-Autoregressive Generation", the DeepSpec repo) is DFlash plus a small semi-autoregressive Markov head: where DFlash takes an independent argmax at each block position (every position marginalizes over all possible predecessors, so acceptance decays along the block), DSpark adds a low-rank, previous-token-conditioned logit bias and samples the block left-to-right, so each draft conditions on the one actually sampled before it. This lifts accepted length at near-zero extra draft cost.

DSpark reuses the entire DFlash machinery unchanged — the encoder/decoder graph, target-layer feature extraction (llama_set_embeddings_layer_inp / _nextn), KV-cache injection, and the verify/accept path. The only additions are:

  • a new draft architecture dspark (llama_model_dspark : llama_model_dflash) that reuses the DFlash graph and additionally loads the Markov head (markov_w1, markov_w2) and an optional confidence head; it shares the target's token-embeddings / lm_head (same as DFlash);
  • a new speculative type draft-dspark (common_speculative_impl_draft_dspark : common_speculative_impl_draft_dflash) that reuses process() (extraction + injection) and overrides only draft(): the block is anchor-first (position 0 already predicts the first draft token) and sampled with the Markov bias bias(prev) = markov_w2 · markov_w1[prev], computed on-device (llama_dspark_markov_bias);
  • a Qwen3DSparkModel converter.

Greedy decoding is lossless: the Markov bias only changes which tokens are proposed; every draft is still verified against the target, so the output is identical to non-speculative greedy.

The confidence head is converted/loaded but not used at inference in this PR (phase 1); the draft-quality win from the Markov head is self-contained and is what the numbers below measure.

How to run

Complete example from scratch (Qwen3-8B). Drafts for other sizes are on the same org: deepseek-ai/dspark_qwen3_{4b,8b,14b}_block7.

1. Get the models — target + its DSpark draft:

huggingface-cli download Qwen/Qwen3-8B --local-dir Qwen3-8B
huggingface-cli download deepseek-ai/dspark_qwen3_8b_block7 --local-dir dspark_qwen3_8b

2. Convert to GGUF — the draft ships no tokenizer and reuses the target's, so pass --target-model-dir:

python convert_hf_to_gguf.py Qwen3-8B --outtype bf16 --outfile Qwen3-8B.gguf
python convert_hf_to_gguf.py dspark_qwen3_8b --outtype bf16 \
    --target-model-dir Qwen3-8B --outfile Qwen3-8B-DSpark.gguf

You may quantize the target (e.g. llama-quantize Qwen3-8B.gguf Qwen3-8B-Q4_K_M.gguf Q4_K_M); keep the draft bf16 — it's tiny, and acceptance is unaffected by target quant.

3. Build with CUDA:

cmake -B build -DGGML_CUDA=ON && cmake --build build --config Release -j

4. Run — the only DSpark-specific flags are -md <draft> and --spec-type draft-dspark
(--spec-draft-n-max = draft tokens per step; the released checkpoints use block size 7):

./build/bin/llama-server -m Qwen3-8B.gguf -md Qwen3-8B-DSpark.gguf \
    --spec-type draft-dspark --spec-draft-n-max 7 \
    --temp 0 --top-k 1 -np 1 -c 4096 -ngl 99 -fa on --jinja

5. Send a request (the server logs draft acceptance = ... per request):

curl http://localhost:8080/v1/chat/completions -H "Content-Type: application/json" -d '{
  "messages": [{"role": "user", "content": "Explain the Pythagorean theorem."}],
  "temperature": 0, "max_tokens": 256
}'

llama-cli works the same way (-m ... -md ... --spec-type draft-dspark). Note: draft-dspark needs the target's hidden states (KV-cache injection), so use llama-server / llama-cli — the speculative-simple example does not drive that path.

Performance

SpeedBench (llama.cpp's own tools/server/bench/speed-bench)

Qwen3-8B (bf16), matched --spec-draft-n-max 7, qualitative split (11 categories), greedy. Baseline is the same server with no draft model. DSpark reaches 1.88× overall decode speedup vs baseline (DFlash is 1.55×), and beats the merged DFlash on every one of the 11 categories (overall 1.21×).

DSpark vs baseline:

category       base_avg_pred_t/s  spec_avg_pred_t/s  decode_speedup  base_avg_latency  spec_avg_latency  latency_speedup  accept_rate
-------------  -----------------  -----------------  --------------  ----------------  ----------------  ---------------  -----------
coding         58.16              123.57             2.12x           11.172s           5.458s            2.05x            0.3219
humanities     58.22              99.06              1.70x           9.573s            5.646s            1.70x            0.2340
math           58.21              109.86             1.89x           10.313s           5.409s            1.91x            0.2840
qa             58.23              107.32             1.84x           8.313s            4.486s            1.85x            0.2659
rag            57.91              123.54             2.13x           9.521s            4.639s            2.05x            0.3264
reasoning      58.21              99.29              1.71x           9.570s            5.622s            1.70x            0.2347
stem           58.19              98.92              1.70x           8.827s            5.205s            1.70x            0.2332
writing        57.82              111.32             1.93x           9.765s            5.282s            1.85x            0.2807
multilingual   58.18              121.96             2.10x           8.691s            4.250s            2.05x            0.3187
summarization  58.36              102.74             1.76x           5.309s            3.001s            1.77x            0.2530
roleplay       58.20              102.56             1.76x           14.139s           8.274s            1.71x            0.2454
overall        58.15              109.10             1.88x           9.563s            5.207s            1.84x            0.2698

DSpark vs the merged DFlash (same --spec-draft-n-max 7):

category       dflash_avg_pred_t/s  dspark_avg_pred_t/s  decode_speedup  dflash_avg_latency  dspark_avg_latency  latency_speedup  accept_rate
-------------  -------------------  -------------------  --------------  ------------------  ------------------  ---------------  -----------
coding         106.00               123.57               1.17x           6.343s              5.458s              1.16x            0.3219
humanities     83.61                99.06                1.18x           6.674s              5.646s              1.18x            0.2340
math           90.48                109.86               1.21x           6.529s              5.409s              1.21x            0.2840
qa             85.20                107.32               1.26x           5.650s              4.486s              1.26x            0.2659
rag            98.61                123.54               1.25x           5.733s              4.639s              1.24x            0.3264
reasoning      83.51                99.29                1.19x           6.681s              5.622s              1.19x            0.2347
stem           83.60                98.92                1.18x           6.154s              5.205s              1.18x            0.2332
writing        90.28                111.32               1.23x           6.443s              5.282s              1.22x            0.2807
multilingual   102.94               121.96               1.18x           5.016s              4.250s              1.18x            0.3187
summarization  85.85                102.74               1.20x           3.606s              3.001s              1.20x            0.2530
roleplay       79.96                102.56               1.28x           10.451s             8.274s              1.26x            0.2454
overall        90.00                109.10               1.21x           6.298s              5.207s              1.21x            0.2698

Hardware: RTX 4090. Target Qwen/Qwen3-8B (bf16), draft deepseek-ai/dspark_qwen3_8b_block7 (bf16). Greedy (--temp 0 --top-k 1), no-thinking, --spec-draft-n-max 7. Baseline = same llama-server with no draft model. DFlash is the merged drafter (z-lab/Qwen3-8B-DFlash, b16), run at the same matched draft size for an apples-to-apples comparison. Per-domain aggregate over the listed prompt counts.

Losslessness

Greedy decoding is lossless by construction (the draft is verified against the target). Output is coherent and matches non-speculative greedy.

Qwen3-4B, target bf16

DSpark vs baseline (DFlash was not benchmarked at 4B — no nested-schema 4B DFlash draft available):

Domain Baseline t/s DSpark t/s (accept) DSpark
GSM8K (40) 103.1 354.0 (75.3%) 3.43×
MATH500 (30) 102.9 341.3 (71.7%) 3.32×
HumanEval (30) 103.9 340.0 (72.9%) 3.27×
MBPP (30) 103.6 281.4 (57.2%) 2.72×
MT-Bench (30) 102.8 190.4 (31.7%) 1.85×
geomean 2.85×

Qwen3-8B, target bf16

Domain Baseline t/s DFlash t/s (accept) DSpark t/s (accept) DFlash DSpark
GSM8K (40) 58.5 182.4 (53.7%) 237.3 (78.9%) 3.12× 4.06×
MATH500 (30) 58.5 195.7 (59.2%) 223.2 (72.8%) 3.35× 3.82×
HumanEval (30) 59.1 238.8 (77.2%) 241.4 (81.7%) 4.04× 4.08×
MBPP (30) 59.6 177.3 (53.3%) 193.1 (63.7%) 2.98× 3.24×
MT-Bench (30) 58.6 93.5 (19.7%) 120.4 (31.3%) 1.60× 2.05×
geomean 2.89× 3.35×

Qwen3-8B, target Q8_0

Domain Baseline t/s DFlash t/s (accept) DSpark t/s (accept) DFlash DSpark
GSM8K (40) 100.6 246.4 (53.2%) 322.9 (77.8%) 2.45× 3.21×
MATH500 (30) 100.5 266.2 (59.2%) 305.7 (72.2%) 2.65× 3.04×
HumanEval (30) 101.3 319.5 (76.5%) 327.9 (81.4%) 3.15× 3.24×
MBPP (30) 102.2 242.4 (54.3%) 268.0 (64.3%) 2.37× 2.62×
MT-Bench (30) 100.7 126.7 (19.3%) 167.8 (31.4%) 1.26× 1.67×
geomean 2.28× 2.68×

Qwen3-8B, target Q4_K_M

Domain Baseline t/s DFlash t/s (accept) DSpark t/s (accept) DFlash DSpark
GSM8K (40) 155.4 259.0 (52.9%) 340.7 (77.4%) 1.67× 2.19×
MATH500 (30) 155.2 284.9 (60.4%) 326.1 (73.9%) 1.84× 2.10×
HumanEval (30) 156.5 314.3 (71.1%) 332.0 (78.4%) 2.01× 2.12×
MBPP (30) 157.5 257.5 (55.5%) 281.3 (66.0%) 1.63× 1.79×
MT-Bench (30) 155.5 135.2 (19.7%) 174.4 (30.6%) 0.87× 1.12×
geomean 1.54× 1.81×

DSpark beats the merged DFlash on every domain (higher accept rate and higher throughput), for a ~1.16× geomean speedup over DFlash. The gains are largest on reasoning (GSM8K +25pp accept, 1.30× over DFlash) and open chat (MT-Bench, 1.29×); on code (HumanEval) the two are close as both already accept ~80%.

Confidence Evaluation

Qwen3-8B Q4_K_M target, SPEED-Bench qualitative, 132 completed samples, OSL 512.

Concurrency Unified KV conf_min Average decode t/s Average latency Acceptance Total elapsed
1 No 0.0 198.14 2.171 s 36.6% 286.58 s
1 Yes 0.0 195.64 2.193 s 36.6% 289.53 s
1 Yes 0.3 190.16 (-2.8%) 2.276 s (+3.8%) 41.6% 300.52 s
1 Yes 0.6 191.74 (-2.0%) 2.189 s (-0.2%) 62.5% 289.04 s
8 No 0.0 61.43 7.014 s 35.7% 119.68 s
8 Yes 0.0 54.81 7.621 s 35.1% 129.49 s
8 Yes 0.3 56.98 (+4.0%) 7.425 s (-2.6%) 42.4% 126.65 s
8 Yes 0.6 59.01 (+7.7%) 7.360 s (-3.4%) 62.7% 125.59 s
32 No 0.0 16.20 24.662 s 31.3% 122.29 s
32 Yes 0.0 15.17 28.326 s 32.1% 133.05 s
32 Yes 0.3 15.53 (+2.4%) 27.341 s (-3.5%) 40.9% 129.85 s
32 Yes 0.6 17.26 (+13.8%) 24.511 s (-13.5%) 59.8% 117.40 s

Percentage changes on the unified-KV rows are relative to the same-concurrency unified-KV conf_min=0.0 baseline.

Confidence pruning has no benefit at concurrency 1, begins to help at concurrency 8.The intended operating environment is high-concurrency serving with packed/unified KV batching.

Do not enable confidence pruning with non-unified KV at high concurrency. Ragged verification causing CUDA Graph reuse to collapse.

Future work

  • Confidence head (phase 2): wire the confidence-scheduled prefix pruning, with the paper's Sequential Temperature Scaling calibration. The big serving win in the paper comes from the batched scheduler, which is a separate, larger change.
  • Markov-bias graph reuse: the bias is computed as a tiny per-step ggml graph on the draft context's scheduler; building it once per block and re-running with new inputs would cut overhead. A fused bias+argmax kernel is a further option but would add a backend-specific op (the current path is pure ggml, no new operator).

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: Yes, use Claude to help discuss and design the DSpark architecture, ask clarifying questions, and assist with writing tests. Everything remains under my control, and I reviewed every single line of AI-generated code.

@github-actions github-actions Bot added model Model specific conversion labels Jun 30, 2026
@ggml-gh-bot

This comment was marked as resolved.

@wjinxu
wjinxu force-pushed the dspark-upstream branch from f3b83cd to d74ff77 Compare June 30, 2026 14:16
@github-actions github-actions Bot added the testing Everything test related label Jun 30, 2026
@wjinxu
wjinxu force-pushed the dspark-upstream branch from d74ff77 to 37f2513 Compare June 30, 2026 14:39
@wjinxu
wjinxu marked this pull request as ready for review June 30, 2026 17:00
@wjinxu
wjinxu requested review from a team, CISC, JohannesGaessler and ggerganov as code owners June 30, 2026 17:00
@wjinxu

wjinxu commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Hi @CISC @ggerganov , this adds DSpark speculative decoding on top of the merged DFlash drafter (#22105). It's a small change — a new dspark draft arch and draft-dspark spec type that reuse DFlash's graph, feature extraction, KV-cache injection and verify path unchanged; the only new logic is the semi-autoregressive Markov head in draft(). Greedy stays lossless.

I benchmarked it against the merged DFlash using DeepSeek's released Qwen3 DSpark drafts. On Qwen3-8B at bf16 / Q8_0 / Q4_K_M, DSpark beats DFlash on every domain (e.g. GSM8K bf16 4.06× vs 3.12×; full per-domain tables in the PR description).

I believe it's ready for review and I'm happy to walk through any part of it.

@ruixiang63

ruixiang63 commented Jun 30, 2026

Copy link
Copy Markdown
Member

Can you run SpeedBench to do the full comparison between DFlash and DSpark with the same --spec-draft-n-max? https://github.com/ggml-org/llama.cpp/tree/master/tools/server/bench/speed-bench

@CISC
CISC requested a review from ruixiang63 June 30, 2026 17:47
Comment thread conversion/qwen.py Outdated
Comment thread conversion/qwen.py Outdated
Comment thread conversion/qwen.py Outdated
@wjinxu

wjinxu commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@ruixiang63 I've run the SpeedBench test set as you suggested, and updated the results in the PR description. DSpark does outperform DFlash across the board.

@nipeone

nipeone commented Jul 1, 2026

Copy link
Copy Markdown

could you give some examples how to use?

@wjinxu

wjinxu commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

could you give some examples how to use?

Good point — I've updated the PR description with a more detailed, copy-pasteable end-to-end example (download → convert → build → run → curl). Let me know if anything's unclear.

Comment thread src/llama-model.cpp
@wjinxu
wjinxu force-pushed the dspark-upstream branch from d8b38f2 to 47f3442 Compare July 1, 2026 05:17
@am17an

am17an commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

DSV4 support was merged in #24162, ideally this PR should cover that model as well and try to replicate a similar speedup

@wjinxu

wjinxu commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

DSV4 support was merged in #24162, ideally this PR should cover that model as well and try to replicate a similar speedup

Thanks! DeepSeek hasn't open-sourced the DSpark weights for DeepSeek-V4 though — only the Qwen3 and Gemma4 drafts are released. So this PR covers Qwen3 for now, and I'll add Gemma4 as a small follow-up.

@am17an

am17an commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

I think they're a part of the spec decoding module https://huggingface.co/deepseek-ai/DeepSeek-V4-Flash-DSpark i.e not distributed separately

@wjinxu

wjinxu commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

I think they're a part of the spec decoding module https://huggingface.co/deepseek-ai/DeepSeek-V4-Flash-DSpark i.e not distributed separately

Sorry, and thanks for the heads-up. For this PR I'd like to keep the scope a bit narrower for now - land the Qwen3 DSpark path first and get it solid, then add Gemma4 and DSV4 as follow-ups. Does that sound ok?

@am17an

am17an commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Okay, will try to review. From a cursory look it does not look like adding llama_dspark* is the right thing to do. Honestly don't have a good feeling about the PR, too much AI code.

@wjinxu

wjinxu commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Okay, will try to review. From a cursory look it does not look like adding llama_dspark* is the right thing to do. Honestly don't have a good feeling about the PR, too much AI code.

I agree that llama_dspark_* shouldn't be part of the API. The issue is that the Markov bias computation (W2 @ W1[prev]) needs the draft weights and has to run on the backend - I measured it, and doing it on the host is too slow. But the DSpark drafter lives in common/, which can't reach the draft weights, so once the API is removed there's no way to trigger that computation from common/.

@wjinxu

wjinxu commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

spectypelog.txt So i downloaded the dspark-upstream directly, and compiled it myself, using CUDA, tested everything normally and it works Tried to use --spec-type draft dspark and it says that it is an invalid argument Is there some extra files I need to download after getting the dspark-upstream.zip?

I found that your command is incorrect. If you want to put the command on a single line, please remove the . Alternatively, you can copy the command from my description and keep the line breaks as they are.

@FHRacing

Copy link
Copy Markdown

I would have posted results from speed-bench, but since they were going to take 12+ hours, i decided not to
Using the latest commit on dspark-upstream, using ZLUDA (CUDA for AMD GPUs) on a Radeon 780M with 16GB of LPDDR5 8400
Tested with Qwen 3.5 0.8B Q8 quant, and a Dspark BF16 (Tested with a BF16 version of base model as well, but perf suffered quite a bit
Baseline tokens got up to 73.35tok/s, also hitting a minimum 59.5tok/s
DSpark tokens got up to 46.54tok/s, and also hitting a minimum of 28.20tok/s
Coding generation felt way faster than 46tok/s, kind of impressed

@ngxson

ngxson commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

/bot review

@ggml-gh-bot

ggml-gh-bot Bot commented Jul 26, 2026

Copy link
Copy Markdown
Automated code review

Review of PR #25173 — adding the draft-dspark speculative draft type (DFlash backbone + Markov/confidence head).

I traced the new graph code in src/models/dflash.cpp, the drafting logic in common/speculative.cpp, and the conversion in conversion/qwen.py.

Blocking

(point 1) Markov head assumes all drafting sequences use an identical block size, and silently corrupts logits when they don't. In build_dspark_markov_head (src/models/dflash.cpp), block_drafts = n_tok / n_blocks and the strided views (token_stride, base_stride, base_i) all rely on the batch being a uniform grid of n_blocks contiguous blocks each of length block_drafts. The only guards are n_blocks == 0 and n_tok % n_blocks != 0 (and a dead block_drafts > block_size). But the DSpark drafting loop in common/speculative.cpp:1137-1144 computes a per-sequence n_draft = std::min(params.n_max, dp.n_max) — and dp.n_max is explicitly used to clamp per-sequence by remaining context (common/speculative.h:42 "can be used to constraint the max draft based on the remaining context size"). If two drafting sequences clamp to different n_block_tokens whose sum is still divisible by n_seqs_unq, the guard passes but the strided views read across block boundaries, producing a wrong Markov chain and a corrupted res->t_logits, which is then sampled. Fix by enforcing/validating uniform block size (e.g. bail when the per-seq n_block values differ, or force every drafting seq to the same n_draft when is_dspark), rather than only checking divisibility.

Will slow the review

(point 2) p_min is silently ignored for DSpark. The dflash branch breaks on cur_p->data[0].p < params.p_min (common/speculative.cpp ~1207), but the new is_dspark branch never checks p_min. The param is still logged (... p_min=%.2f, conf_min=%.2f ...) and accepted via --spec-draft-p-min, so a user setting it for dspark gets no behavior. Either apply p_min here too or document explicitly that conf-min replaces it for dspark.

(point 3) conf_min > 0 with a draft model that has no confidence head reads stale/wrong embeddings. res->t_h_nextn is only set by build_dspark_markov_head inside if (cat_conf); without dspark_conf_proj, the decoder never produces t_h_nextn for this decode, so llama_get_embeddings_nextn(ctx_dft) returns whatever embd_nextn last held — for DSpark that is the prior llama_encode output (dimension n_embd_inp_enc, not n_embd_dec). Then conf[(size_t) idx * n_embd_dec] reads mismatched data. Suggest: record at construction whether the draft model actually has a conf head (presence of conf_proj.weight), and if conf_min > 0 is requested without it, warn and disable conf gating (or hard-error).

(point 4) Verify the conversion's block_size hparam key for the DeepSpec schema. DSparkModel.__init__ normalizes target_layer_ids and mask_token_id into dflash_config, but not block_size. DFlashModel.set_gguf_parameters reads self.hparams.get("block_size", 16) (conversion/qwen.py:668). If the DeepSpec config names that field differently, the GGUF silently gets block_size=16, which then both mis-clamps n_max and mis-sizes the Markov chain in build_dspark_markov_head. Confirm the key matches, or normalize it in DSparkModel.__init__.

(point 5) No tests or acceptance/perf numbers. This adds a new speculative draft type, a new conversion path, and new GGUF tensors, with no test, no sample GGUF, and no acceptance-rate / latency numbers. The skill guidance for new drafting features expects at least a converted sample plus measured acceptance on a supported target (e.g. Qwen3-4B). Add a smoke conversion + a speculative run, and ideally wire draft-dspark into the speculative test harness.

Nits

(point 6) build_dspark_markov_head parses dflash.block_size with std::stoi(it->second) on every draft decode (src/models/dflash.cpp:139), which can throw on malformed GGUF and is inconsistent with common/speculative.cpp:957 which uses std::atoi. Use atoi (or cache the parsed value on the model once) and avoid exceptions in the decode path.

(point 7) The no-op returns in build_dspark_markov_head (n_blocks == 0, n_tok % n_blocks != 0) are silent. Add a LOG_DBG so silent degradation to "no Markov bias applied" is diagnosable; the block_drafts > block_size return is dead given clamping and can be dropped.

(point 8) tools/server/server-schema.cpp:212 adds speculative.conf_min inside the existing #if 0 block, so it has no runtime effect (server-side spec tuning is disabled). That's consistent with the surrounding disabled fields, but worth a one-line acknowledgement so reviewers don't think it's wired through the API.

(point 9) The clamp warning text (common/speculative.cpp:970) "exceeds the trained block size %d -- clamping to %d" prints identical numbers for DSpark (block_size vs block_size). Minor wording tweak.

The overall approach — reusing the DFLASH architecture, gating the Markov head on tensor presence, and routing the confidence output through the existing t_h_nextn/embd_nextn plumbing — is reasonable and avoids duplicating a model class. The indexing math in the position-major -> block-major reshape/permute is correct. Main concern is the uniform-block-size assumption in (point 1).

This review was generated automatically by pi coding agent using zai-org/GLM-5.2. It may contain mistakes. Maintainers make the final call.

@ngxson

ngxson commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

@wjinxu can you address the first point in the auto review? (feel free to tell if it's valid or not - other points are optional)

…tion

The DSpark markov head views the draft batch as a uniform [n_seqs x block]
grid, but the per-seq dp.n_max clamp could produce blocks of different
sizes, silently corrupting the strided views and the resulting logits.

Drop the clamp and always draft the full n_max block for every sequence:
dp.n_max is already enforced by the central truncation in
common_speculative_draft(), the same way eagle3 handles it.

Co-authored-by: Zaire404 <3147879462@qq.com>
@wjinxu

wjinxu commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Same approach as eagle3: draft a full n_max block for every sequence and let the central truncation in common_speculative_draft() enforce dp.n_max, which makes the equal-size blocks the markov head needs hold by construction.
It can waste a few draft-model tokens near the context tail — the same cost eagle3 already pays — in exchange for a strict simplification with no special cases.

@ngxson

ngxson commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

I only did a quick code review, not a deeper logic review, but all seems good. Just running another auto review to see if there is any other blocking issues

/bot review

@ggml-gh-bot

ggml-gh-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown
Automated code review

Review of PR #25173 (DSpark speculative decoding). I read pr.diff, the DFlash base code in src/models/dflash.cpp and common/speculative.cpp, the converter (conversion/qwen.py), gguf-py tensor mapping/constants, and the context-output plumbing in src/llama-context.cpp.

Correctness (should fix before merge)

(point 1) --spec-draft-conf-min > 0 reads stale embeddings when the draft has no confidence head. In common_speculative.cpp the DSpark branch gates the conf read only on params.conf_min > 0.0f:

const float * conf = params.conf_min > 0.0f ? llama_get_embeddings_nextn(ctx_dft) : nullptr;
...
if (conf && conf[(size_t) idx * n_embd_dec] < params.conf_min) { break; }

But the graph only writes t_h_nextn (the confidence tensor) when model.dspark_conf_proj is present (build_dspark_markov_head: ggml_tensor * conf_inp = model.dspark_conf_proj ? res->t_embd : nullptr;). If the draft is loaded as draft-dspark but lacks conf_proj (e.g. a future DSpark checkpoint without the head, or a plain DFlash draft mistaken for DSpark), t_h_nextn is not produced by the token-batch decode, and llama_get_embeddings_nextn() returns the stale embd_nextn buffer from the previous llama_encode() in process() (src/llama-context.cpp:1533 only copies when t_h_nextn is non-null). The block is then truncated against feature values, not confidence. The draft() side must mirror the graph's guard — gate on whether the confidence head actually loaded (detect once at construction, or expose a model getter) and LOG_WRN/zero conf_min when absent.

(point 2) Silent fallback in build_dspark_markov_head (src/models/dflash.cpp). The three early returns (n_blocks == 0, n_tok % n_blocks != 0, block_drafts > block_size) leave res->t_logits as the raw, unbiased DFlash logits with no warning. The structural invariants (n_tok % n_blocks == 0, block_drafts <= block_size) should never trigger given n_max is clamped to block_size; a silent return here degrades DSpark to DFlash quality invisibly (and a ubatch-split that partial-blocks could hit the divisibility guard coincidentally). Prefer GGML_ASSERT for the invariants, or at least LOG_WRN, so a misconfigured or split ubatch fails loudly instead of silently discarding the Markov bias.

Will slow the review

(point 3) p_min silently ignored for draft-dspark. The DFlash branch breaks on cur_p->data[0].p < params.p_min; the DSpark branch omits that check entirely, so --spec-draft-p-min is a no-op for DSpark despite still being parsed/logged. Either restore the gate (the Markov-biased top-1 p is still a valid probability) or note in the PR/docs that p_min doesn't apply to DSpark.

(point 4) std::stoi on GGUF metadata. build_dspark_markov_head reads dflash.block_size via std::stoi(it->second), which throws std::out_of_range/std::invalid_argument on malformed metadata. The sibling code in common/speculative.cpp:958 reads the same key with std::atoi (no throw). GGUF metadata is attacker-controlled; use atoi or wrap in try/catch for consistency and robustness.

(point 5) Description vs. code disagree on whether confidence is "used at inference". The description says "The confidence head is converted/loaded but not used at inference in this PR (phase 1)", yet this PR adds --spec-draft-conf-min (arg.cpp, server-schema), wires the read into draft(), computes the head in the graph, and the perf section benchmarks conf_min=0.3/0.6. Pick one story: if it's truly phase-2, the flag/server field shouldn't ship here (premature public surface per server-scope guidance); if it's live, fix the description. As written a maintainer can't tell what's intended.

(point 6) Shared DFlash draft() path was restructured, not just extended. The dp.n_max pre-clamp was removed (commit "defer dp.n_max to the central truncation") and the sampling loop was split into is_dspark/else branches. The dp.n_max change is behaviorally covered by the post-truncate at common/speculative.cpp:2632, but it changes the block size forwarded by the DFlash decoder when dp.n_max < params.n_max (previously a smaller block, now always params.n_max). This is an unannounced behavior change to existing DFlash bundled into a DSpark PR — verify no DFlash regression and call it out in the description.

Nits

(point 7) Description overstates the architecture. It claims "a new draft architecture dspark (llama_model_dspark : llama_model_dflash)" and "a new draft architecture dspark", but the code adds no new arch enum and no subclass: the Markov head is detected by the presence of markov_w1.weight inside llama_model_dflash::load_arch_tensors, and the converter sets model_arch = DFLASH. The implementation is actually cleaner than described (reuses DFLASH, no near-duplicate) — update the description so reviewers aren't looking for a class that doesn't exist.

(point 8) Minor wording: the anchor-view comment "anchor (committed last) token of every block: token 0" conflates "committed last" (the accepted target token) with "token 0 of the block input". The anchor is dp.id_last, which is token 0 of the block; "committed last" reads as an output-position description. Clarify.

This review was generated automatically by pi coding agent using zai-org/GLM-5.2. It may contain mistakes. Maintainers make the final call.

@ngxson

ngxson commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

I think point 1 and 2 worth having a look, but feel free to tell me if it's valid / not valid otherwise. Other points are optional and can be follow-up if needed

…e conf head

With the draft batch always submitting equal-size n_max blocks, a
non-divisible token count can only mean the batch was split across
ubatches or a caller broke the layout - fail loudly instead of silently
dropping the markov bias. The block_drafts > block_size early return
stays: worst-case graph reserve passes legitimately build with
n_seq_tokens > block_size.

Also make conf_proj required when the markov head is present: the
confidence head is part of the DSpark checkpoint format, and a missing
head would otherwise leave --spec-draft-conf-min silently reading stale
embeddings instead of confidences.

Co-authored-by: Zaire404 <3147879462@qq.com>
@wjinxu

wjinxu commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

(point 1) --spec-draft-conf-min > 0 reads stale embeddings when the draft has no confidence head. In common_speculative.cpp the DSpark branch gates the conf read only on params.conf_min > 0.0f:

The confidence head is part of the DSpark checkpoint format (every DSpark model ships conf_proj alongside the markov head), so conf_proj is now a required tensor whenever the markov head is present — a checkpoint lacking it fails at load time instead of silently misbehaving at runtime.

The one remaining way to reach it would be pointing --spec-type draft-dspark at a plain DFlash draft and setting --spec-draft-conf-min — a stacked misconfiguration we're treating as user error rather than a supported path.

(point 2) Silent fallback in build_dspark_markov_head (src/models/dflash.cpp).

Partially done. The divisibility check is now a GGML_ASSERT: with the batch always submitting equal-size blocks (see the earlier fix), a non-divisible token count can only mean the batch was split across ubatches or a caller broke the layout, so it should fail loudly — agreed.

The block_drafts > block_size early return has to stay, we tried asserting it and the server aborts at startup, before serving a single request.

Comment thread common/common.h Outdated
float p_split = 0.1f; // speculative decoding split probability
float p_min = 0.0f; // minimum speculative decoding probability (greedy)

float conf_min = 0.0f; // DSpark: min predicted acceptance from the confidence head (0 = disabled)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reuse the p_min and avoid introducing the new conf_min?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

p_min and conf_min express the same thing - the minimum predicted
survival probability for a drafted position - differing only in how the
estimate is obtained: token probability for regular drafters, the
trained confidence head for DSpark. The DSpark readback never used
p_min, so reuse it for the confidence threshold and drop the separate
--spec-draft-conf-min flag. Both defaulted to 0 (disabled), so behavior
is unchanged.

Co-authored-by: Zaire404 <3147879462@qq.com>
@dev-shaiban

This comment was marked as off-topic.

@MrDrMcCoy

Copy link
Copy Markdown

Waiting for this PR to be merged.😁

Same.

Comments like these are spam. Pull requests are for discussing the code itself to move it forward. If you don't have a meaningful contribution or constructive criticism relating to the code, take it elsewhere. They don't increase the priority or make things go faster. All they do is waste the time of every subscriber to the thread. The only positive analogue to "is it done yet?" is "what tasks remain before this is ready to merge?", because the answer is useful to contributors.

When progress has stalled for a long time, you may politely ask if the work will continue. This is clearly not the case here. This PR is active, and as such the answer is implicit.

None of us are entitled to the time and effort of others. Stop it.

Comment thread src/models/dflash.cpp
conf = ggml_cont(ctx0, ggml_permute(ctx0, conf, 0, 2, 1, 3));
conf = ggml_reshape_2d(ctx0, conf, 1, n_tok);

conf = ggml_repeat(ctx0, conf, res->t_embd);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this repeat a placeholder? If yes, add a TODO that later the correct confidences will be computed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this looks like a workaround to reuse the llama_get_embeddings_nextn for extracting the confidences.

That's fine for now - just add a // note: ... to avoid confusion in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Requested in review: the ggml_repeat only adapts the [1, n_tok]
confidences to the n_embd-wide embd_nextn transport so that
llama_get_embeddings_nextn can be reused - not a placeholder.

Co-authored-by: Zaire404 <3147879462@qq.com>
Comment thread src/models/dflash.cpp Outdated
Comment thread src/models/dflash.cpp
}
conf = ggml_sigmoid(ctx0, conf);

cat_conf = cat_conf ? ggml_concat(ctx0, cat_conf, conf, 1) : conf;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These concats might be possible to avoid and optimize via ggml_set_inplace, similar to:

v = ggml_set_inplace(ctx0, v, o_ch, v->nb[1], v->nb[2], v->nb[3], chunk * v->nb[2]);

Can be tried in a follow-up PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@ggerganov

Copy link
Copy Markdown
Member

I haven't tested the code. Unless @ruixiang63 has additional comments, we can merge I think?

@ruixiang63

Copy link
Copy Markdown
Member

I haven't tested the code. Unless @ruixiang63 has additional comments, we can merge I think?

Yes, looks good to me! Let's merge. @ggerganov

@ruixiang63 ruixiang63 added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Jul 28, 2026
@ggerganov
ggerganov merged commit 8407527 into ggml-org:master Jul 28, 2026
1 check passed
@ggerganov

Copy link
Copy Markdown
Member

Added DSpark here: https://huggingface.co/ggml-org/Qwen3-8B-GGUF

Let me know if someone gives it a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conversion documentation Improvements or additions to documentation merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. model Model specific server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: DSpark confidence-scheduled verification & semi-autoregressive drafting