Skip to content

feat(qwen35): derive scalars from weights, assert vs GGUF metadata - #359

Merged
davide221 merged 1 commit into
Luce-Org:mainfrom
dusterbloom:split/04-gguf-scalar-assert
Jun 10, 2026
Merged

feat(qwen35): derive scalars from weights, assert vs GGUF metadata#359
davide221 merged 1 commit into
Luce-Org:mainfrom
dusterbloom:split/04-gguf-scalar-assert

Conversation

@dusterbloom

Copy link
Copy Markdown
Collaborator

Re-carved from #274 (commit 5819648), DRY'd into a shared helper + the unit test the original lacked.

After loading weights, the qwen35 target loader and the dflash draft loader derive head_dim/n_head/n_head_kv from the actual weight-tensor shapes and assert against the GGUF-declared hparams; on mismatch → set_last_error + return false at load time, making the "stale scalar at graph-build time" bug class structurally impossible. Load-time only, no runtime cost; well-formed GGUFs pass through unchanged.

DRY: pure verify_derived_scalars() in server/src/common/derived_scalars.h, unit-tested (13 cases). The qwen35 target Q-projection packs Q‖gate (ne[1] = n_head·n_embd_head_k·2, per the loader's own contract); the draft loader uses the standard n_head·head_dim. gemma4 has an equivalent inline check on a different (unpark) path with different semantics — left as-is, noted in the header.

Validation: helper unit-tested (13 cases); both modified loader TUs compile clean against the new call sites. Real-GGUF end-to-end load not yet exercised — this is a defensive load-time check that only fires on a genuine weight-shape↔metadata mismatch.

5 files, +266.

Load-time guard: after loading wq/wk, derive head_dim/n_head/n_head_kv from
tensor shapes and assert against GGUF-declared values; set_last_error+return
false on mismatch. Makes the stale-scalar-at-graph-build bug class impossible.

DRY: extracted verify_derived_scalars() pure helper into
server/src/common/derived_scalars.h (no IO, header-only); wired at both new
sites (draft loader layer 0, qwen35 target first full-attn layer). gemma4
inline block is a silent override not an assert; left as-is with comment.

Unit test: server/test/test_derived_scalars.cpp — 13 assertions, 0 failures.

@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.

Review completed

Re-trigger cubic

@davide221
davide221 merged commit 3e12dc9 into Luce-Org:main Jun 10, 2026
7 checks passed
davide221 added a commit that referenced this pull request Jun 12, 2026
…gression fix

Spec decode now runs on the pool everywhere it exists. gemma4 was the
last gap:

- gemma4_verify_batch gains the kvflash path: set_rows kv-index inputs
  (full layers -> pool slots, SWA -> ring rows), slot-space causal mask
  via the shared helper, FA span + mask width clamped to the pool.
  Gemma4DFlashTarget allocates the verify block's slots up front; the
  spec loop's KV-truncation rejection maps directly onto the pool's
  validity rule (rejected slots hold future positions, masked until the
  next verify rewrites them). Both backend spec gates removed.
- Pre-existing regression fixed (blocks gemma spec on MAIN, not just
  here): PR #359's strict assert reads dflash.n_target_layers, which
  the published gemma draft fills with the TARGET layer count (30)
  while its fc tensor is sized for the 6 CAPTURE layers — the draft
  refused to load at all. Per that PR's own weights-are-ground-truth
  rule, derive the capture count from fc when it divides n_embd and
  warn on the metadata mismatch; genuinely inconsistent shapes still
  fail.
- gemma4 accept_rate now reaches the HTTP usage block (was silently
  0.0 while the loop logged the real rate — same reporting-only class
  as the PR #321 layer-split gap).

A/B on the 3090 (26B-A4B + published q8_0 draft, 600 tokens): pooled
and full cache produce IDENTICAL acceptance (407/3104 = 13.1%,
avg_commit 3.09) and identical text; usage reports 0.131 on both.

Co-Authored-By: WOZCODE <contact@withwoz.com>
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