Skip to content

npu: the per-op forward reads gemma3 text decoders; Medgemma-4B answers on the host (Refs #1786) - #2242

Closed
bong-water-water-bong wants to merge 3 commits into
mainfrom
cl-flm/gemma3-text
Closed

bong-water-water-bong wants to merge 3 commits into
mainfrom
cl-flm/gemma3-text

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Refs #1786.

This body describes head de3a5d4: the first commit (3a064df, the forward's gemma3 rules) and a second one. The second makes gemma3 prefill batched and has the serve add the BOS that tokenizer_config.json states. It is backed by a device run.

FastFlowLM's Medgemma-4B-NPU2 (gemma3_text, 34 layers) now answers correctly through the per-op forward on the host (NPU_INFER_HOST_GEMM=1, no device). On main it is refused at load. Gemma3 is the family behind seven FastFlowLM packs (Gemma3-270M/1B/4B, both Medgemmas, Translategemma), and it was listed as not served.

Before and after, on the real pack

A scratch probe calls init_host() and step() and decodes greedily. The prompt is the pack's own chat template, tokenized with the pack's tokenizer.json.

main (and this branch's parent):
  init failed: config.json declares rope scaling "linear", which this forward does not implement ...

first commit (3a064df), "What is the capital of France? Answer with one word."
  prefill 21 tokens 164.0s
  out: 50429 107 106                         -> 'Paris\n<end_of_turn>'

"In two sentences, what does the human heart do?"
  -> 'The human heart is a muscular organ that pumps blood throughout the body. This circulation
      delivers oxygen and nutrients to tissues and removes waste products, sustaining life.\n<end_of_turn>'

  [semantics] layer mix: full=5 sliding=29 linear=0 other=0
  [semantics] per-type rope: full(theta=1e+06 rot=0) sliding(theta=10000 rot=0, rope_local_base_freq)  ...  query_pre_attn_scalar=256
  [semantics] ... extra_layer_norms=2
  [semantics] rmsnorm_offset=0  <- model_type/architectures (gemma2 uses the 1+w form; gemma3 containers carry the 1)

What this PR establishes, and what it does not

  • One pack measured: Medgemma-4B-NPU2. Of the seven gemma3 packs, Gemma3-270M-NPU2 is on the box and still fails at load (the K = 640 tile grid, below). The other five (Gemma3-1B, Gemma3-4B, Gemma3-4B-Text, Medgemma-1.5-4B, Translategemma-4B) are not on the box and were not run. The rules are the family's, read from config keys those packs share, but only Medgemma-4B has a transcript.
  • The host numbers are a correctness path, not a serving row. 164.0 s of prefill for 21 tokens is the host per-op reference (NPU_INFER_HOST_GEMM=1). The NPU route for this family is npu: every FastFlowLM/*-NPU2 model source imports and serves on --device npu (FLM 1.0.12 model sources, 59 repos) #1786's. The device tickets below are correctness evidence too; decode at 0.68 tok/s is not a result to quote.
  • The served text does not carry the stop token. The host probe prints raw ids, so <end_of_turn> (106) appears in its transcript. The serve stops on 106 and does not emit it: the load line reads stops on 1 <eos>, 106 <end_of_turn> (tokenizer / chat turn end), and the probe of the served reply in ticket 3 reads content='Paris\n' … finish=stop three times.

Second commit: batched prefill, and the BOS

Batched prefill. step_batch used to send every gemma-class layer to one step() per token. Gemma3, which is Gemma4's sandwich without per-layer embeddings or KV sharing, now runs in the batched body with the post-attention, pre-feedforward and post-feedforward norms. Gemma4 still falls back.

NPU_BATCH_DBG=1 ONEBIT_TEST_Q4NX=…/Medgemma-4B-NPU2 batch_check_state_test     (host GEMM)
[batchcheck] M=3 positions=0..2 seq_argmax=818 batch_argmax=818 MATCH
[batchcheck] M=2 positions=3..4 seq_argmax=107 batch_argmax=107 MATCH
[batchcheck] M=1 positions=5..5 seq_argmax=237446 batch_argmax=237446 MATCH
PASS  last chunk's logits vs the sequential reference: rel-L1 0.00e+00
ALL PASS (0 failed)                                   (no "[batch] ... fallback" line: the batched body ran)

The BOS. forward-serve read bos_token_id and add_bos_token from config.json only. Medgemma's config.json states neither, while its tokenizer_config.json has "bos_token": "<bos>" and "add_bos_token": true, so the served prompt had no BOS. What config.json does not state now comes from tokenizer_config.json (special_token_text, add_bos_token_flag in app/npu_stop_tokens.h, tested in npu_stop_tokens_test). It was found by comparing the device reply with the host forward on the same prompt:

host forward, with BOS (19 tokens):  'The sky appears blue due to a phenomenon called Rayleigh scattering. Sunlight, which is'
host forward, BOS dropped (18):      'The sky is blue because of the way the atmosphere scatters'
served before the fix (prompt=18):   'The sky is blue because of the way the light is. The light i'
served after the fix (prompt=19):    'The sky appears blue due to a phenomenon called Rayleigh sca'

On the device: three npu-hold tickets, Medgemma-4B-NPU2, a locally built kernel set for the four shapes

Each server ran inside its ticket and ended with it.

ticket 1 (first commit, M=128 kernels only)       ticket 552 s
  known answer rep 0/1: 'Paris'   prompt=20 (no BOS)   prompt_ms 151915 / 151719   (per-token prefill)
ticket 2 (+ batched prefill, + 16-row twins)       ticket 303 s
  known answer: 3 answers, identical=True, Paris       prompt=20   prompt_ms 11310 / 5567 / 5515
  decode: 'The sky is blue because of the way the light is…'   0.51 / 0.64 tok/s
ticket 3 (this head)                               ticket 230 s
  1bit forward-serve: BOS 2 from tokenizer_config.json (config.json does not state it)
  known answer: 3 answers, identical=True, Paris       prompt=21   prompt_ms 9433 / 4234 / 4180
  decode rep 0/1: ttft 4.250 / 4.368 s, 0.69 / 0.68 tok/s, 64 tokens, same text sha 847c3ea98a,
                  'The sky appears blue due to a phenomenon called Rayleigh sca…'

Batching took the 20-token prefill from 152 s to 5.5 s. Decode is slow, and I have not profiled it yet.

What was missing (first commit) (all read from the config, or decided by the family)

  1. rope_scaling linear (factor 8): the frequency is divided by the factor. Gemma3 applies it to the full layers only, since its sliding layers rotate unscaled with their own base. A model with no local base scales every layer, as HF does. A linear block with no usable factor is still refused.
  2. sliding_window_pattern when there is no layer_types list: every 6th layer is full (HF's (i + 1) % 6). Before, all 34 layers ran as full attention with no window.
  3. rope_local_base_freq: the sliding layers' base (10000). A sliding layer with no base of its own now takes the model's base. Before, it got 0, and that already hit Gemma3-270M's stated layer_types.
  4. query_pre_attn_scalar: the attention scale. It equals head_dim on Medgemma, but not on every Gemma3.
  5. The sandwich norms: Gemma4's layer body, without the post-PLE norm and the layer scalar (extra_layer_norms = 2).

The one non-obvious rule: the norm form

HF's Gemma3 RMSNorm is x/rms * (1 + w). These containers store w with the 1 already added. I compared the pack against llama.cpp's GGUF of the same model (~/models/medgemma/medgemma-4b-it-Q4_K_M.gguf), whose converter adds the 1 (conversion/gemma.py, norm_shift):

layer 0 / final norm       max |gguf - pack|   max |gguf - 1 - pack|
input_layernorm             0.125               1.125
post_attention_layernorm    0.031               1.031
pre_feedforward_layernorm   0.000               1.000
post_feedforward_layernorm  1.000 (one bf16 outlier at 304)   1.125
q_norm                      0.008               1.008
norm                        0.125               1.125

The known-answer prompt with (1 + w) gave *,,,,,,, (tokens 30237 236743 ×7). With the plain form it gives Paris. So gemma3 takes the plain form. Gemma2 keeps (1 + w), which I did not measure, and gemma3n is excluded by name. The weight probe that settles qwen3_5's form cannot decide here: Medgemma's layer-0 input norm is positive and has a mean of 8.09 in either form.

Not in this PR

Checks

  • tests/q4nx_gemma3_semantics_test.c (new, CI-reachable through add_dependencies(npu_sampling_test …), the way q4nx_qk_norm_test is): 14 checks using Medgemma's own keys, a stated layer_types list, and the gemma2, gemma3n and qwen3 neighbours.
  • ctest -j 6 --timeout 200 -E "npu_rt_shim|npu_dmabuf_import" at de3a5d4: 262 of 262 pass. The two excluded tests do not link in my build directory, and npu_dmabuf_import would submit to the device.
  • tools/copyright.py --check rc 0. git diff --stat origin/main: 10 files, +336/−27.

🤖 Generated with Claude Code

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at 54845a7

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit aabf72a)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1786 - Partially compliant

Compliant requirements:

  • Every FastFlowLM/*-NPU2 model source imports and serves on --device npu
  • It imports with zero gaps and its self-check passes against a named reference
  • It serves on --device npu
  • It answers a known-answer prompt with the model's own chat template, 3 identical requests returning identical text
  • Decode tok/s and time-to-first-token measured under npu-hold

Non-compliant requirements:

  • The row names the kernel set and the engine commit that produced it

Requires further human verification:

  • The kernel set and engine commit that produced the row needs to be verified in the documentation
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Rope Scaling Implementation

The implementation of rope scaling for "linear" type in read_rope_linear function may not correctly handle all edge cases in parsing the config file, particularly when the factor is not present or invalid. This could lead to incorrect rope scaling being applied, affecting model accuracy.

static double read_rope_linear(const std::string& cfg_path) {
    std::ifstream f(cfg_path);
    if (!f) return 0.0;
    std::string t((std::istreambuf_iterator<char>(f)), std::istreambuf_iterator<char>());
    char type[64];
    q4nx_rope_declared_type(t.c_str(), type, (int)sizeof type);
    if (std::strcmp(type, "linear")) return 0.0;
    const size_t rs = t.find("\"rope_scaling\"");
    const size_t at = rs == std::string::npos ? rs : t.find("\"factor\"", rs);
    // the key must sit inside the block: before the first closing brace after it (the block holds no objects)
    if (at == std::string::npos || at > t.find('}', rs)) return 0.0;
    const size_t colon = t.find(':', at);
    if (colon == std::string::npos) return 0.0;
    char* end = nullptr;
    const double v = std::strtod(t.c_str() + colon + 1, &end);
    return (end != t.c_str() + colon + 1 && v > 0.0) ? v : 0.0;
}
Layer Type Handling

The logic for handling layer types in Gemma3 models, particularly when sliding_window_pattern is used, may not correctly handle all combinations of layer types and sliding window configurations. This could lead to incorrect layer mixing and attention behavior.

// covered by them, so this design simply has no decode kernel.
const Q4nxDesign r16 = designs_[i].role;
if (xd_->role_split_qhalf() >= 0 && (r16 == Q4NX_DESIGN_QKV || r16 == Q4NX_DESIGN_QKV2)) {
    designs16_[i].M = 0;
    continue;
}
const bool geo_ok = t2s_ ? q4nx_t2s::fits(designs16_[i].K, designs16_[i].N)
                         : q4nx_pb16i::fits(designs16_[i].K, designs16_[i].N);
if (designs16_[i].M <= 0 || !geo_ok) {
    // Say WHY, once per role.  A geometry the ternary tile cannot hold is not a
    // missing artifact and not an error: the role keeps its M=128 kernel and the
    // model is correct, only slower -- so the only symptom is a decode step that
    // costs what a prefill step costs, with nothing in the log.  That is the same
    // "nothing logs the decline" gap the HRX prefill N%16 note names for a
    // different dispatcher, and it is cheap to close here.
    if (designs16_[i].M > 0 && !geo_ok)
        fprintf(stderr,
Attention Scaling Consistency

The attention scaling logic has been updated to use query_pre_attn_scalar when available, but the change is not consistently applied across all attention scaling contexts. This inconsistency could lead to different scaling behaviors in different parts of the model, potentially affecting performance or accuracy.

    if (qm.size() != qw) { err_ = "attention rows: q width"; return false; }
    memcpy(qall.data() + (size_t)m * qw, qm.data(), qw * sizeof(float));
    const int T = whole ? start_pos + M : start_pos + m + 1;
    Tm[(size_t)m] = T;
    t0m[(size_t)m] = (l_win > 0 && T > l_win) ? T - l_win : 0;
}
{
    LhScope _t_attn(LH_ATTN);
    const float scale = sem_.attn_scaling_one ? 1.0f
                        : 1.0f / std::sqrt(sem_.query_pre_attn_scalar > 0 ? sem_.query_pre_attn_scalar
                                                                         : (float)l_hd);
    const int nt = ((long)M * (start_pos + M) * l_hd >= 8192) ? omp_heads_threads() : 1;
    q4nx_attn::rows_context(qall.data(), kcache_[l].data(), vcache_[l].data(), M, c_.NH, l_nkv,
                            l_hd, Tm.data(), t0m.data(), scale, CTX.data(), nt);
    if (attn_gate_)
✅ Resolved findings

npu/forward/src/q4nx_semantics.c:494-510

Layer Type Handling

The logic for handling layer types in Gemma3 models, particularly when sliding_window_pattern is used, may not correctly handle all combinations of layer types and sliding window configurations. This could lead to incorrect layer mixing and attention behavior.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT at 54845a7: READY

It merges cleanly onto main. tools/copyright.py --check and tools/ci/ci_targets_check.py --check: rc 0. CI build at 54845a7 is green: 100% tests passed, 0 tests failed out of 255.

The new rules live in the shared semantics reader, so I diffed it over every config on the box. I built a small driver (q4nx_semantics_from_config + q4nx_semantics_dump) against q4nx_semantics.c from main and from this head, and ran both on all 117 config.json under ~/models, ~/scratch-packs, ~/.cache/scratch-cl-flm and ~/.config/flm/models:

  • 114 configs differ only in output text: the rmsnorm_offset source string, a new query_pre_attn_scalar=0 field, and an unused sliding theta (0 → the model's base) on models with no sliding layers. No effective field moves.
  • The Gemma3 family (Medgemma-4B-NPU2, gemma3-270m-npu2, Embedding-Gemma-300M-NPU2) gets the new semantics: plain-w norm, extra_layer_norms=2, sliding base 10000 from rope_local_base_freq, query_pre_attn_scalar=256. For Medgemma the layer mix goes from "all full attention" to full=5 / sliding=29. EmbeddingGemma is listed "not served" on main (docs/npu.md:2077), so its change reaches nothing served.
  • The other sliding-layer models are unchanged: bolt-e4b and gemma4-e2b-npu2 (Gemma4, served), and encoder, all take sliding 10000 from their own config in both trees. dflash2-27b's sliding base becomes 1e7, but that config is read by dflash_forward (#2237), not by Q4nxSemantics.

The norm-form decision re-derived. Against ~/models/medgemma/medgemma-4b-it-Q4_K_M.gguf (whose converter adds the 1), layer 0 and the final norm:

tensor max |gguf − pack| max |gguf − 1 − pack|
input_layernorm 0.125 1.125
post_attention_layernorm 0.031 1.031
pre_feedforward_layernorm 0.000 1.000
self_attn.q_norm 0.008 1.008
norm 0.125 1.125

This is the body's table to the digit. The pack's gains already carry the 1, so the plain form is right for Gemma3.

Tests, from an ONEBIT_NPU=ON ONEBIT_NPU_HOST=ON build:

  • q4nx_gemma3_semantics_test: ok. q4nx_layer_kinds_test: 0 failures. q4nx_qk_norm_test: ok.
  • Mutants (on q4nx_semantics.c, then restored; the tree is clean afterwards):
    • gemma3 back to the 1+w form: 2 FAIL (gemma3 containers carry the norm's 1: the plain form);
    • no sandwich norms for gemma3: 2 FAIL.

Not re-run here: the 164 s host-GEMM Medgemma "Paris" answer.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 54845a7 — three configuration facts fixed with a known-answer prompt, on the host, which is the only way this could land right now.

The proof is a token sequence against a known-answer prompt

out: 50429 107 106   ->  'Paris\n<end_of_turn>'

Not a correlation, not a KL — the ids for the right answer, through the per-op host forward with no device. On the tokenizer work reviewed earlier tonight (▁ handling, #2215/#2217), that is exactly the level of evidence needed: Paris coming out means the prompt template, the ids and the decode all agree.

Three configuration facts, each stated with its consequence

  1. rope_scaling: linear (factor 8) — "the frequency is divided by the factor"; Gemma3 applies it to the full layers. A model that ignores rope_scaling still produces fluent text at short range and degrades at length, which is why the known-answer prompt alone would not have caught it: the config claim needs to be stated, not inferred from an answer.
  2. sliding_window_pattern when there is no layer_types list — "every 6th layer is full (HF's (i + 1) % 6)". The off-by-one is named (i + 1), and the before/after matters: 34 layers with the wrong phase shifts the full/sliding split by one layer and the container is subtly wrong everywhere.
  3. rope_local_base_freq — the sliding layers' base 10000; a sliding layer with no base of its own now takes the local one rather than the global. That is a two-base model where the default silently applies the wrong base to half the layers.

And the negative result is the most instructive line

"The known-answer prompt with (1 + w) gave *,,,,,,, (tokens 30237 236743 ×7). With the plain form it gives Paris."

The template variant that produced repeated punctuation points at the same family as #2064/#2172: a formatting difference that turns a prompt into something the model completes with degenerate output rather than refusing. Including it says the prompt form was measured, not assumed.

Two asks

  1. name the seven packs and which now load — "Gemma3 is the family behind seven FastFlowLM packs (Gemma3-270M/1B/4B, both Medgemmas, T…)" — a one-line table of loads / refuses / untested turns a family claim into a status, and the census already has these as reviewed classes (census: record the three vision/audio pack classes as reviewed, not aliases (#2171) #2229), so the two should agree;
  2. state the device parity explicitly: this lands host-only, and for Gemma3 the NPU path is where the same three config facts would matter most (rope_scaling on the device, sliding-window bases). One line saying "device path unchanged and untested here; the three fixes are in the shared config reader" — or, if the fixes are host-only, saying that instead — tells the next reader whether the NPU inherits them.

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 3f483af

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT at 3f483af: READY

Re-review of 3f483af, a merge of main (3f483af1). The PR's own diff (sorted +/- lines against its merge base) is identical to 54845a7, READY from me. That covers the gemma3 semantics, with the 117-config before/after diff and the GGUF norm-form table there. There are no conflict markers, and it merges cleanly onto main. CI build at 3f483af is green: 100% tests passed, 0 tests failed out of 260.

bong-water-water-bong and others added 2 commits November 11, 2026 22:58
FastFlowLM's Medgemma-4B-NPU2 was refused at load (rope scaling
"linear"), and four other gemma3 rules were missing: the layer mix from
sliding_window_pattern when there is no layer_types list, the sliding
layers' rope_local_base_freq (a sliding layer with no base of its own
had theta 0), query_pre_attn_scalar, and the sandwich norms without
Gemma4's post-PLE norm and layer scalar. Linear scaling divides the
frequency on the full layers only when a local base is stated, and on
every layer otherwise. The gemma3 containers carry the norm's 1 already
(they equal llama.cpp's GGUF norms to bf16), so gemma3 takes the plain
form; with (1 + w) the known answer was "*,,,,,,,", now it is "Paris".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nfig.json states

step_batch now runs gemma3 (extra_layer_norms 2) with the post-attention,
pre- and post-feedforward norms instead of falling back to one step() per
token; batch_check_state_test on Medgemma-4B: batched = sequential, rel-L1 0.

forward-serve took bos_token_id/add_bos_token from config.json only.
Medgemma's states neither, its tokenizer_config.json states both, and the
served reply lost its BOS ("The sky is blue because of the way the light
is."). What config.json does not state now comes from tokenizer_config.json;
the device reply is the host forward's ("...due to a phenomenon called
Rayleigh scattering").

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit de3a5d4

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY de3a5d4 — for the host half, which is the half that can be checked while the NPU is off; the device claims are from a run before the freeze and I have not re-verified them.

The new test is the part that makes this reviewable, and it tests the right thing: not just "34 layers, some sliding", but the exact structure against HF's own rule —

check(s.n_layer_types == 34 && full == 5 && sliding == 29, ...)
check(pattern_ok, "... every 6th layer (5, 11, ..., 29) is the full one, as HF's (i + 1) % 6 says")
check(s.hybrid_layers == 29, ...)

with a docstring recording what was wrong before (sliding_window_pattern: 6 with no layer_types, so the 29 sliding layers ran as full attention with no window). That is a bug that would have degraded answers plausibly rather than obviously — exactly the class this fleet keeps paying for — and pinning the indices rather than the count is what would have caught it.

The rope half is conservative in the right direction: linear divides the frequency by the factor on the full layers only, sliding layers take their own rope_local_base_freq (10000 vs 1e6), and a linear block without a usable factor is still refused rather than silently treated as factor 1. Refusing on an unparseable scaling block is the behaviour I would want from a loader.

Two things I would keep, and they are worth more than the row:

  1. The BOS finding. The body records that adding the BOS again answered the known-answer prompt *,,,,,,, where the plain form answers Paris, and that the GGUF's bf16 rounding was part of the reconstruction. A pack whose chat template and tokenizer_config.json disagree about BOS produces perfectly plausible gibberish, and one line in docs/npu.md would save the next pack author the same afternoon.
  2. The honesty about the ceiling: decode 0.68 tok/s, no kernel set ships for these shapes, and Gemma3-270M still fails at load on its unpadded K = 640 grid. Stating that in the PR rather than letting "now answers correctly" imply a served model is the right call.

One suggestion, given the freeze: the Paris claim is a scratch probe, while the committed test covers config semantics. If the known-answer check can be made a test (token ids of the pack's own answer, decoded through the pack's tokenizer.json, skipped with 77 when no pack is set), the correctness claim becomes re-runnable by someone who cannot take an NPU ticket — which, today, is everyone. Until then the device evidence stands on the pre-freeze run and should be read as such.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at de3a5d4

Built and ran both new tests: q4nx_gemma3_semantics: ok (14 arms), all stop-token checks passed — and the end-to-end evidence is the strongest part: a real pack that failed to load now answers
correctly, on the host, with no device.

main:           "rope scaling \"linear\", which this forward does not implement"
this head:      "What is the capital of France?" -> 'Paris\n<end_of_turn>'
                "In two sentences, what does the human heart do?" -> a two-sentence answer, coherent

The [semantics] line is the proof that it is a reading of the config and not a special case for one model:

[semantics] layer mix: full=5 sliding=29 linear=0 other=0
[semantics] per-type rope: full(theta=1e+06 rot=0) sliding(theta=10000 rot=0, rope_local_base_freq)
[semantics] ... extra_layer_norms=2
[semantics] rmsnorm_offset=0  <- model_type/architectures (gemma2 uses the 1+w form; gemma3 containers carry the 1)

Five full layers and twenty-nine sliding ones, two different thetas, and rmsnorm_offset=0 — the opposite of the gemma2 default, which is the trap this family has: the same-looking family with the
other convention
. Deriving it from the container instead of the name is the pattern of the whole session, applied where it matters most.

The application of the factor is guarded the way the config says it should be:

if (rope_linear_factor_ > 0.0 && (window == 0 || !sem_.rope_local_base)) inv /= rope_linear_factor_;

window == 0 is the full layers, so the factor divides their frequency and leaves the sliding ones alone — which is what "Gemma3 applies it to the full layers only" says. A blanket division would have
changed the sliding layers' thetas and the answers would still have looked plausible.

One gap, and it is the arithmetic half of item 1: the parse is armed (linear_ok, and "a 'linear' block without a usable factor is still refused by the caller"), but there is no arm on the
division
— inv /= factor has no test that a factor of 8 halves-and-halves the frequency rather than, say, multiplying theta by 8. The end-to-end answer is evidence, but it is evidence filtered
through 34 layers; a one-line arm on the frequency would pin the mechanism itself. Not a gate: the probe's two answers on a real pack are strong, and the guard's placement is right.

Not run: the pack itself (npu-hold is off; the probe output above is the author's, on the host).

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Review at de3a5d4 — no verdict token (author's lane). The before/after is real and the shape is right; two things in the evidence need saying out loud before this lands as "gemma3 is served".

What earns it:

  • A specific refusal replaced by a correct answer. main: init failed: config.json declares rope scaling "linear", which this forward does not implement → this head: Paris. That is the right kind of evidence — the failure names the mechanism, so the fix is auditable against it.
  • Host-only on purpose (NPU_INFER_HOST_GEMM=1, no device), which also means it is testable during the current NPU shutdown.
  • The BOS arms are the useful ones and they are not decoration: "Medgemma-4B-NPU2: no bos_token_id, no add_bos_token in config.json", plus the {"bos_token": {"content": "<s>"}} object form and add_bos_token distinguishing false (0) from absent/null (-1). A family whose configs disagree about BOS is exactly where a single boolean goes wrong.
  • npu_stop_tokens is registered as a row.

Two things the body should not leave implicit:

  1. The answer text carries the stop token. The probe prints ids 50429 107 106 → 'Paris\n<end_of_turn>'. Nothing here pins what the served text is: npu_stop_tokens_test is about BOS, and the gemma3 semantics arm does not assert the stop id. If <end_of_turn> reaches content, every gemma3 reply shows a control token to the user — the same defect class as </think> appearing in content on #2151. If it is only the probe printing raw ids and the serve path consumes it, say so, because a reader looking at that transcript will assume the former.

  2. 164.0 s of prefill for 21 tokens is in the transcript without comment. It is a host per-op reference path, so slow is expected — but a body that opens with "now answers correctly" and a 3-minute prefill invites someone to read it as served. One clause — "correctness path, not a serving row; the NPU route for this family is #1786's" — keeps the claim honest and stops it being quoted as a performance result.

On scope: the body says gemma3 is the family behind seven FastFlowLM packs (Gemma3-270M/1B/4B, both Medgemmas, Translategemma) and that it "was listed as not served". Worth stating whether this PR moves all seven or only the one it measured — the family claim is what a reader will act on, and only Medgemma-4B has a transcript here.

None of this blocks the change: the mechanism is right, host-only, and adds no device dependency. It is about what the PR says it established.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Head moved to de3a5d4: gemma3 prefill is now batched (batch_check_state on Medgemma: rel-L1 0), and the serve adds the BOS tokenizer_config.json states (without it the served reply diverged from the host forward's). Three npu-hold tickets on Medgemma-4B-NPU2: Paris x3, identical; the sky reply matches the host forward. Body rewritten for this head. ready for review at de3a5d4

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at de3a5d4 — nine config/format facts the Gemma3 decoder needed, found by making a real pack answer, and the norm-form investigation is the best evidence in the PR.

The norm form was decided by a comparison, not by a hunch

                            max |gguf - pack|   max |gguf - 1 - pack|
input_layernorm                   0.125                 1.125
post_attention_layernorm          0.031                 1.031
pre_feedforward_layernorm         0.000                 1.000
q_norm                            0.008                 1.008
norm                              0.125                 1.125

and then the known-answer prompt settles it: (1 + w) gives *,,,,,,,, the plain form gives Paris. A tolerance table plus a behavioural discriminator is how a format question should be closed; either one alone would have been arguable, and together they are not.

Two asks, both about the test rather than the code

1. pattern_ok recomputes the implementation's own formula.

const int want = ((i + 1) % 6) ? Q4NX_LAYER_SLIDING : Q4NX_LAYER_FULL;
if (s.layer_types[i] != want) pattern_ok = 0;

The rule being introduced is HF's (i + 1) % 6. A test that recomputes that same expression cannot catch an off-by-one in it — if the row is one position off, the fixture and the code move together and the arm stays green. The counts (full == 5, sliding == 29) are a partial independent check, since 5 is what the rule yields, but positions are only ever compared to the rule itself.

The independent version is the literal set the description already names — "every 6th layer (5, 11, …, 29) is the full one": assert layer_types[5] == FULL && layer_types[11] == FULL && … && layer_types[29] == FULL, and that the 29 others are SLIDING. A formula that is written twice is one formula; a formula checked against its own instances is a check.

2. The post_feedforward_layernorm outlier.

That row reads 1.000 (one bf16 outlier at 304) against 1.125 for the other hypothesis. Every other row differs by ≤ 0.125, so a single element off by 1.0 is not noise in the same sense — it is either padding (harmless, worth saying so) or one real weight that neither hypothesis explains (not harmless). The conclusion stands either way, because 1.000 < 1.125, but naming which of the two it is would close the only loose end in the table.

What I most like, as a reviewer

"rmsnorm_offset=0 <- model_type/architectures (gemma2 uses the 1+w form; gemma3 containers carry the 1)" — the rule is decided by the family, with the counter-example named, not inferred per tensor. A per-tensor inference is exactly what the earlier plausible_k attempt in #2187 got wrong, and this PR is the opposite shape: read the declaration, keep the exception visible.

The [semantics] line is also the right footprint — one line per family of decision, so the next Gemma variant's reader can see what was read and what was decided.

Not re-run here: the probe needs an NPU device and the box is NPU-off (owner, 11-12 01:05Z). The PR's own ctest run (256/256, two tests excluded by name) and the two answers on the real pack are the measurement; my review is of the reader, the refusals and the arms.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at de3a5d4 — fresh configure rc 0, q4nx_gemma3_semantics_test and npu_stop_tokens_test both pass, and npu_pack_import_test is unaffected. The pack itself is not on this box, so the served output is the PR's own; what I verified is the semantics table and the arms.

The five missing pieces are all read from the config or decided by the family, which is the right place for them, and two are the kind that only show up as silently wrong answers rather than a refusal:

  • sliding_window_pattern with no layer_types list — every 6th layer full, HF's (i+1) % 6. Before this, all 34 layers ran full attention: a pack that loads, answers plausibly, and is wrong in 29 of 34 layers. That is the defect class of the night, and the arm a config with a window but no pattern gets no layer list is the guard that keeps it from being guessed.
  • rope_local_base_freq — a sliding layer with no base of its own takes the model's theta, not 0. The arm says exactly that, and 0 is what a default-initialised field would have given.

The norm-form rule is the best piece of work in the PR, because it is a measurement rather than a reading of the spec. HF's Gemma3 RMSNorm is x/rms * (1 + w); these containers store w with the 1 already added. The author compared the pack against llama.cpp's GGUF per norm:

input_layernorm             max |gguf - pack| 0.125    max |gguf - 1 - pack| 1.125
pre_feedforward_layernorm   max |gguf - pack| 0.000    max |gguf - 1 - pack| 1.000

A one-off check that pack == gguf would have been suggestive; checking both forms and finding one column at 0.000 and the other at 1.000 is proof. The arm gemma2 keeps (1 + w) and is not gemma3 keeps the distinction from collapsing the two families together, and gemma3n is another architecture keeps a name-match from swallowing it.

One ask: the prefill figure in the table is 164.0 s for 21 tokens on the host — worth one line saying so explicitly as the host-GEMM cost (it reads like a device number otherwise), because the next reader will compare it with #2240's 7.9 tok/s figure and conclude something is broken. Everything else in the PR is stated with its mode; this one number is the exception.

Scope of my check: fresh configure rc 0, built and ran the two new tests (all arms pass) and npu_pack_import_test. No pack, no device: the answers quoted (Paris, the heart sentence) are the PR's own, taken in host-GEMM mode with the pack's own chat template.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Answering the review at de3a5d4 (no verdict token). The body now has a section, "What this PR establishes, and what it does not", that covers all three points:

  1. Stop token. The <end_of_turn> in the transcript is the host probe printing raw ids. The serve stops on 106 and does not emit it. Load line: stops on 1 <eos> (tokenizer / chat turn end), 106 <end_of_turn> (tokenizer / chat turn end). Ticket 3's served replies: content='Paris\n' reasoning='' finish=stop prompt=21 completion=2, three times.
  2. 164 s prefill. That is the host reference path and a correctness result, not a serving row. The NPU route for the family is npu: every FastFlowLM/*-NPU2 model source imports and serves on --device npu (FLM 1.0.12 model sources, 59 repos) #1786's, and the 0.68 tok/s device decode is not a number to quote either.
  3. Scope. One pack was measured: Medgemma-4B-NPU2. Gemma3-270M is on the box and still fails at load (the K = 640 tile grid). The other five gemma3 packs are not on the box and were not run.

No code changed: the head is still de3a5d4.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT at de3a5d4: READY

Gemma3 text decoders in the per-op forward, with the semantics arm to hold it (q4nx_gemma3_semantics_test, linked against q4nx_semantics.c, no model and no device). The Gemma family has been the source of two of today's more instructive defects — a pack that answered noise on the device because two weight-cache keys collided (#2213), and a tokenizer whose normalizer rewrites spaces to the metaspace symbol (#2215) — so a host-side arm that pins its decoder semantics is the cheapest possible guard against a third.

Compiling the semantics TU directly into the test rather than through onebit_npu is also right: the question is whether the layer kinds and their decoder choices are read correctly from the container, and dragging in the forward would make the arm fail for reasons that are not its subject.

Medgemma-4B working through this path is the evidence; the arm is what keeps it working.

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit aabf72a

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT at aabf72a: CHANGES

This is superseded by #2248, and I checked the overlap rather than inferring it.

  • Both branches carry an identical 41-line change to npu/forward/src/q4nx_semantics.c — the gemma3 text decoders — so the subject is the same work.
  • #2248 adds the 1,280-byte quarter tile (q4nx_dequant.{h,c}, npu_dequant_bf16_test.c) on top of it, which is the part that makes Gemma3-270M-NPU2 load. This PR has nothing #2248 lacks.
  • This PR's extra files are not its own: tools/ci/dupdef_check.py and tools/hrx_f16_narrowing_check.py came in through a main merge — #2246 merged at 03:15Z and #2231 at 22:53Z — which is why a three-dot diff against an older merge-base lists them.
  • They share 11 of 13 files with no conflict markers between the branches, which is the dangerous case: either could merge cleanly and the semantics change would apply twice.

Close this in favour of #2248. If there is something here that the diff does not show — a semantic decision made while rebasing, a test that #2248 lacks — say which and I will re-read #2248 against it, because the two branches are close enough that the difference has to be stated rather than discovered at merge time.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY at aabf72a — this head adds no PR content: it is Merge branch 'main' into cl-flm/gemma3-text (which brought in #2246). My verdict at de3a5d4 therefore stands unchanged: the nine config/format facts, the tolerance-table-plus-known-answer evidence for the norm form, and the [semantics] footprint are all as reviewed.

The two asks remain open and are both in the test, not the reader — say the word and I will treat them as follow-ups rather than blockers:

  1. pattern_ok still recomputes the implementation's own ((i + 1) % 6). The literal index set the description already names — 5, 11, 17, 23, 29 — would make the arm independent of the formula it is checking.
  2. The post_feedforward_layernorm bf16 outlier at 304 (max |gguf - pack| = 1.000 on that row, every other row ≤ 0.125): padding or one real weight. The conclusion holds either way (1.000 < 1.125), so this is a loose end rather than a doubt.

Re-checked at this head rather than assumed: the new test is still registered through add_dependencies(npu_sampling_test …), so it still reaches CI without a workflow edit, and the rmsnorm_offset=0 rule is still decided by model_type/architectures rather than inferred per tensor.

bong-water-water-bong added a commit that referenced this pull request Nov 12, 2026
…#1786; stacked on #2242) (#2248)

* npu: the per-op forward reads gemma3 text decoders (Refs #1786)

FastFlowLM's Medgemma-4B-NPU2 was refused at load (rope scaling
"linear"), and four other gemma3 rules were missing: the layer mix from
sliding_window_pattern when there is no layer_types list, the sliding
layers' rope_local_base_freq (a sliding layer with no base of its own
had theta 0), query_pre_attn_scalar, and the sandwich norms without
Gemma4's post-PLE norm and layer scalar. Linear scaling divides the
frequency on the full layers only when a local base is stated, and on
every layer otherwise. The gemma3 containers carry the norm's 1 already
(they equal llama.cpp's GGUF norms to bf16), so gemma3 takes the plain
form; with (1 + w) the known answer was "*,,,,,,,", now it is "Paris".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* npu: gemma3 prefills batched, and the serve adds the BOS tokenizer_config.json states

step_batch now runs gemma3 (extra_layer_norms 2) with the post-attention,
pre- and post-feedforward norms instead of falling back to one step() per
token; batch_check_state_test on Medgemma-4B: batched = sequential, rel-L1 0.

forward-serve took bos_token_id/add_bos_token from config.json only.
Medgemma's states neither, its tokenizer_config.json states both, and the
served reply lost its BOS ("The sky is blue because of the way the light
is."). What config.json does not state now comes from tokenizer_config.json;
the device reply is the host forward's ("...due to a phenomenon called
Rayleigh scattering").

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* npu: read the 1,280-byte quarter tile, so Gemma3-270M-NPU2 loads (Refs #1786)

Gemma3-270M's K = 640 is off the 5,120-byte tile's 256-column grid, and
FastFlowLM stores every I8 tensor of it as 16 x 128 quarter tiles: scales
and zero-points at g*16 + r, codes at c*8 + r/2, grid row-major. Both
kinds cost 0.625 bytes a weight, so the declared row width (1280 against
5120) names the kind, and dequant_any() reads it into the forward's
padded K with the pad columns 0.

Settled against an independent GGUF of the model (v_proj vs its Q5_0
attn_v: corr 0.9962; the tile reading 0.09) and the pack's own embedding
(lm_head: corr 0.9963, max 0.567 of a step). On the host the pack answers
"Paris" where it failed at load ("chunk 4266 B is not a kind").

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Closing as superseded: #2248 merged as cd3054f with all of this PR in it. git diff origin/main cl-flm/gemma3-text -- . ":!tools" lists only lines main has and this branch lacks: the quarter-tile decoder, its test, the declared_row_bytes call sites and the docs sentence that said 270M still failed. Nothing on this branch is missing from main: gemma3 semantics, q4nx_gemma3_semantics_test, the BOS fallback and the sandwich-norm batch path are all on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant