Skip to content

spec: keep the DFlash drafter working after an image in the prompt - #344

Merged
TheTom merged 2 commits into
TheTom:feature/turboquant-kv-cachefrom
jasstrong:pr/dflash-image-positions
Sep 3, 2026
Merged

spec: keep the DFlash drafter working after an image in the prompt#344
TheTom merged 2 commits into
TheTom:feature/turboquant-kv-cachefrom
jasstrong:pr/dflash-image-positions

Conversation

@jasstrong

Copy link
Copy Markdown

Overview

With a DFlash drafter, any image in the prompt silently destroyed speculative decoding for the rest of the conversation: acceptance fell to ~0.03 and decode dropped below plain speed. With the default sliding-window draft cache it was worse, the server aborted on the image chunk (llama_decode(ctx_dft) failed, failed to process image).

Two faults, both only visible with M-RoPE targets and multimodal prompts:

  1. draft() placed the noise block at dp.n_past, the slot's token count. After an image that is no longer the next position, because image tokens advance the position by the grid size rather than by their count, so the block landed hundreds of positions past the draft's own cache and every draft was rejected. The block now starts at the target's llama_memory_seq_pos_max() + 1; without images the two values are equal, so nothing changes for text-only use.
  2. process() mirrored image chunks into the draft: vision-layer target states the draft was never trained on, all at one temporal position. The sliding-window cache could not even place them (find_slot failed). Embedding batches are now skipped; the draft keeps the target's positions across the gap, and the text after the image carries the image's influence in its injected features.

The TODO pointing at ggml-org#24669 stays: a proper batch API would let the draft know about the chunk, but nothing here depends on it.

Additional information

Qwen3.8-27B TQ3_1S + DFlash2 + mmproj-F16 on an MI210 (ROCm 7.2.3), a 20 KB text prefix plus a 1024×768 image, then a text-only turn in the same conversation:

image turn text turn after it
before acceptance 0.013, 26.8 t/s 0.026, 28.7 t/s
after 0.34, 66.0 t/s 0.43, 78.0 t/s

Measured with the default draft cache. Mirroring the image chunks with only the position fix applied still crashes the ring cache, and a full-size draft cache (--swa-full) is not needed once they are skipped; both variants were run.

The same code and the same TODO exist on ggml-org master, so this applies there as well.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES — Claude Code was used for the diagnosis (the bisection that separated positions from the chunk injection), the change, and the verification runs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NxP6x5bmUDYFvmouceN2mR

Two faults, both only visible once a prompt contains an image:

1. draft() placed the noise block at dp.n_past, the slot's token count.
   For an M-RoPE target that stops being the next position after an
   image: image tokens advance the position by the grid size, not by
   their count, so the block landed hundreds of positions past the
   draft's own cache and every draft was rejected (acceptance 0.03 for
   the rest of the conversation, decode below plain speed). Take the
   next position from the target's memory instead; without images the
   two are equal.

2. process() mirrored image chunks into the draft: vision-layer states
   the draft was never trained on, all at one temporal position. With
   the default sliding-window draft cache that could not even be placed
   (find_slot failed, then the server aborted). Skip embedding batches;
   the draft keeps the target's positions across the gap and the text
   after the image carries the image's influence in its injected
   features.

Qwen3.8-27B TQ3_1S + DFlash2 + mmproj on MI210, 20 KB text + a 1024x768
image, then a text turn: acceptance 0.013 / 0.026 -> 0.34 / 0.43, decode
27 / 29 -> 66 / 78 t/s, with the default draft cache. Mirroring the
chunks with the position fix still crashes; a full-size draft cache is
not needed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NxP6x5bmUDYFvmouceN2mR
(cherry picked from commit 9fa62c4d52407857429db56261d2874813e51e05)
@TheTom

TheTom commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Both fixes read correctly to me. llama_memory_seq_pos_max() + 1 is what the target actually needs for the next position, and it equals dp.n_past for text-only slots, so this is a no-op outside M-RoPE. Skipping embedding batches matches what the draft was trained on. No overlap with #340's speculative.cpp hunks (those are in the MTP struct).

One thing I could not confirm. The new comment in process() says the gap "is only legal with a full-size draft cache ... which the server enables when a projector is loaded." I do not find that anywhere: common_base_params_to_speculative does not touch swa_full, the only place it is set true is the --swa-full flag in arg.cpp, and upstream master is the same. Your description also says the numbers were taken with the default draft cache and that --swa-full is not needed once the chunks are skipped, which reads as the opposite. If the draft you tested is non-SWA that would explain the measurement; either way the sentence should say what actually happens, or point at the code that does it. Happy to merge as soon as that comment matches reality.

Nit: has_tokens == has_embeddings || has_embeddings is just !has_tokens || has_embeddings.

CI: the Windows x64-vulkan failure is test-thread-safety exiting 0xc0000374, the same runner flake #336 and #337 hit; openblas was a fail-fast cancel. Re-running both.

The comment claimed the gap left by skipped image chunks needs a
full-size draft cache that the server enables; nothing does that, and
the measurements were taken with the default sliding-window cache,
which is fine: every batch the draft sees has consecutive positions.
Say so, and write the condition as !has_tokens || has_embeddings.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NxP6x5bmUDYFvmouceN2mR
@jasstrong

Copy link
Copy Markdown
Author

You're right, that sentence described a server-side change I had tried and then reverted: forcing swa_full for the draft when a projector is configured. Once the image chunks are skipped it turned out not to be needed (every batch the draft still sees has consecutive positions, which is all find_slot asks; the crash was the image batch itself, whose tokens all carry one temporal position), so the forcing went, and the comment should have gone with it. 5d037a5 rewrites the comment to say what actually happens and takes the !has_tokens || has_embeddings nit. Measurements in the description stand: default draft cache, no server change.

@TheTom
TheTom merged commit 340b345 into TheTom:feature/turboquant-kv-cache Sep 3, 2026
9 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants