[bong-water-water-bong]

Root cause

npu/pack_import.cpp's decode_tile() read the zero-point nibbles of a 2,176-byte subvolume as rows i and i+16, while the codes of that same subvolume pair rows 2i and 2i+1. The shipped pack's bytes say even/odd. encode_tile() mirrored the halves reading, which is exactly why nothing caught it: encode(decode(t)) == t is invariant under any permutation applied on both sides, and the committed goldens were generated by this same decoder.

Measured before/after

A correctly paired zero point sits where its 128 codes cluster, so per (row, K block) mean(code) must track zp[row][block]. Reproducer: tile 2 of layer 0 taken straight out of the pack's dd_metastate_Llm_Token_Token_*.fconst, decoded by the real decode_tile().

pairing	corr(zp, mean code)	mean |mean code − zp|
halves (main before this PR)	−0.0227	2.5007
even/odd (this PR)	+0.9977	0.1450

w = (code − zp) * scale, so every container imported since the attention payload landed (#829) carried another row's zero point in half of its rows, for q/k/v/o — a constant offset of Δzp × scale on those rows. Wrong numbers, not a crash, and every byte-level gate so far passed with it.

The fix

The two 2-line blocks (decode_tile, encode_tile), plus what the class needs to stay fixed:

• tools/oga_attn_tile_golden.py carried the same halves reading — it is the "independent" numpy implementation the fixture is checked against, so it could not catch this either. Re-paired, and tests/golden/oga_attn_tile_q4_1.bin regenerated from the pack. The raw fixture oga_attn_tile.bin is unchanged: it is the vendor's bytes.
• two assertions in tests/npu_pack_import_test.cpp, because the round trip cannot see this class: (a) a structural check on a tile whose zero nibbles are all distinct (byte i must give rows 2i/2i+1), and (b) the code-cluster statistic on the shipped tile (corr > 0.9). Negative control: with only the pairing reverted, the test fails on both new assertions and the container golden (3 check(s) failed); with the fix, all npu-pack checks passed.
• a note in TileW's header comment, so the MLP decoder (npu-pack: import the .fconst MLP section (gate|up fused, then down_proj) #842) does not repeat it.

Verification

Fresh Release/ninja/gcc-16 configure, ONEBIT_OGA_PACK set to the real 0.5B pack:
./npu_pack_import_test tests/golden <pack> → all npu-pack checks passed (golden tile decode/encode exact, both new assertions, container tiles identical, crafted-pack refusals, real-pack import). python3 tools/copyright.py --check → rc 0. tools/oga_attn_tile_golden.py <pack> --check → golden matches the pack.

Sequencing (from the issue)

#842 (open) generates tests/golden/oga_mlp_gu*.bin with the old decoder and has the same halves reading in its 4,288-byte path — it needs the same two lines and its golden regenerated once this lands.

Closes #849

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 5acd50c)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 829 - Partially compliant Compliant requirements: Import the .fconst attention payload (q/k/v/o + their biases) Decode and verify the attention payload byte-by-byte Ensure the tile layout matches FACTS §7.13 and §7.15 The importer successfully imports attention projections and their biases The engine's own Model can open the resulting container Non-compliant requirements: The importer refuses MLP projections and final norm by name (this is a separate ticket) Requires further human verification: Verification that the engine's own Model can open the resulting container (requires hardware testing) 842 - Partially compliant Compliant requirements: Import the .fconst MLP section (gate|up fused, then down_proj) Decode and verify the MLP section byte-by-byte Ensure the layout matches FACTS §7.17 The importer refuses the final norm by name The engine's own Model can open the resulting container Non-compliant requirements: None Requires further human verification: Verification that the engine's own Model can open the resulting container (requires hardware testing) 124 - Partially compliant Compliant requirements: None Non-compliant requirements: Restructure the output pass in reduce_completed.multipass to be O(1) per output element instead of O(blocks) Close the decode gap above 2048 tokens Ensure decode performance recovers toward the <=2048 tok/s corridor Requires further human verification: Verification of decode performance improvements on hardware (requires benchmarking)

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Zero-point pairing inconsistency The zero-point nibbles in decode_tile were previously paired with rows i and i+16, which was inconsistent with how codes were paired (rows 2i and 2i+1). This inconsistency has been corrected to pair zero points with rows 2i and 2i+1 to match the code pairing, which is critical for correct decoding and encoding of attention tiles. The change ensures that a correctly paired zero point sits where its 128 codes cluster, improving correlation from -0.0227 to +0.9977. for (int i = 0; i < 16; ++i) { const uint8_t z = s[kZerosOff + i]; t.zp[size_t(r0 + 2 * i) * size_t(kbs) + size_t(kb)] = z & 0x0F; t.zp[size_t(r0 + 2 * i + 1) * size_t(kbs) + size_t(kb)] = z >> 4; } Zero-point pairing update The Python script oga_attn_tile_golden.py was updated to reflect the correct pairing of zero-point nibbles with rows 2i and 2i+1, matching the C++ implementation. This ensures that the golden test fixture correctly validates the pairing and prevents future regressions. # the zero-point nibbles pair with rows even/odd, exactly as the codes do -- a correct zero # point sits where its 128 codes cluster, and only this pairing satisfies that on the shipped # tiles (engine#849). Pairing them in halves puts another row's zero point in half the rows. zp[r0:r0 + 32, kb] = np.stack([z & 0x0F, z >> 4], axis=-1).reshape(32) scale[r0:r0 + 32, kb] = (np.frombuffer(s[2112:2176].tobytes(), dtype=np.uint16).astype(np.uint32) << 16).view(np.float32)

