Skip to content

npu: LFM2.5-1.2B answers on the forward route — conv layers run, the symmetric tile kind is read, q/k norm on - #2170

Merged
bong-water-water-bong merged 11 commits into
mainfrom
cl-flm/lfm2-forward
Nov 11, 2026
Merged

bong-water-water-bong merged 11 commits into
mainfrom
cl-flm/lfm2-forward

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #1786 (step (b) and (c) of the LFM2 plan on that issue; #2158 was the refusal, #2161 the host reference).

FastFlowLM/LFM2.5-1.2B-NPU2 is now served on the per-op NPU route from its published container. Three things were needed, and only the first is about convolutions.

What changes

  1. The conv layers run. Ten of the sixteen layers carry shortconv.* and no attention. Q4nxModelDims.shortconv_n derives one more design (the fused-projection role at K=H, N=3H, no Z); in_proj and out_proj go to the device (weight keys 0 and 1, the slots an attention layer's QKV and O have), and the gate + causal depthwise conv between them is lfm2_shortconv.h (npu: LFM2's gated short convolution as host arithmetic, with known answers — first slice of serving the lfm2 packs (Refs #1786) #2161), shared by decode, batched prefill and the verify rollback (shortconv_push). The state lives in conv_state_, so snapshot / restore / reset needed no new code. Which layers are conv layers and the kernel length come from the container's tensors. The npu forward: a pack with short-convolution layers is refused by name at load — LFM2.5 said ready and answered 500 to everything (Refs #1786) #2158 refusal is replaced by narrower ones (kernel length differs between layers, taps not bf16 [H][L], an image weight layout).
  2. A second kind of 5,120-byte weight tile. The LFM2 packs store symmetric tiles: same layout, every zero-point 0, a signed scale, a two's-complement code. Nothing in the container names the kind, so q4nx_dequant_tile() reads it off the tile (all 256 zero-points zero).
  3. lfm2 gets per-head q/k RMSNorm, and norm_eps is read when a config has no rms_norm_eps.

Measured (this box, 2026-11-11)

First device run, before (2) and (3): loads, three identical answers, and the text is word fragments (' separ Comm stapirm stalgi…'). A host-GEMM run gave the same first tokens, so it was not the kernels.

The tile kind. Pack tensors against LFM2.5-1.2B-Instruct-Q4_K_M.gguf, correlation per tensor:

                 before    after
in_proj          -0.393    0.994     (B | C | X blocks on the diagonal both times: the order was right)
out_proj         -0.394    0.994
q/k/v/o_proj     -0.39..   0.993-0.994
gate/up/down     -0.393    0.994-0.996
conv taps, norms, embedding: 1.000 both times (bf16)

Tie test, the pack's quantised lm_head against its own bf16 token_embd (first 2048 rows), |dequant - embedding| / |scale|:

two's complement * scale   corr  0.99573   max 1.000   mean 0.243   over 0.5: 0.48 %
(code - 8) * scale         corr -0.45833   max 9.000   mean 7.971
unsigned code * scale      corr -0.39544   max 16.5    mean 7.270

Census of every 5,120-byte tile in the fifteen packs on the box: 1,889,236 tiles in the non-LFM2 packs, 0 with an all-zero zero-point block (so their decode is unchanged); 285,696 in LFM2-1.2B + LFM2.5-1.2B, all of them.

q/k norm. With only the tiles fixed, host run: What is the capital of France? Answer with one word. -> France. The GGUF on CPU -> Paris. With qk_norm=1 -> Paris.

Against FastFlowLM 1.0.12's runtime, one npu-hold ticket, theirs then ours, same model.q4nx (sha256 b341d42021e617a5… both), five greedy prompts, max_tokens 40:

p0 SAME  'Paris'
p1 SAME  'One, Two, Three, Four, Five, Six, Seven, Eight, Nine, Ten'
p2 DIFF  common prefix 181 chars; theirs then ', known as Rayleigh scattering, makes the sky'; ours then ' makes the sky look blue to observers on the ground'
p3 DIFF  common prefix 0 chars; theirs 'Certainly! Below is a Python function…'; ours "Here's a Python function…"
p4 SAME  'Das Wetter ist heute schön und wir gehen spazieren.'
identical answers: 3 of 5

Where they part, and by how much (top logprobs at that token):

p3 first token   ours, device:      Here -0.6364   Certain -1.2590   S -2.1025   To -3.3823
                 ours, host float:  Here -0.6571   Certain -1.2312   S -2.2114   To -3.1700
                 theirs:            Certain -0.8118   Here -1.0618   S -1.8118   To -3.6868     (same answer 3 runs of 3)
                 GGUF on CPU:       opens with "Here's"
p2 after 'This scattering effect'   ours, device:  ' makes' -0.7527   ' gives' -1.7105   ',' -2.0324

So it is not a near-tie on this side, the device agrees with float math on the same weights, and the two runtimes rank the same candidates with the first two swapped. I do not know their arithmetic and am not claiming which is right; the docs row says exactly this.

The rest, same pack, one ticket:

known answer: 3 answers, identical=True, content is Paris=True
[batchcheck] M=1 x5, M=4, M=19, M=20, M=21, M=22, M=26: all MATCH   (NPU_BATCH_CHECK=1, five prompts)
prefix cache: A1 x5 identical across cache-on and cache-off, incl. two answered with cached=351 of 352; B1 x2 identical
decode: 4.6 tok/s (load 3-4), 3.5-3.7 tok/s (load 8-10); FastFlowLM on the same replies: 48-83 tok/s

The prefix-cache script printed FAIL on its last line: its rule wants every step in both arms and my sequence ran step A2 in one arm only. Every answer it did compare is identical; I am reporting the line rather than rerunning for a green word.

Unit tests (all rc 0): lfm2_shortconv_test (new rollback arm), npu_dequant_bf16_test (8 new tile-kind checks), npu_design_grid_test (11 new), q4nx_layer_kinds_test (6 new), npu_forward_loader_test, npu_pack_test, dflash_draft_test. tools/copyright.py --check rc 0.

Not covered

  • LFM2-1.2B-NPU2 (same tensor grid and tile kind, not run) and LFM2-2.6B-NPU2 (not run).
  • The pin-on-first-request path reported pinned=False on this pack; answers are unaffected, the pin is not exercised.
  • Speed: this is the per-op route. No fused set for this geometry.
  • On my build dir, after merging main, npu_rt_shim_test and npu_dmabuf_import_test fail to link (npu_rt_api::ext::bo::bo undefined). They came with npu: execute on an imported dma-buf -- the engine's arm, and the claim it settles (engine#753) #2037 and touch nothing here; CI will say whether that is my stale configure.

🤖 Generated with Claude Code

bong-water-water-bong and others added 6 commits November 11, 2026 05:53
…swers (Refs #1786)

npu/forward/include/lfm2_shortconv.h: the block between an LFM2 conv
layer's two projections (B*X, a causal depthwise convolution over the
last L-1 values, gated by C), with its per-layer state, defined once for
decode, batched prefill and state snapshots. Not wired into the forward
yet; this is the reference the wiring will be checked against.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A layer that carries shortconv.* tensors is no longer refused: in_proj
(H -> 3H) derives one fused-projection design, out_proj reuses the O design,
and the gate + depthwise causal conv between them is lfm2_shortconv.h on the
host, shared by decode, batched prefill and the verify rollback. The state
lives in conv_state_, so snapshot/restore/reset cover it. norm_eps is read
when a config has no rms_norm_eps.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The LFM2 packs store their projections as symmetric tiles: the same
5120-byte layout with every zero-point 0, a signed scale and a
two's-complement code. Read as the asymmetric kind they correlate -0.39
with the upstream weights; the tile decoder now tells the kinds apart by
the zero-point block. lfm2 also gets per-head q/k RMSNorm, which its
attention layers carry.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…inst FastFlowLM's runtime

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
# Conflicts:
#	npu/forward/include/lfm2_shortconv.h
#	tests/lfm2_shortconv_test.cpp
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at 8cc2c98

One thing a reviewer should know: the device rows in the body were taken on a binary built at d64e4f0 (the code as it is here), before main was merged in at 8cc2c98. After the merge I rebuilt 1bit and the seven test targets and reran the unit tests (all rc 0); I did not rerun the device tickets. The merge brought no change to the files this PR touches (git diff origin/main --stat is the thirteen files listed in the diff).

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 61143ba)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1786 - Partially compliant

Compliant requirements:

  • The model source FastFlowLM/LFM2.5-1.2B-NPU2 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 row names the kernel set and the engine commit that produced it
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Tile Kind Detection Precision

The q4nx_tile_kind function detects whether a tile is symmetric or asymmetric based on zero-point values. The current implementation assumes that if any zero-point is non-zero, the tile is asymmetric. However, it's possible that a tile could have zero-points that are not exactly zero (e.g., due to floating-point precision issues) but still represent a symmetric tile. This could lead to incorrect dequantization behavior, especially in hardware contexts where precision is critical.

int q4nx_tile_kind(const uint8_t* tile) {
    const uint8_t* sc_b = tile;
    const uint8_t* zp_b = tile + Q4NX_TILE_ROWS * Q4NX_GROUPS_PER_ROW * 2; /* +512 */
    for (int i = 0; i < Q4NX_TILE_ROWS * Q4NX_GROUPS_PER_ROW; i++)
        if ((le16(zp_b + 2 * i) & 0x7FFF) != 0) return Q4NX_TILE_ASYMMETRIC;   /* +0.0 and -0.0 both count as zero */
    for (int i = 0; i < Q4NX_TILE_ROWS * Q4NX_GROUPS_PER_ROW; i++) {
        const uint16_t sc = le16(sc_b + 2 * i);
        if ((sc & 0x8000) && (sc & 0x7FFF)) return Q4NX_TILE_SYMMETRIC;        /* a scale below zero */
    }
    return Q4NX_TILE_UNDECIDED;
}
Conv Layer State Management

The implementation of gated short-convolution layers introduces new state management for convolution layers, including handling of conv_state_ and sc_rows_. The code relies on conv_state_ for snapshot/restore/reset functionality, but there's a potential for incorrect state handling during batched prefill and verify operations. Specifically, the conv_apply_rows function and related state management in commit_verify_rows need to ensure that the convolution state is correctly updated and restored, especially when dealing with rollback scenarios.

}
const auto cv_t1 = std::chrono::steady_clock::now();
verify_open_ = false;                        // the replay below must not re-record
if (accepted > 0 && shortconv_) conv_apply_rows(accepted);
if (accepted > 0 && !gdn_apply_rows(0, accepted)) return false;
Conv Layer Initialization

