Skip to content

feat(gemma4): add dflash draft runtime#181

Closed
dusterbloom wants to merge 11 commits into
Luce-Org:mainfrom
dusterbloom:split/gemma4-09-dflash-draft-runtime
Closed

feat(gemma4): add dflash draft runtime#181
dusterbloom wants to merge 11 commits into
Luce-Org:mainfrom
dusterbloom:split/gemma4-09-dflash-draft-runtime

Conversation

@dusterbloom

Copy link
Copy Markdown
Collaborator

PR #9 of the Gemma4 split sequence. Adds Gemma4 DFlash draft graph execution, draft KV cache lifecycle, and a smoke test that exercises the full draft forward path (loader → KV cache alloc → prefill → forward). No MTP, no γ>1 speculative decode, no server routing — those land in PR #10/#11/#13.

Depends on #180#179#177#176#171#170#169#168. Stacks on `split/gemma4-08-draft-loader-quant`; diff includes ancestor commits until they merge.

Scope (5 files, +761 / -8)

  • `dflash/src/gemma4_dflash_graph.cpp` — restores the graph-builder block stripped in feat(gemma4): add draft loader and quantization support #180: two helpers (`draft_swa_trunc_enabled`, `draft_rope` with optional YaRN), `build_draft_kv_prefill_graph` (prefix-direct KV materialisation, SGLang style), `build_gemma4_draft_graph` (KV-cached forward returning softcapped logits).
  • `dflash/src/gemma4_target_graph.cpp` — adds `create_draft_kv_cache` + `free_draft_kv_cache` (these live on `GemmaTargetCache` because the draft KV is per-target-cache). Wires `free_draft_kv_cache(c)` into `free_gemma4_cache` and `c.draft_kv_pos = 0` into `reset_gemma4_cache`.
  • `dflash/src/internal.h` — adds the `draft_kv_*` fields to `GemmaTargetCache` (ctx, buf, K/V vectors, cap, pos) and prototypes for the four new functions.
  • `dflash/test/gemma4/smoke_gemma4_draft_forward.cpp` — end-to-end smoke: loads target + draft, allocates draft KV, runs `build_draft_kv_prefill_graph`, runs `build_gemma4_draft_graph`, asserts non-null logits.
  • `dflash/CMakeLists.txt` — wires the new smoke target.

Risk: HIGH

First runtime path that actually executes the DFlash draft. Risk lives in:

  • KV cache bounds: `create_draft_kv_cache` reserves `sliding_window + block_size + 32` slots; `build_draft_kv_prefill_graph` aborts on overflow.
  • Draft RoPE: optional YaRN scaling via env (`DFLASH_DRAFT_YARN`), default off.
  • SWA truncation: optional via env (`DFLASH_DRAFT_SWA_TRUNC`), default off — preserves the pre-PR#129 behavior unless opted in.

Deferred — `test_gemma4_dflash.cpp` integration hunks

The full test driver in `feature/gemma4-support` is 2962 lines with 71 MTP references intermixed. The `--draft-method dflash` flag and dflash-specific code paths cannot be cleanly carved out without (a) duplicating large chunks of the driver scaffolding into a PR-specific copy, or (b) pulling in MTP symbols this PR explicitly excludes. The full driver lands with PR #11 (MTP runtime), which already needs both `--draft-method dflash` and `--draft-method mtp` paths together.

For PR #9, the dedicated `smoke_gemma4_draft_forward` covers the same code paths end-to-end (loader → KV cache → prefill graph → forward graph → logits). The test driver integration is documented to maintainers and tracked.

Forbidden-symbol audit (vs spec exclusion list)

```
grep -cw 'mtp|MTP' → 0 (all 4 files)
grep -nE 'h_prev|MtpDrafter|MtpLayerWeights|build_mtp_step_graph' → no matches
```