[bong-water-water-bong]

VERDICT: READY

The fix is the one #849 asked for, and I verified it in both directions on this box rather than reading the table.

Positive arm — fresh Release/ninja/gcc-16 configure at this head, test built from it, real OGA pack
(~/models/oga/Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K) passed as the pack argument, so the real-pack leg runs (it is the
one that prints nothing on success):

attention golden: 1 tile, encode/decode exact, 8 container tiles identical
all npu-pack checks passed            (rc 0)
tools/oga_attn_tile_golden.py <pack> --check   ->  golden matches the pack

Negative control, reproduced exactly as the body claims — I reverted only the two pairing blocks
(decode_tile and encode_tile) to main's text (r0 + i / r0 + 16 + i) and rebuilt:

FAIL attention tile: zero-point nibbles pair with rows 2i / 2i+1 (even/odd)
FAIL attention tile: the zero points sit under their own codes (corr -0.0227, want > 0.9)
FAIL attention golden: container tiles byte-identical to the recorded ones
  first tile difference at byte 514: golden 08, ours 1c
3 check(s) failed

Three checks, the same three — and the corr −0.0227 on the old pairing is the same number I measured independently when
I filed #849 (my own script on a different tile gave +0.0229 halves / +0.9970 even-odd), so the statistic and the pairing
are the same quantity in both measurements.

Also confirmed, because these are the two ways a fix like this can look green while being incomplete:

• the golden really was regenerated (the old pairing now fails the container-golden row, which it could not do if the
  file still matched the halves reading), and the raw vendor fixture tests/golden/oga_attn_tile.bin is not in this
  PR's diff — only the _q4_1 file it derives;
• the generator that produced the fixture carried the same bug and is re-paired here, which is the reason the fixture
  could not catch it before: it was generated by the decoder under test.

tools/copyright.py --check rc 0. The sequencing note for #842 is right: its 4,288-byte path has the same two blocks and
its oga_mlp_gu* goldens were generated with the old reading, so it needs both the lines and the regeneration — and #844
sits behind #842, so that pair lands together or not at all.

One niggle for the follow-up rather than this PR: the structural assertion uses a tile whose zero nibbles are all
distinct, which is the right shape — the class only stays dead if that case is regenerated whenever the tile layout
changes, so the generator should keep asserting distinctness of the manufactured tile (it does today).

[github-actions]

Persistent review updated to latest commit 0b9abce

[bong-water-water-bong]

VERDICT: READY

At 0b9abce21 (merge of main into the head I reviewed; both files blob-identical, so the content is the content I verified).
Build success at this head.

Evidence: the fix is the two-line pairing change in decode_tile/encode_tile, verified both ways on this box — positive: the new assertions and the container golden pass, all npu-pack checks passed; negative: reverting only those two blocks reproduces exactly your three failures (pair with rows 2i / 2i+1, corr -0.0227, want > 0.9, attention golden … first tile difference at byte 514: golden 08, ours 1c). The -0.0227 matches the number I measured independently when I filed #849, and the vendor fixture oga_attn_tile.bin is untouched while the _q4_1 golden is regenerated.

[github-actions]

Persistent review updated to latest commit 5acd50c

[bong-water-water-bong]

VERDICT: READY

At 5acd50c8b (merge of main into the head I reviewed; 5 of 5 files blob-identical, so the content is what I verified).
Build success at this head.

Evidence stands: the pairing fix verified both ways — positive arm passes all checks and the _q4_1 golden; reverting only decode_tile/encode_tile reproduces the three claimed failures including corr -0.0227 (matching my own independent measurement when I filed #849), and the vendor fixture is untouched while the derived golden is regenerated.