The initialization logic for short-convolution layers in init_impl includes checks for kernel length consistency and validation of tensor shapes. If the kernel length differs between layers or if the tensor shapes do not match the expected format, the function returns an error. This is critical for ensuring that the convolution layers are correctly initialized, but the error messages could be more specific to aid in debugging.

{
    int first_conv = -1;
    const int n_conv = model_shortconv_layers(mw_, cfg.num_layers, &first_conv);
    shortconv_ = n_conv > 0;
    sc_layer_.assign((size_t)cfg.num_layers, 0);
    sc_taps_ = 0;
    if (shortconv_) {
        if (pb16i_) {
            err_ = "model.q4nx has gated short-convolution layers (LFM2's hybrid) and config.json declares an "
                   "image weight layout (pb16i/t2s); the conv layers' projections have no image form here";
            return false;
        }
        for (int l = 0; l < cfg.num_layers; l++) {
            const LayerWeights* lw = &mw_->layers[l];
            if (lw->shortconv_in_proj_weight.shape[0] == 0) continue;
            sc_layer_[(size_t)l] = 1;
            const TensorDesc* t = &lw->shortconv_conv_weight;
            const int L = (int)t->shape[1];
            const std::string where = "model.layers." + std::to_string(l) + ".shortconv";
            if (lw->shortconv_out_proj_weight.shape[0] == 0 || t->shape[0] == 0) {
                err_ = where + " has in_proj but not conv.weight and out_proj.weight";
                return false;
            }
            if ((int)t->shape[0] != cfg.hidden_size || L < 1 || L > 64 ||
                (size_t)t->data_size != (size_t)cfg.hidden_size * (size_t)L * 2) {
                err_ = where + ".conv.weight is not bf16 [hidden_size][L] (shape " +
                       std::to_string((long long)t->shape[0]) + " x " + std::to_string((long long)t->shape[1]) +
                       ", " + std::to_string((unsigned long long)t->data_size) + " bytes, hidden_size " +
                       std::to_string(cfg.hidden_size) + ")";
                return false;
            }
            if (sc_taps_ == 0) sc_taps_ = L;
            else if (L != sc_taps_) {
                err_ = where + ".conv.weight has kernel length " + std::to_string(L) + " where layer " +
                       std::to_string(first_conv) + " has " + std::to_string(sc_taps_) +
                       "; one kernel length per model is what this forward runs";
                return false;
            }
        }
        if (cfg.num_attention_heads * cfg.head_dim != cfg.hidden_size) {
            err_ = "gated short-convolution layers need out_proj (hidden_size -> hidden_size) to be the attention "
                   "output geometry, and num_attention_heads * head_dim is not hidden_size here";
            return false;
        }
        fprintf(stderr, "  [forward] %d gated short-convolution layer(s) of %d, kernel length %d (LFM2 hybrid)\n",
                n_conv, cfg.num_layers, sc_taps_);
    }
}

