feat(gemma4): add draft loader and quantization support - #180
feat(gemma4): add draft loader and quantization support#180dusterbloom wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
8 issues found across 19 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 `dflash/deps/llama.cpp` redirected to personal fork feature branch (`dusterbloom/llama-cpp-turboquant-cuda.git` branch `feature/tq3-kv-cache-clean`) without justification, introducing dependency governance and build-reproducibility risk.</violation>
</file>
<file name="dflash/test/test_flash_attn_sparse.cpp">
<violation number="1" location="dflash/test/test_flash_attn_sparse.cpp:114">
P2: The dense-vs-sparse equivalence check uses an overly loose 1.0 absolute-error threshold, so this smoke test can miss real regressions.</violation>
</file>
<file name="dflash/src/gemma4_target_loader.cpp">
<violation number="1" location="dflash/src/gemma4_target_loader.cpp:352">
P2: Failure paths after `out.ctx = meta_ctx` assignment leak `meta_ctx` and leave `out` partially mutated because they only call `gguf_free(gctx)` without freeing `out.ctx` or resetting `out`. The `cleanup_out()` helper exists but is declared later and unused for these paths.</violation>
</file>
<file name="dflash/CMakeLists.txt">
<violation number="1" location="dflash/CMakeLists.txt:450">
P1: Gemma4 test targets hardcode `ggml-cuda` instead of `${DFLASH27B_GGML_BACKEND_TARGET}`, breaking HIP builds</violation>
<violation number="2" location="dflash/CMakeLists.txt:542">
P2: New Gemma4 CUDA tests are missing from the `CUDA::cudart` propagation list</violation>
</file>
<file name="dflash/scripts/quantize_draft_q8.py">
<violation number="1" location="dflash/scripts/quantize_draft_q8.py:169">
P1: Nullable Gemma4 config values are not sanitized before numeric conversion, causing quantization to crash on explicit null values in config.json.</violation>
</file>
<file name="dflash/src/gemma4_target_graph.cpp">
<violation number="1" location="dflash/src/gemma4_target_graph.cpp:899">
P1: PLE extraction uses incorrect offset/shape assumptions for 2D per-layer input tensors.</violation>
</file>
<file name="dflash/src/gemma4_dflash_graph.cpp">
<violation number="1" location="dflash/src/gemma4_dflash_graph.cpp:334">
P2: Safetensors shard discovery hardcodes `model-00001-of-00001` instead of matching the claimed `model-00001-of-NNNNN` pattern, so valid multi-shard files are not found.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/test/gemma4/smoke_load_gemma4_target.cpp") | ||
| add_executable(smoke_load_gemma4_target test/gemma4/smoke_load_gemma4_target.cpp) | ||
| target_include_directories(smoke_load_gemma4_target PRIVATE ${DFLASH27B_SRC_INCLUDE_DIRS}) | ||
| target_link_libraries(smoke_load_gemma4_target PRIVATE dflash27b ggml ggml-cuda) |
There was a problem hiding this comment.
P1: Gemma4 test targets hardcode ggml-cuda instead of ${DFLASH27B_GGML_BACKEND_TARGET}, breaking HIP builds
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dflash/CMakeLists.txt, line 450:
<comment>Gemma4 test targets hardcode `ggml-cuda` instead of `${DFLASH27B_GGML_BACKEND_TARGET}`, breaking HIP builds</comment>
<file context>
@@ -436,6 +444,26 @@ if(DFLASH27B_TESTS)
+ if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/test/gemma4/smoke_load_gemma4_target.cpp")
+ add_executable(smoke_load_gemma4_target test/gemma4/smoke_load_gemma4_target.cpp)
+ target_include_directories(smoke_load_gemma4_target PRIVATE ${DFLASH27B_SRC_INCLUDE_DIRS})
+ target_link_libraries(smoke_load_gemma4_target PRIVATE dflash27b ggml ggml-cuda)
+ endif()
+ if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/test/gemma4/smoke_gemma4_target_forward.cpp")
</file context>
| if arch == "gemma4": | ||
| target_layer_ids = dflash_cfg.get("target_layer_ids", defaults["TARGET_LAYER_IDS"]) | ||
| cfg.update(dict( | ||
| LOGIT_SOFTCAP = float(raw.get("final_logit_softcapping", defaults["LOGIT_SOFTCAP"])), |
There was a problem hiding this comment.
P1: Nullable Gemma4 config values are not sanitized before numeric conversion, causing quantization to crash on explicit null values in config.json.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dflash/scripts/quantize_draft_q8.py, line 169:
<comment>Nullable Gemma4 config values are not sanitized before numeric conversion, causing quantization to crash on explicit null values in config.json.</comment>
<file context>
@@ -10,45 +14,176 @@
+ if arch == "gemma4":
+ target_layer_ids = dflash_cfg.get("target_layer_ids", defaults["TARGET_LAYER_IDS"])
+ cfg.update(dict(
+ LOGIT_SOFTCAP = float(raw.get("final_logit_softcapping", defaults["LOGIT_SOFTCAP"])),
+ SLIDING_WINDOW = raw.get("sliding_window", defaults["SLIDING_WINDOW"]),
+ TARGET_LAYER_IDS = target_layer_ids,
</file context>
| const int n_embd_per_layer = w.n_embd_per_layer > 0 ? w.n_embd_per_layer | ||
| : (int)in.per_layer_inp->ne[0]; | ||
| ggml_tensor * ple_emb; | ||
| if (ggml_n_dims(in.per_layer_inp) >= 3 || (int)in.per_layer_inp->ne[1] == w.n_layer) { |
There was a problem hiding this comment.
P1: PLE extraction uses incorrect offset/shape assumptions for 2D per-layer input tensors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dflash/src/gemma4_target_graph.cpp, line 899:
<comment>PLE extraction uses incorrect offset/shape assumptions for 2D per-layer input tensors.</comment>
<file context>
@@ -0,0 +1,996 @@
+ const int n_embd_per_layer = w.n_embd_per_layer > 0 ? w.n_embd_per_layer
+ : (int)in.per_layer_inp->ne[0];
+ ggml_tensor * ple_emb;
+ if (ggml_n_dims(in.per_layer_inp) >= 3 || (int)in.per_layer_inp->ne[1] == w.n_layer) {
+ // Shape [n_embd_per_layer, n_layer] or [n_embd_per_layer, n_tokens, n_layer]
+ ple_emb = ggml_view_2d(ctx, in.per_layer_inp,
</file context>
| 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 |
There was a problem hiding this comment.
P2: Submodule dflash/deps/llama.cpp redirected to personal fork feature branch (dusterbloom/llama-cpp-turboquant-cuda.git branch feature/tq3-kv-cache-clean) without justification, introducing dependency governance and build-reproducibility risk.
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 `dflash/deps/llama.cpp` redirected to personal fork feature branch (`dusterbloom/llama-cpp-turboquant-cuda.git` branch `feature/tq3-kv-cache-clean`) without justification, introducing dependency governance and build-reproducibility risk.</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>
| smoke_draft_graph | ||
| test_vs_oracle | ||
| smoke_load_target | ||
| smoke_load_gemma4_target |
There was a problem hiding this comment.
P2: New Gemma4 CUDA tests are missing from the CUDA::cudart propagation list
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At dflash/CMakeLists.txt, line 542:
<comment>New Gemma4 CUDA tests are missing from the `CUDA::cudart` propagation list</comment>
<file context>
@@ -511,6 +539,7 @@ if(DFLASH27B_TESTS)
smoke_draft_graph
test_vs_oracle
smoke_load_target
+ smoke_load_gemma4_target
smoke_load_target_laguna
smoke_laguna_forward
</file context>
| 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"; |
There was a problem hiding this comment.
P2: Safetensors shard discovery hardcodes model-00001-of-00001 instead of matching the claimed model-00001-of-NNNNN pattern, so valid multi-shard files are not found.
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 334:
<comment>Safetensors shard discovery hardcodes `model-00001-of-00001` instead of matching the claimed `model-00001-of-NNNNN` pattern, so valid multi-shard files are not found.</comment>
<file context>
@@ -0,0 +1,784 @@
+ 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>
491d3b9 to
5f8cc92
Compare
14bfceb to
5ddd3a9
Compare
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.
5ddd3a9 to
686581a
Compare
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.
686581a to
44bb2ce
Compare
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.
Record the 2026-05-28 07:19 unattended run, including fresh direct probes and PR Luce-Org#180 Codex feasibility triage. No code changes were made.
|
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. |
PR #8 of the Gemma4 split sequence. Adds Gemma4 DFlash draft weight loading (safetensors + GGUF) and the Q8_0 quantization tool that produces the draft GGUF. No graph execution, no draft KV cache, no speculative decode runtime — those land in PR #9.
Depends on #179 → #177 → #176 → #171 → #170 → #169 → #168. Stacks on `split/gemma4-07-pflash-integration`; diff includes ancestor commits until they merge.
Scope (5 files)
Removed: `build_draft_kv_prefill_graph`, `build_gemma4_draft_graph`, `draft_rope`, `draft_swa_trunc_enabled` (all graph-execution code, deferred to PR docs: fix readme consistency #9).
Risk: Medium
Loader code is pure mmap/parse, no GPU execution. Risk is metadata / tensor-name mismatches against future Gemma4 draft GGUFs, which the smoke test catches end-to-end.
Review checklist (from spec)
Validation