feat(dflash): config-driven drafter converter + spec-decode fixes + rope_parameters Q4 converter#447
Open
dusterbloom wants to merge 4 commits into
Open
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 20 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
dusterbloom
force-pushed
the
pr/dflash-converter-and-fixes
branch
2 times, most recently
from
549ae92 to
166a409
Compare
…ectness fixes DRAFTER CONVERTER (config-driven): - convert_dflash_to_gguf.py reads all architecture params from config.json (hidden_size, n_layer, mask_token_id, target_layer_ids, layer_types for SWA, sliding_window). No hardcoded constants. - quantize_draft_q8.py shares load_arch with the converter. - GGUF metadata: dflash.mask_token_id, dflash.target_layer_ids[], dflash.block_size, attention.sliding_window + pattern. - draft_gguf_loader.cpp: read_draft_capture_config(), mask from GGUF metadata, block_size override, SWA pattern from metadata. - draft_safetensors_loader.cpp: dynamic layer count, SWA+mask from config.json. - gguf_target_loader.cpp: respect drafter-specified capture layers instead of overwriting with evenly-spaced heuristic. - qwen35_backend.cpp: early-read capture sync + mask token propagation. - internal.h: capture_layer_ids[16], DFLASH_MAX_CAPTURE_LAYERS=16. - dflash27b.h: DFLASH_MAX_CAPTURE_LAYERS=16. SPEC-DECODE PERFORMANCE: - graph_builders.cpp: build_lm_head_projection_step skips rebuild when ctx alive + n_tokens matches (centralized guard; was per-call-site). - qwen35_backend.cpp: do_spec_decode uses member draft_sg_ (not local) for graph persistence; kFastRollbackThreshold env-tunable (DFLASH_FAST_ROLLBACK_MIN, default 5). - dflash_draft_graph.cpp: exact-ctx_len non-view reuse guard (DFLASH_DRAFT_GRAPH_REUSE, default ON). 4MB ctx alloc (was 256MB). - graph_builders.cpp: 4MB ctx alloc (was 64MB). - step_graph.h: graph_ctx_len + graph_used_view tracking fields. SPEC-DECODE CORRECTNESS: - qwen35_target_graph.cpp: DFLASH_FEAT_RING_CAP env overrides the hardcoded 4096 feature ring cap. Default 4096 causes acceptance collapse from 85% to 7.7% EXACTLY at 4096 prompt tokens (ring wrap corrupts features). - qwen35_backend.cpp: mirror init honors DFLASH_FEAT_RING_CAP. - qwen35_dflash_target.cpp: guard against invalid token IDs from GPU argmax at long context (NaN/Inf → clamp to 0, verify rejects gracefully). MOE EXPERIMENTAL (behind flags): - qwen35moe_backend.cpp: DFLASH_MOE_ALLHOT_HYBRID=1 builds moe_hybrid storage even with 0 cold experts to enable pipelined spec-decode verify. - Persistent moe_hybrid_logits_sg_ graph (was 64MB per-token alloc in hybrid_forward_one_token). GPU argmax (4 bytes vs 1MB vocab readback). - Batched verify/replay via hybrid_forward_batch (was 8 sequential forwards). VALIDATED: - 27B dense + reconverted drafter: 57% accept on code gen, 85% on short prompts. block=16 gives 252 tok/s (2.2x AR) on code generation. - 35B-A3B MoE + reconverted new drafter: 86% accept, 245 tok/s (2.1x AR). - Feature ring cap=16384: 85% holds to 5K tokens, 58% to 10K. - Full pFlash + dFlash stack: goldgate agentic trace passes (100% tool calls valid), pFlash cuts 34K prefill from 475s to 208s (2.3x). - repo_inspection prompt: correct answers, spec at 33.8% accept, 34 tok/s.
…al_dflash_to_gguf.py) resolve_rope_theta reads rope_parameters.rope_theta -> top-level rope_theta -> sys.exit(1); drops the silent 1M default that baked wrong RoPE for Qwen3.6 drafters whose true 10M lives nested under rope_parameters (missed by the old top-level-only pick(), default-baked 1M -> long-ctx spec-decode acceptance collapse). Q4_K_M output mirrors quantize_draft_q8.py (256 superblock, norm tensors stay F32). 12 pure-python tests cover the 35B-a3b nested case, 27B legacy top-level, gemma4 genuine-1M, and missing-theta fail-loud. Original convert_dflash_to_gguf.py untouched.
…OB, UAF, atoi-UB, re-prefill fallback, pin_range, gguf type guards)
dusterbloom
force-pushed
the
pr/dflash-converter-and-fixes
branch
from
166a409 to
02f2eb6
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/src/draft/draft_gguf_loader.cpp">
<violation number="1" location="server/src/draft/draft_gguf_loader.cpp:202">
P2: Malformed `dflash.target_layer_ids` can still be accepted as long as the count matches. Because the target graph only captures layers whose IDs equal a real `il`, negative/out-of-range IDs leave capture slots unwritten and duplicate IDs reuse the same layer, while spec decode proceeds with a full `n_capture_layers` feature vector. It would be safer to validate each ID (non-negative, within the loaded target layer count, and probably unique) before assigning it to `w_.capture_layer_ids`, otherwise fall back or fail early.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const int32_t * ids = | ||
| static_cast<const int32_t *>(gguf_get_arr_data(gctx, tli_id)); | ||
| for (size_t k = 0; k < n; k++) { | ||
| capture_ids[k] = (int)ids[k]; |
Contributor
There was a problem hiding this comment.
P2: Malformed dflash.target_layer_ids can still be accepted as long as the count matches. Because the target graph only captures layers whose IDs equal a real il, negative/out-of-range IDs leave capture slots unwritten and duplicate IDs reuse the same layer, while spec decode proceeds with a full n_capture_layers feature vector. It would be safer to validate each ID (non-negative, within the loaded target layer count, and probably unique) before assigning it to w_.capture_layer_ids, otherwise fall back or fail early.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/draft/draft_gguf_loader.cpp, line 202:
<comment>Malformed `dflash.target_layer_ids` can still be accepted as long as the count matches. Because the target graph only captures layers whose IDs equal a real `il`, negative/out-of-range IDs leave capture slots unwritten and duplicate IDs reuse the same layer, while spec decode proceeds with a full `n_capture_layers` feature vector. It would be safer to validate each ID (non-negative, within the loaded target layer count, and probably unique) before assigning it to `w_.capture_layer_ids`, otherwise fall back or fail early.</comment>
<file context>
@@ -182,24 +182,51 @@ bool read_draft_capture_config(const std::string & path,
+ const int32_t * ids =
+ static_cast<const int32_t *>(gguf_get_arr_data(gctx, tli_id));
+ for (size_t k = 0; k < n; k++) {
+ capture_ids[k] = (int)ids[k];
+ }
+ ids_count = (int)n;
</file context>
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
1 participant
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.
Wave-2 base. The config-driven drafter converter + spec-decode perf/correctness fixes, plus a new corrected converter.
New in this PR (commit 166a409): server/scripts/convert_modal_dflash_to_gguf.py — resolves rope_theta from the nested rope_parameters scope then top-level (legacy), and FAILS LOUD (sys.exit) on a missing theta instead of silently baking a 1M default. This fixes the root cause of the drafter RoPE mismatch: Qwen3.6 drafters publish their true 10M under rope_parameters, which the old top-level-only pick() missed, baking 1M and collapsing long-ctx spec-decode acceptance (gemma4 is genuinely 1M so the bug was invisible there). Q4_K_M output, 12 pure-python tests. Original convert_dflash_to_gguf.py untouched.
Depends on the pooled-prefill core (#430).