⚠️ Review coverage: The following files were not included in this review because of the token budget:

  • docs/npu.md
✅ Resolved findings

npu/forward/src/q4nx_dequant.c:32-37

Tile Kind Detection

The q4nx_tile_is_symmetric function detects whether a tile is symmetric or asymmetric based on zero-point values. The current implementation assumes that if any zero-point is non-zero, the tile is asymmetric. However, it's possible that a tile could have zero-points that are not exactly zero (e.g., due to floating-point precision issues) but still represent a symmetric tile. This could lead to incorrect dequantization behavior.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 8cc2c98 — a real acceptance, an honest before/after, and the one number the body must not leave implicit.

The acceptance is model-level, which is the bar this row needs

before (first device run): loads, three identical answers, and the text is **word fragments**
after q/k norm:  "What is the capital of France? Answer with one word." -> "France"
                  known answer: 3 answers, identical=True, content is Paris=True

"Word fragments" → "France" is the same shape #1976 used for Hunyuan-7B, and it is the right bar for a row that says serves: not a metric, a correct answer to a factual question, repeated. 24 arms alongside it is the right density.

The number to state as a follow-up, not to hide

decode: 4.6 tok/s (load 3-4), 3.5-3.7 tok/s (load 8-10);  FastFlowLM on the same replies: 48-83 tok/s