Review checklist (from spec)

  • DFlash draft runtime works without MTP code ✓ (compiles independently — verified via syntax-only build of both .cpp files)
  • Draft KV cache wrap behavior covered or deferred — deferred: prefix-direct prefill aborts on overflow rather than wrapping; the wrap-or-overflow policy is a follow-up decision once decode-loop logic lands. Smoke exercises happy-path only.
  • Test-driver flags introduced here are only DFlash — deferred to PR fix(scripts): auto-fit --max-ctx to prompt size #11 per above.
  • No server behavior is changed ✓.

Validation

  • `cmake` reconfigure picks up the new smoke target ✓
  • `g++ -fsyntax-only gemma4_dflash_graph.cpp` via project's compile_commands flags → exit 0
  • `g++ -fsyntax-only gemma4_target_graph.cpp` → exit 0
  • Full `cmake --build smoke_gemma4_draft_forward` deferred to CI (no local CUDA build budget this turn)
  • Runtime invocation requires both a target GGUF and a draft directory on disk.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

4 issues found across 20 files

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=".gitmodules">

<violation number="1" location=".gitmodules:3">
P2: Submodule switched from org-maintained repo to personal fork + feature branch without PR documentation, contradicting README</violation>
</file>

<file name="dflash/CMakeLists.txt">

<violation number="1" location="dflash/CMakeLists.txt:547">
P2: Four newly added Gemma4 test targets are missing from the CUDA runtime backfill list and will fail to build in CUDA configurations</violation>
</file>

<file name="dflash/src/gemma4_dflash_graph.cpp">

<violation number="1" location="dflash/src/gemma4_dflash_graph.cpp:231">
P2: Missing draft KV cache vector validation before indexing by layer</violation>

<violation number="2" location="dflash/src/gemma4_dflash_graph.cpp:662">
P2: Multi-shard safetensors discovery is broken: the fallback hardcodes `model-00001-of-00001.safetensors` instead of matching any `model-00001-of-NNNNN.safetensors` as the comment claims. A multi-shard draft (e.g. `model-00001-of-00002.safetensors`) will fail to load.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread .gitmodules Outdated
path = dflash/deps/llama.cpp
url = https://github.com/Luce-Org/llama.cpp-dflash-ggml.git
branch = luce-dflash
url = https://github.com/dusterbloom/llama-cpp-turboquant-cuda.git

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Submodule switched from org-maintained repo to personal fork + feature branch without PR documentation, contradicting README

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .gitmodules, line 3:

<comment>Submodule switched from org-maintained repo to personal fork + feature branch without PR documentation, contradicting README</comment>

<file context>
@@ -1,7 +1,7 @@
 	path = dflash/deps/llama.cpp
-	url = https://github.com/Luce-Org/llama.cpp-dflash-ggml.git
-	branch = luce-dflash
+	url = https://github.com/dusterbloom/llama-cpp-turboquant-cuda.git
+	branch = feature/tq3-kv-cache-clean
 [submodule "dflash/deps/Block-Sparse-Attention"]
</file context>

