[bong-water-water-bong]

Refs #1786 (board row 17, Hy-MT2-1.8B-NPU2, "not started" until today).

FastFlowLM/Hy-MT2-1.8B-NPU2 could not be read by anything in the engine: its projections are stored 4,608 bytes to a tile, and the readers know 5,120, 4,736 and 8,704. This adds that tile to the forward's decoder. Nothing else was needed: the pack then serves on the per-op NPU route with four kernel shapes that already existed for other packs.

The tile

scales [256] bf16 @[0,512)     index g*32 + r
codes  [256][16]  @[512,4608)  byte c*16 + r/2, even row = low nibble
value[r][c] = (code >= 8 ? code - 16 : code) * scale[g][r]

The size names the kind, so unlike the 5,120-byte symmetric tile (#2170) nothing is inferred from the data.

How the layout was settled: the pack ties its embeddings, embed_tokens is bf16 and lm_head is its quantised copy, so each candidate reading was scored against the embedding over the first 1024 rows, in units of the scale.

scales group codes region   twos     corr  0.00961  |d|/|s| mean 3.899 max 16.000
scales group codes region   minus8   corr  0.01001  |d|/|s| mean 5.629 max 16.000
scales group codes region   unsigned corr -0.06038  |d|/|s| mean 7.921 max 23.000
scales group codes colmajor twos     corr  0.99611  |d|/|s| mean 0.244 max 1.000      <- this one
scales group codes colmajor minus8   corr -0.50807  |d|/|s| mean 7.975 max 9.000
scales group codes colmajor unsigned corr -0.39325  |d|/|s| mean 7.395 max 16.500
(the six with row-major scales: |corr| < 0.01)

Mean 0.244 and maximum 1.000 is what a rounding quantiser leaves (0.25, with the +8 clip), and it is the same figure the LFM2 packs gave.

Before and after

On the first four tiles of the pack's layer-0 q_proj, main's decoder and this branch's:

main:   q4nx_dequant_tensor_chunked(4 tiles of 4608 B) rc=-1   q4nx_tile_row_kind=0
branch: q4nx_dequant_tensor_chunked(4 tiles of 4608 B) rc=0   q4nx_tile_row_kind=4608   first weights -0.01099 0.00732 -0.00366 0.00366

Served, on this box, 2026-11-11

Host float GEMMs (no device), a translation request: Das Wetter ist heute schön. Wir werden zum Sp (12 tokens).

One npu-hold ticket, FastFlowLM 1.0.12's runtime then ours, same model.q4nx (sha256 a992683e04c2110e… both), five greedy translation requests:

p0 DIFF  theirs 'Das Wetter ist heute schön, und wir gehen spazieren.'
         ours   'Das Wetter ist heute schön. Wir werden zum Spazierengehen gehen.'
p1 SAME  'Où se trouve la gare de train la plus proche ?'
p2 DIFF  theirs 'Quisiera una taza de café con leche, por favor.'       ours 'Me gustaría una taza de café con leche, por favor.'
p3 DIFF  theirs '会议已改至下周二下午三点举行。'                              ours '会议已改期至下周二下午三点。'
p4 DIFF  theirs 'The train to Berlin leaves from track seven at half nine.'   ours '…leaves at half nine from track seven.'
identical answers: 1 of 5        decode: ours 2.1-2.8 tok/s, theirs 50-53

All ten are correct translations. Ours on the array is the wording the host float run gives (p0), so the device agrees with float arithmetic on these weights, as it did for LFM2. I have no reference for this model that is independent of both runtimes (no GGUF on the box), so I am not claiming whose wording is the model's, and the docs row says that. If there is a remaining difference in how this family is modelled (its rope_scaling is dynamic), this evidence would not show it; that is the open item.

A second ticket (42 s): the known-answer prompt three times, Paris three times, identical; decode 2.05 and 2.00 tok/s, first token 0.888 and 0.976 s on a 13-token prompt.

Tests

• npu_dequant_bf16_test: q4s_chunk (8 checks: every element against the layout indexed from its description, the three layouts it is not each shown to give a different table, the size told apart from 5,120, one byte short refused), and the existing row-equals-whole arm now runs on four kinds.
• ctest -j 6: 237 of 239 pass; the two that do not are npu_rt_shim and npu_dmabuf_import, which do not link in my build directory (they did not in npu: LFM2.5-1.2B answers on the forward route — conv layers run, the symmetric tile kind is read, q/k norm on #2170 either, and passed in its CI).
• tools/copyright.py --check rc 0.

Not in this PR

• The padded-grid search (q4nx_packed_grid) does not try 4,608: adding a fourth size there changes which grids tie for existing padded containers, and this pack's grids are exact. A padded 4,608 container would be refused by name as today.
• The fast lane's reader (npu/q4nx.cpp) and the importer do not know this tile.
• Leaderboard records for this row: bench: the Qwen3-1.7B same-pack pair against FastFlowLM's runtime becomes two leaderboard records #2190 regenerates the same page, so they follow it.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at 3d183af

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 0911320)

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 The model source is the Q4NX container, and the kernels come from our own runtime 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: None Requires further human verification: None

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing Implementation for Q4S Chunk The PR introduces a new chunk type Q4NX_CHUNK_Q4S with a size of 4608 bytes, but the definition is added to the header file without a corresponding implementation in the source file. This could lead to a compilation error or undefined behavior if the header is included without the implementation. #define Q4NX_CHUNK_Q4S 4608 Missing Implementation for Q4S Chunk The PR adds a new chunk type Q4NX_CHUNK_Q4S to the header file but does not include the corresponding implementation in the source file. This will cause a compilation error when the new chunk type is used. void q4nx_dequant_tile_q4s(const uint8_t* t, float* out, int ld) { const uint8_t* sc_b = t; /* [256] bf16, index g*32 + r */ const uint8_t* qs = t + 512; /* [256][16], byte c*16 + r/2 */ for (int r = 0; r < Q4NX_TILE_ROWS; r++) { for (int c = 0; c < Q4NX_TILE_COLS; c++) { const int g = c / Q4NX_GROUP_SIZE; const uint8_t byte = qs[c * 16 + r / 2]; int code = (r % 2 == 0) ? (byte & 0xF) : (byte >> 4); if (code >= 8) code -= 16; /* two's complement nibble */ out[(size_t)r * ld + c] = (float)code * bf16_to_f32(le16(sc_b + 2 * (g * Q4NX_TILE_ROWS + r))); } } }

✅ Resolved findings

npu/forward/include/q4nx_dequant.h:101

Missing Q4S Chunk Definition

The PR introduces a new chunk type Q4NX_CHUNK_Q4S with a size of 4608 bytes, but the definition is added to the header file without a corresponding implementation in the source file. This could lead to a compilation error or undefined behavior if the header is included without the implementation.

[bong-water-water-bong]

VERDICT: READY 3d183af

A new packed tile is exactly where a decoder can be plausible and wrong, so I checked the arithmetic against the layout, both ways round.

The tile accounts for itself to the byte. With Q4NX_TILE_ROWS 32 and Q4NX_TILE_COLS 256 (q4nx_dequant.h:49-51): scales are 8 groups x 32 rows x 2 B = 512 B, and the largest code index is 255*16 + 15 = 4095, so codes occupy [512, 4608) and fill the 4,608-byte chunk exactly. Nothing is left over and nothing is read past the end — the size genuinely names the kind, as the header comment claims.

Decoder and fixture agree on all four independent choices: byte c*16 + r/2, even row from the low nibble, code >= 8 -> code - 16 (two's complement), and scale index g*32 + r where g = c/32. The decoder spells it as g * Q4NX_TILE_ROWS + r and the fixture as g*32 + r, which are equal only because Q4NX_TILE_ROWS == Q4NX_GROUP_SIZE == 32 — and that is the one thing I would guard.

The suggestion (non-blocking, and it is the only substantive one I have): g * Q4NX_TILE_ROWS + r tiles the 256 scales correctly iff Q4NX_TILE_ROWS == Q4NX_GROUP_SIZE. If GROUP_SIZE were ever raised to 64 while TILE_ROWS stayed 32, the index would reach 3*32+31 = 127 for g=3... and for c >= 224, g = 3 with a different meaning — the point is the pair is coupled by an invariant nothing states. A static_assert(Q4NX_TILE_ROWS == Q4NX_GROUP_SIZE, "the Q4S scale index g*rows + r assumes they are equal") next to the decoder turns a silent misread into a compile error, which is cheap insurance for a format whose failure mode is "answers slightly wrong", not "crashes".

The test is the strong part, and worth naming because three things make it a real oracle rather than a mirror: the expected values are computed from the description (Q4S_U/Q4S_SC macros index the raw bytes, not the decoder); the comparison is bit-exact (got != (float)sgn * scale); and the three layouts it is not — the 5,120 tile's code order, row-major scales, offset-binary codes — are each shown to produce a different table, so the fixture can tell them apart. It is also run across 4 kinds x 64 rows plus 5 refusal arms in the existing npu_dequant_bf16_test, so this is covered in CI rather than in a one-off probe. The known-answer check (Paris three times) and 2.0 tok/s decode are consistent with the board row, and "the size names the kind" removes the data-sniffing #2170 had to do.

One small thing: the fixture's literal 32s are deliberate — an oracle should not import the implementation's constants — and that is right, but a one-line comment saying so would stop a future reader from "fixing" the fixture to use Q4NX_TILE_ROWS and quietly making the test tautological.

[bong-water-water-bong]

Head moved to 0911320 (docs only, plus a merge of main), before any verdict. I found a hole in my own row and measured it rather than leave it for a reviewer.

The PR body says "if there is a remaining difference in how this family is modelled (its rope_scaling is dynamic), this evidence would not show it". There is one, and it was not hard to find: main runs every hunyuan_v1_dense pack at RoPE base 10000, while the family's rope_scaling.alpha of 1000 makes the base 10000 * 1000^(128/126) = 11158839.9 at every position (our own fork's conversion/hunyuan.py:99 computes exactly that), and it normalises q/k before RoPE where the family does it after. Both are engine#1889 and are fixed in the open #1976, which is another lane's; I have not copied them here.

So I merged #1976's head (a43aea3) into a local scratch branch on top of this one (not pushed) and reran the same paired ticket:

                                          byte-identical to FastFlowLM's answer
this PR alone (the tile reader on main)   1 of 5
this PR + #1976 merged locally            4 of 5

p0  theirs 'Das Wetter ist heute schön, und wir gehen spazieren.'
    before 'Das Wetter ist heute schön. Wir werden zum Spazierengehen gehen.'    now SAME
p3  theirs '会议已改至下周二下午三点举行。'      before '会议已改期至下周二下午三点。'        now SAME
p4  theirs '…leaves from track seven at half nine.'   before '…at half nine from track seven.'   now SAME
p2  theirs 'Quisiera una taza de café con leche, por favor.'   ours 'Me gustaría una taza…' both times

What this means for this PR:

• The tile reader is still correct and still needed: it is the tie test against the pack's own bf16 embedding, which does not involve RoPE, and without it nothing loads.
• The row it produced on main is a pack that serves, not yet the model as defined. All five answers were correct translations on the wrong base, so fluency was no evidence. The docs now say this, with the table above; the claim that "the array agrees with float arithmetic, so the row does not say whose wording is the model's" is gone, because the answer turned out to be: theirs was, and ours was a different model.
• The three-identical-answers and decode figures in the body were taken on the tile reader alone.

This PR does not depend on #1976 to be correct as far as it goes; the row depends on #1976 to be a parity row. They touch different files except docs/npu.md.

ready for review at 0911320

[github-actions]

Persistent review updated to latest commit 0911320

[bong-water-water-bong]

VERDICT at 0911320: READY

It merges cleanly onto main.

The layout, re-derived independently. I wrote a numpy decoder from the header comment alone (not from the C) and ran the tie test on the pack on this box (~/.cache/scratch-cl-flm/Hy-MT2-1.8B-NPU2/model.q4nx): the first 1024 rows of lm_head.weight (I8 [30208, 4608]) against model.embed_tokens.weight (BF16 [120818, 2048]):

codes	sign	corr	|d|/|s| mean	max
column-major	two's complement	+0.99611	0.244	1.000
column-major	minus 8	-0.50807	7.975	9.000
column-major	unsigned	-0.39325	7.395	16.500
row-major	two's / minus 8 / unsigned	<= 0.014	3.9 to 7.9	16 to 23

This is the body's table, to the digit, from a second implementation.

Tests, from an ONEBIT_NPU=ON ONEBIT_NPU_HOST=ON build at the head:

• npu_dequant_bf16_test: all cases ok, including the 8 q4s arms and the Q4S row-equals-whole arm.
• npu_design_grid_test: ok.
• Mutants (one edit each to q4nx_dequant.c, rebuilt, then restored; the tree is clean afterwards and the test passes again):

mutant	result
even row reads the high nibble	rc 1, 2 FAIL
scale index r*8 + g (row-major scales)	rc 1, 3 FAIL
4608 dropped from q4nx_tile_row_kind	rc 1, 3 FAIL

Code:

• 4,608 is a safe new size. Every reader that takes a chunk size accepts 4,608 in the same place it accepts 4,736 and 8,704. dequant_any infers the size as nbytes / tiles, and a padded grid can only make that larger, so no 5,120, 4,736 or 8,704 container can come out as 4,608.
• The fused int8 packer cannot misread these tiles. q4nx_pack_q4_1_tiles_to_int8 has no caller in the forward, so a 4,608 tensor reaches the device only through this tested dequant.

CI: build at 0911320 failed in Tools for checkout and the build at 15:31Z. That is the runner DNS outage, not this tree.

Non-blocking: the served comparison is 1 of 5 identical to FastFlowLM's wording, and all ten are correct translations. As the body says, that settles the reader, not the modelling (rope_scaling: dynamic is the open item). An independent reference (a GGUF or the HF checkpoint) is what would settle it.

[bong-water-water-bong]

VERDICT: READY at 0911320 — fresh configure rc 0, built and ran both suites: npu_pack_import_test all pass, npu_dequant_bf16_test 96 arms, all ok, including the two that matter for adding a tile size:

ok   a tile-sized q4_1 grid still decodes
ok   the tile path yields the packed codes (1.0 / 3.0), so bf16 did not hijack it
ok   a tile-plus-2-byte buffer is refused        ok   a short buffer is refused
ok   chunked row: a row that is not a tile multiple is refused (a last partial row has no tile)

The 4,608-byte chunk is the fifth tile size this reader knows, and the arms prove it was added without disturbing the others — that second arm is the one I would have asked for by hand: a tile-sized buffer must still be read as a q4_1 grid, not silently re-interpreted as bf16 because the new branch matched first. Adding a size to a decoder whose branches are distinguished by length is exactly where a wrong-order if turns one format into another; the arm pins the ordering.

Two properties of the fix worth naming as right: the pack needed no new kernels (its four shapes already existed for other packs, so the row closes on the reader alone), and the refusal paths are unchanged (a tile-plus-2-byte buffer is refused, a short buffer is refused) — a length-based decoder that gains a branch must not gain tolerance for off-by-two buffers.

Two asks:

1. A negative arm at the neighbouring tile sizes. The suite proves 4,608 does not hijack the q4_1 grid, but the interesting regression after adding a length is a buffer of 5,120 or 4,736 being mis-classified into the new branch. One assertion that each of the five known sizes still lands in its own decoder would make the matrix explicit rather than implied by the existing cases.
2. The tile's layout is described in the body (scales [256] bf16 at [0,512), index g*32 + r) but not in a comment at the branch. For a fifth size that is exactly the documentation a reader needs — the header's own note should say, for each known tile, its size and index formula, so the sixth size is added by pattern.

Scope of my check: fresh configure rc 0, both suites built in a scratch dir and run (96 bf16 arms + the importer's full set). No device: the pack serving on the per-op NPU route is the PR's own ticket.

[bong-water-water-bong]

VERDICT: READY at 0911320 — the fix is right and, after it, the set is closed, which is the thing to say out loud.

The bug is the reader being narrower than the artifact, in the smallest possible form

npu/forward/include/q4nx_dequant.h:91:#define Q4NX_CHUNK_Q4K 4736
npu/forward/include/q4nx_dequant.h:92:#define Q4NX_CHUNK_Q8  8704
npu/forward/include/q4nx_dequant.h:101:#define Q4NX_CHUNK_Q4S 4608

Three chunk constants exist in the tree, and the two predicates listed two of them:

-    if (cb == Q4NX_TILE_BYTES || cb == Q4NX_CHUNK_Q4K || cb == Q4NX_CHUNK_Q8)
+    if (cb == Q4NX_TILE_BYTES || cb == Q4NX_CHUNK_Q4K || cb == Q4NX_CHUNK_Q8 || cb == Q4NX_CHUNK_Q4S)

The constant had a definition, a docstring line and a value since it was introduced; what it did not have was a mention in the two places that decide "is this byte count a chunk". So the pack was readable by nothing and the failure looked like a format problem, not a classification problem — exactly the class of defect that costs a day going the wrong way.

After this, the predicate set is exhaustive — say so

With Q4S added, both predicates list all three constants the header defines. That is a stronger statement than "Hy-MT2 now imports" and it belongs in the body: it tells the next reader that a fourth chunk size is the only way to hit this again. Which leads to the one ask:

One helper instead of two lists

Both sites — q4nx_forward.cpp and npu/model.cpp — spell the same comparison out. That duplication is why one site could be updated and the other not, and it is the mechanism that will fail again at the fourth constant. A single q4nx_is_chunk(int64_t) next to the definitions makes it one edit, and a static_assert (or a switch over the constants, which -Wswitch checks) makes forgetting a new one a compile error rather than a silent false. If that is out of scope here, one sentence in the body is enough: "a fourth chunk constant must be added in both places; see #NNNN".

And the test earns its 63 lines

tests/npu_dequant_bf16_test.c growing by that much for a two-line fix is the right ratio: the assertion that matters is that a 4,608-byte block dequantises to the same values as the reference, not that the import no longer errors. State the reference in one line (which values, computed how) and the arm is reusable for the fifth format.

[bong-water-water-bong]

Folded into #2218 at adb81c4 (the lane cap of two open PRs, orchestrator 16:45Z): this branch is merged there unchanged, and the row is now measured at one commit with the family's RoPE base (4 of 5 against FastFlowLM's runtime). Closing this one.