That is a ~13× gap, measured on the same replies, and the honest thing is that it is fine for this row and not fine to leave unsaid. #1786's row is "every FastFlowLM NPU2 source serves" — serving is what this PR establishes, and the gap is the next work item. Put it in the body as exactly that ("serves; 4.6 vs 48–83, per-op route, tracked as …"), or the next reader will take "served" for "at parity" and be surprised in a demo.

Also worth keeping: the rates are load-conditioned (4.6 at load 3-4, 3.5-3.7 at load 8-10). That is #2140's discipline applied without being asked — a rate quoted with its conditions.

The stack is right, and it is worth saying because of tonight

#2170 contains #2161: YES   (git merge-base --is-ancestor)

So lfm2_shortconv.h arrives here by inheritance, not by copy. Two other PRs tonight (#2147, #2164) carried byte-identical copies of open PRs' files with no ancestry and had to be unwound; this one is the correct shape — #2158 refusal → #2161 host reference → this — and it is why the diff could be reviewed as "the serving step" rather than as three lanes' work.

Nothing else: thirteen files, the two siblings named, the acceptance measured, and one number that belongs in the body as the follow-up.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Head moved to 551db57 (docs only): a second pack of the family, and it answers the caveat in my last comment, because this ticket ran the binary built at the merged head 8cc2c98.

LFM2-1.2B-NPU2 (sha256 2755ab40cd927e3c…, same file for both runtimes), same paired script, same kernel directory, one npu-hold ticket of 76 s:

p0 SAME  'Paris'
p1 SAME  'One, two, three, four, five, six, seven, eight, nine, ten'
p2 DIFF  common prefix 21 chars
p3 DIFF  common prefix 58 chars   (theirs returns "Input should be a positive integer" for n <= 0, ours returns 0)
p4 DIFF  common prefix 53 chars   (same German sentence, then a different follow-up remark)
identical answers: 2 of 5        decode: ours 3.67-3.76 tok/s, theirs 42.0-46.8

No logprobs were taken for this pack and each prompt was asked once; the docs row says so.

ready for review at 551db57

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 551db57

@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 551db57: CHANGES

Reviewed head 551db57 (refs #1786, LFM2 steps (b) and (c)). This is the first verdict. Most of this is right and well tested. The blocker is the per-tile kind rule, and CI found the case that breaks it.

CI is red on this PR, and it reproduces. The Test step has one failure out of 233: npu_moe_container with "FAIL and it recovers the known weights exactly (the engine converter's container)". The test self-compiles tests/npu_moe_container_test.cpp from source each run, and it passes on main but fails at this head (ctest -R '^npu_moe_container$' locally, both trees).

Why. That fixture builds a q4_1 tile with scale 1.0 and zero-point 0 (npu_moe_container_test.cpp:53, "a scale of 1.0 and a zero of 0 so the decoded weight is exactly the code"), codes row % 16. Every zero-point is 0, so q4nx_tile_is_symmetric() calls it symmetric, and codes 8–15 decode as −8…−1. That is the hazard the header's own parenthesis waves off ("an asymmetric tile whose 256 group minima are all exactly 0 would be a matrix with no negative weight"). It is not hypothetical: the engine's own converter test writes one. The census of fifteen packs shows real packs do not hit it today, but the rule decides the kind from data that an asymmetric tile is allowed to have.

Suggested direction. Decide the kind once per tensor, or per container, instead of per tile, and refuse a tensor whose tiles disagree:

  • An LFM2 tensor is all-symmetric, so "every tile has an all-zero zero-point block" still identifies it.
  • A tensor with some all-zero blocks among asymmetric ones (this fixture's single tile alone cannot tell, but a real projection has many tiles) stays asymmetric.
  • Better still, key it on something the container says. Where FastFlowLM's metadata or the pack's config.json names "Q4_0" vs "Q4_1", that is a fact, not an inference.

Just changing the fixture's zero-point to make the test pass would hide the hazard rather than fix it.

What holds

  • Every 5,120-byte-tile decoder goes through q4nx_dequant_tile() (q4nx_dequant.c:129/161/242/288, q4nx_pack.c:710), so the reading reaches the full_i8_* device weights too.
  • npu_dequant_bf16_test, npu_design_grid_test, q4nx_layer_kinds_test and lfm2_shortconv_test pass.
  • Three decode mutants are caught by named arms: only 8 zero-points checked, −0.0 counted as non-zero, offset-binary decoding.
  • q4nx_forward.cpp compiles clean with the NPU flags. git merge-tree onto main is clean.

bong-water-water-bong and others added 2 commits November 11, 2026 08:33
…e kind

A zero offset alone decided the kind per tile, and npu_moe_container's
fixture (scale 1.0, zero-point 0) was read as symmetric. A tile is now
symmetric only with all zero-points zero and a scale below zero; the
tensor-level readers take the kind from the whole tensor, undecided tiles
follow it, and a tensor with both kinds is refused.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Answering the CHANGES at 551db57: correct, and I should have caught it, because I ran the seven tests I touched and not the suite. Fixed at 7a315fa.

Reproduced first.

$ ctest -R '^npu_moe_container$'           # at 551db57
FAIL and it recovers the known weights exactly (the engine converter's container)      1 of 1 failed

The rule now (q4nx_dequant.h has the text, q4nx_dequant.c the code):

  • A tile is symmetric on its own evidence only when its zero-points are all zero and some scale is below zero. A zero offset with positive scales decides nothing (Q4NX_TILE_UNDECIDED) and reads as asymmetric, as it always did. That is your fixture.
  • A tensor is one kind. The four tensor-level readers (_chunked, _packed, _tensor, _tile_row) look at the tensor's tiles first: any symmetric tile makes it symmetric and its undecided tiles follow; a symmetric and an asymmetric tile in one tensor returns -1 with a line on stderr that says why.
  • q4nx_dequant_tile() on a lone tile uses that tile's evidence.

I did not key it on a container fact because there is none: both packs have __metadata__: null, config.json has no quantisation key, and "Q4_0" exists only in FastFlowLM's model list, which is not in a pack. If you would rather have an explicit config.json key than a data rule, say so and I will add it as an override.

Why the sign is the evidence: a symmetric scale is max/-8 of its group, so it is negative whenever the group's largest weight is positive. Census redone with that column: 1,889,236 tiles in the non-LFM2 packs, 0 with an all-zero zero-point block and 0 with a negative scale; 285,696 in the two LFM2 packs, all with both.

After.

$ ctest -R '^npu_moe_container$'           # at 7a315fa
100% tests passed
$ ctest -j 6                               # whole suite, before the merge of main, same code
99% tests passed, 2 tests failed out of 236        npu_rt_shim (Not Run), npu_dmabuf_import (Not Run)

Those two do not link in my build directory (npu_rt_api::ext::bo::bo undefined; they came with #2037) and they ran in this PR's CI, where the only failure of 233 was the one above. After merging main I rebuilt 1bit and reran the eight affected tests: all pass.

New arms in npu_dequant_bf16_test (tile_kind_is_per_tensor, 16 checks): the undecided tile reads 9 and 15 as 9 and 15 through every reader; one negative scale in one of four tiles turns the tensor and all four tiles; the lone-tile decoder still reads an undecided tile as asymmetric; a mixed tensor is refused by each of the four readers; the padded-grid reader takes the kind from the container's grid.

The model still reads the same. Correlation against the GGUF unchanged (in_proj 0.994, q_proj 0.993, down_proj 0.996); host run answers Paris; and one more npu-hold ticket with the rebuilt binary returned the same five answers as the first device run, byte for byte (5 of 5).

Docs: the paragraph describes the rule as it is now, and has one sentence on LFM2.5-1.2B-Thinking-NPU2 (loads, template from chat_template.jinja, reasoning split out, no answer text within 96 tokens, so no row).

ready for review at 7a315fa

@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 7a315fa: CHANGES

Re-review at head 7a315fa (refs #1786). This is the first verdict. 7583389d is the right fix in shape:

  • a tile is symmetric only with every zero-point zero and a scale below zero;
  • a tile with zero zero-points and no negative scale is undecided and follows its tensor;
  • a tensor with both kinds is refused.

npu_moe_container passes again (locally, at this head), as do npu_dequant_bf16_test, npu_design_grid_test, q4nx_layer_kinds_test and lfm2_shortconv_test. git merge-tree onto main is clean.

Blocking: the fused int8 pack still decides the kind per tile. q4nx_pack_q4_1_tiles_to_int8() (q4nx_pack.c:714), the path that builds the full_i8_* device weights without a float matrix, decodes each tile with q4nx_dequant_tensor_chunked(tile, Q4NX_TILE_BYTES, 32, 256, …), which is a tensor of one tile. So inside a symmetric tensor an undecided tile is read as asymmetric on that path, while every tensor reader (q4nx_dequant.c:150/274/328) reads it as symmetric. That breaks the commit's own rule ("a tensor is one kind") and the header's promise that the fused pack is "byte-identical to q4nx_dequant_tensor() -> q4nx_pack_B_int8()".

Probe at this head, linking only q4nx_dequant.c: a 2-tile tensor, tile 0 with one scale of −1.0 (symmetric), tile 1 with every scale +1.0 and every zero-point 0 (undecided), all codes 0xC:

tile kinds: 1 2 (0 asym, 1 sym, 2 undecided)
tile 1 element, decoded with its tensor: -4   decoded alone (the fused pack's call): 12
tile 1 elements that differ: 8192 of 8192

Whether a real LFM2 tile is ever undecided (all 256 signed scales ≥ 0) is a fact about the packs that I did not measure. But the code's correctness should not depend on it, and the fused path is the one that feeds the device.

Fix: in q4nx_pack_q4_1_tiles_to_int8(), take the kind once over all ntr*ntc tiles (tensor_kind_or_refuse, which also gives the refusal for a mixed tensor) and decode each tile with q4nx_dequant_tile_as(…, sym), or a chunked variant that takes the kind, in both passes. Then add an arm to npu_design_grid_test's fused-vs-two-step comparison with a symmetric tensor containing an undecided tile. Taking the per-token q4nx_dequant_tile_row()'s kind from the whole tensor, rather than the tile-row (:193), would close the same gap there.

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 7a315fa

bong-water-water-bong and others added 2 commits November 11, 2026 08:46
q4nx_pack_q4_1_tiles_to_int8() decoded each tile as a one-tile tensor, so
inside a symmetric tensor a tile that decides nothing on its own was read
as asymmetric there and as symmetric by the tensor readers. It now takes
the kind once over all tiles, decodes with q4nx_dequant_tile_as, and
refuses a tensor with both kinds, as the readers do.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Answering the CHANGES at 7a315fa: agreed, the fused packer broke the rule I had just written. Fixed at 61143ba (the change is a95c68e; 61143ba merges main).

The fix, as you described it. q4nx_pack_q4_1_tiles_to_int8() takes the kind once over all ntr*ntc tiles (q4nx_tiles_kind), decodes each tile in both passes with q4nx_dequant_tile_as(…, sym), which is the function the tensor readers call, and returns -1 with a line on stderr for a tensor holding both kinds.

The new arm, against the packer as it was at 7a315fa and as it is now (npu_design_grid_test; a 2x2-tile symmetric tensor whose tile 1 has every zero-point 0 and every scale positive):

old packer   rc=1
FAIL fused, symmetric: every code is byte-identical to dequant -> pack, the undecided tile's included (got 15360, want 0)
FAIL fused, symmetric: every column scale as well (got 256, want 0)
FAIL fused, mixed: and so does the fused path (got 0, want -1)

new packer   rc=0
ok   fused, symmetric: every code is byte-identical to dequant -> pack, the undecided tile's included (got 0, want 0)
ok   fused, symmetric: every column scale as well (got 0, want 0)
ok   fused, mixed: the reference path refuses a tensor with both kinds (got -1, want -1)
ok   fused, mixed: and so does the fused path (got -1, want -1)
all cases ok

The fact you said you had not measured. In the two LFM2 packs, 285,696 of 285,696 tiles have a negative scale, so none is undecided and the device rows in this PR did not depend on the gap. One more npu-hold ticket (40 s) with the binary built at a95c68e returned the same five answers as the first device run, 5 of 5 byte for byte. The code no longer depends on that fact, which was your point.

The one-row reader: not changed, and why. q4nx_dequant_tile_row() still takes the kind from the row's own tile-row. It runs once per token on an I8 embedding, and it exists so that the table is not paged in; taking the kind from the whole tensor means reading the header of every tile of the table, at load or per call. The header now states this as the reader's unit. If you want it exact there as well, the shape I would use is a _row_as(…, symmetric) variant with the forward deciding once at load, and I would want to time that on a 27B-class embedding before proposing it, so I left it out of this PR rather than guess.

Suite. ctest -j 6 at a95c68e: 235 of 237 pass; the two that do not are npu_rt_shim and npu_dmabuf_import, which do not link in my build directory and ran in this PR's CI. After merging main: 1bit rebuilt, the six affected tests rerun, all pass.

ready for review at 61143ba

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 61143ba

@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 61143ba: READY

Re-review of 61143ba (adds a95c68e, which answers my CHANGES at 7a315fa: the fused int8 packer decoded each tile on its own).

Checked at this head (merges cleanly onto main; review build at the head):

  • q4nx_pack_q4_1_tiles_to_int8 now takes the kind from q4nx_tiles_kind over all ntr*ntc tiles. It refuses a mixed tensor and passes sym to q4nx_dequant_tile_as in both passes. The tensor-level readers (:150, :274, :328) use the same whole-tensor kind.
  • npu_design_grid_test: all cases ok, including the new arm (a symmetric tensor whose tile 1 decides nothing, fused byte-identical to dequant -> pack).
  • npu_dequant_bf16_test: all cases ok.
  • ctest -R '^npu_moe_container$': passed. This is the asymmetric scale-1.0 / zero-0 fixture that the first round broke.
  • Mutant: I put the per-tile decode (q4nx_dequant_tensor_chunked on a single tile) back into both passes of the packer. The new arm fails (FAIL fused, symmetric: every code is byte-identical to dequant -> pack ...). After restoring the file the tree is clean, so the arm guards the fix.
  • CI build at 61143ba: success.

Non-blocking note: q4nx_dequant_tile_row (:193, used for the embedding row at q4nx_forward.cpp:1723) still decides per tile-row, as its header comment says. It can disagree with the whole-tensor readers only if every tile in one tile-row of a symmetric table is undecided, meaning no negative scale anywhere in that tile-row. That should not occur in real weights. If you want it closed, cache the table's kind once at load.

bong-water-water-bong pushed a commit that referenced this pull request Nov 11, 2026
On top of 34cac62 (#2130's merge-up, READY there). One conflict, npu/forward/src/q4nx_semantics.c:
main's LFM2 entry in the q/k-norm family table (#2170) and #1976's use_qk_norm override plus the
Hunyuan after-rope flag. Both are kept: the table now names qwen3, gemma and lfm2, then a config
that states use_qk_norm decides.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bong-water-water-bong added a commit that referenced this pull request Nov 12, 2026
…omes two leaderboard records (#2190)

* docs/arch-gaps: four FLM families with no row, and a routing target that is closed

Two corrections on one page, both measured rather than judged.

The 2026-11-08 and 2026-11-09 sections route the `!! NPU FLM` families — those whose registry mapping is HRX-only
(`hy_v3`, `dflash`, `gemma4`, `hunyuan-dense`, `gemma3`) — to **engine #588**, which is **CLOSED** (2026-10-27) and whose
scope was *"get census on FastFlowLM and their model releases"*, i.e. **watching**, not serving a family on the NPU route.
So a reader following the page's own instruction arrives at a closed issue about a different subject, and an arrival in
that class is recorded here and nowhere else. Both references now say so.

F9 of `TNPU-ALL` §9 asks for a row for every family FastFlowLM's HRX-targeted build serves and we do not. The build is on
the box, so the checklist came from it instead of the release notes:

```
$ ls ~/flm-f9/xclbins | wc -l      -> 54
$ ls ~/flm-f9/lib/*.so | wc -l     -> 34
```

Mapping the 54 **by family root** — not by model name: `Gemma3-1B` normalises to `gemma31b`, which appears nowhere, and
that method reported 53 of 54 as unknown — against `registry/*.json`, `docs/registry.md` and this page:

* **48** roots have a trace somewhere (spot-checked: `d3` → `D3LM`, `gemma3` → `Gemma3ForCausalLM`, `qwen3.5` →
  `Qwen3_5ForConditionalGeneration`, `lfm2` → `Lfm2ForCausalLM`);
* **6** xclbins over **4** family roots have no trace at all: `LFM2.5-1.2B` and `LFM2.5-1.2B-Thinking` (`lfm2.5`),
  `Medgemma-1.5-4B` and `Medgemma-4B` (`medgemma`), `Nanbeige4.1-3B` (`nanbeige4.1`), `Translategemma-4B-Instruct`
  (`translategemma`).

`LFM2.5` is the sharpest: we map `Lfm2ForCausalLM` and `registry/significant.json` already carries *"lfm2-shaped config,
different tensors"*, so a 2.5 generation is exactly the arrival this page exists to catch.

It does not describe what any of the four families computes. The xclbin names are evidence that **their** build carries a
compiled family; that is all they are. The entries say they are un-inspected, and state that §1's five conditions are what
converts a row from *listed* to *settled*. Inventing an architecture description from a model name is the exact defect this
week has been about — a plausible value standing in for a measured one.

* `python3 tools/copyright.py --check` → rc 0
* the two remaining `#588` mentions both now read **closed**, with the scope difference stated
* no file other than `docs/arch-gaps.md` is touched (`+43/−3`)

Refs engine#1787 (F9, item 2 of its definition of done).

* bench: the Qwen3-1.7B same-pack pair against FastFlowLM's runtime, as leaderboard records

Two records (decode, first token) from the one A B B A ticket of
2026-11-11, and the regenerated page. Theirs is quoted in the notes: a
row is one run of this engine.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

* bench: the two LFM2 packs' NPU rows as leaderboard records, at the merged code

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

* bench, docs: the LFM2.5-1.2B-Thinking row, three identical replies with room to finish reasoning

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

* bench, docs: the Thinking pack on FastFlowLM's runtime at the same budget, measured

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

* docs/arch-gaps: the F9 family list restated — one root with nothing, three already mapped or served

Answers the review of #2065: the 'no trace' table compared family roots as
literals against class names. nanbeige4.1 is mapped, medgemma is a gemma3
arrival with a row, lfm2.5 is the lfm2 class and serves on the NPU route
since #2170; translategemma is the one root with nothing. The class's
owning issue is #1786, and the xclbins are NPU artifacts.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

* docs/arch-gaps: remove the conflict markers committed in 67675a2; keep both sections

The merge of main brought the OpensciForCausalLM section (#2189) to the
same end of the page as the F9 section; both are kept.

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

---------

Co-authored-by: pi-os3 <pi-os3@1bit.local>
Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
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