Comment thread dflash/CMakeLists.txt
Comment thread dflash/src/gemma4_dflash_graph.cpp
int fd_check = ::open(path.c_str(), O_RDONLY);
if (fd_check < 0) {
// Fall back to first numbered shard
path = dir_path + "/model-00001-of-00001.safetensors";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Multi-shard safetensors discovery is broken: the fallback hardcodes model-00001-of-00001.safetensors instead of matching any model-00001-of-NNNNN.safetensors as the comment claims. A multi-shard draft (e.g. model-00001-of-00002.safetensors) will fail to load.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dflash/src/gemma4_dflash_graph.cpp, line 662:

<comment>Multi-shard safetensors discovery is broken: the fallback hardcodes `model-00001-of-00001.safetensors` instead of matching any `model-00001-of-NNNNN.safetensors` as the comment claims. A multi-shard draft (e.g. `model-00001-of-00002.safetensors`) will fail to load.</comment>

<file context>
@@ -0,0 +1,1112 @@
+        int fd_check = ::open(path.c_str(), O_RDONLY);
+        if (fd_check < 0) {
+            // Fall back to first numbered shard
+            path = dir_path + "/model-00001-of-00001.safetensors";
+            fd_check = ::open(path.c_str(), O_RDONLY);
+            if (fd_check < 0) {
</file context>

@dusterbloom dusterbloom force-pushed the split/gemma4-09-dflash-draft-runtime branch from 7d85138 to 3df8632 Compare May 13, 2026 14:54
@dusterbloom dusterbloom force-pushed the split/gemma4-09-dflash-draft-runtime branch 2 times, most recently from 96097ad to f643866 Compare May 13, 2026 15:58
dusterbloom and others added 6 commits May 13, 2026 21:14
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
The test calls pflash_register_ggml_kernel(), which is defined in
src/pflash_ggml_adapter.cpp. That source is only compiled into
dflash27b on CUDA + sm>=80 (the elseif branch at line ~291). The test
target was unguarded, so CI runners building at sm<80 successfully
built dflash27b without the adapter, then failed to link the test:

    undefined reference to `pflash_register_ggml_kernel()'

Mirror the same guard used by test_flashprefill_kernels at line ~396.
Plain-C++ compile units that include internal.h (e.g. smoke tests built
with g++ rather than nvcc) hit a fatal error on the unconditional
#include <cuda_runtime.h> guarded by GGML_USE_CUDA. The runtime header
is only needed by src/cuda_cross_device_copy.cpp (the implementation
TU), which already includes it directly.

Replace the header include with a forward declaration of cudaStream_t
(typedef struct CUstream_st*) so consumers of the dflash_cuda_copy_
between_devices prototype don't need CUDA include paths.

Found via CI failure on the smoke_load_gemma4_target /
smoke_gemma4_target_forward targets after CI started reaching them
following the test_flash_attn_sparse cmake guard fix.
@dusterbloom dusterbloom force-pushed the split/gemma4-09-dflash-draft-runtime branch from f643866 to fe9354f Compare May 13, 2026 19:16
The smoke driver was leaving GemmaGraphInputs::swa_mask null, causing
gemma4_target_graph SWA layers to fall back to attn_mask via the
effective_mask = swa_mask ?: attn_mask path. attn_mask is sized for the
full-attn view (kv_len_padded), but SWA layers view the full swa_ctx_alloc
slots, so flash attention reads past the end of attn_mask into adjacent
GPU memory. Manifests as all-NaN logits with Q4_0 / TQ3_0 KV (the OOB
bytes are interpreted as fp16 mask values added to attention scores).

Q8_0 tolerated the OOB read by accident; Q4_0 / TQ3_0 do not. The bug is
documented in gemma4_runtime_helpers.cpp:124-130 — the runtime helper used
by the daemon path sets swa_mask correctly. The smoke driver bypassed that
helper.

Allocate swa_mask sized [align_up(swa_ctx_alloc, 256), n_tokens] and fill
the same causal pattern as attn_mask. Caught by running the test on a
real GPU (CI only compiles, doesn't run).

No production code changed.
@dusterbloom dusterbloom force-pushed the split/gemma4-09-dflash-draft-runtime branch from fe9354f to 6756002 Compare May 13, 2026 20:43
easel pushed a commit to easel/lucebox-hub that referenced this pull request May 28, 2026
Record fresh worktree probes for PRs Luce-Org#182, Luce-Org#181, Luce-Org#180, Luce-Org#154, and Luce-Org#131, including Codex feasibility reports for the selective-port candidates and retained audit worktree paths.
easel pushed a commit to easel/lucebox-hub that referenced this pull request May 28, 2026
Record the 2026-05-28 06:55 cron run, current open PR classification, direct merge probes, and PR Luce-Org#181 Codex feasibility results.
@dusterbloom

Copy link
Copy Markdown
Collaborator Author

Closing: targets obsolete dflash/src/gemma4_*.cpp paths. Gemma4 moved to vendored deps/llama.cpp/src/models/gemma4-iswa.cpp; spec-decode integration is now via the common/ interface layer.

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.

1 participant