Skip to content

npu-pack: import the .fconst attention payload (q/k/v/o + their biases) - #829

Merged
bong-water-water-bong merged 3 commits into
mainfrom
1bit/671-oga-attn-payload
Nov 2, 2026
Merged

bong-water-water-bong merged 3 commits into
mainfrom
1bit/671-oga-attn-payload

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

npu-pack: import the .fconst attention payload (q/k/v/o + their biases)

The 96 attention projections of a Ryzen AI OGA NPU pack are now read out of
dd_metastate_Llm_Token_Token_*.fconst instead of being refused. A layer block's
attention payload is a whole number of 64-row tiles of 34,816 B; the layout
(quants / zero points / scales / the writer's bias subvolumes) is the one FACTS
§7.13 established by known-answer write and §7.15 confirmed against shipped
bytes.

The tile -> projection map is measured, not assumed: each tile's bias subvolumes
carry per-row bias values, and on the shipped 0.5B pack they matched the pack's
q/k/v vectors, which identifies the runs as k (2), q (14), v (2), o (14) = the
payload's whole 32 tiles. A pack whose payload is not a whole number of tiles of
its hidden size, or whose q/k/v/o rows do not add up to the measured count, is
refused by name with the numbers, and npu-pack info now reports that
measurement instead of the plan.

Every tile is its own check: before anything is written, each tile is decoded and
re-encoded and the encoder has to reproduce the 34,816 bytes it came from, bias
slots included. 768/768 tiles of the shipped 0.5B pack pass. One of those tiles
is committed as tests/golden/oga_attn_tile.bin (copied verbatim from the pack by
tools/oga_attn_tile_golden.py, with the q4_1 tiles it must produce beside it), so
the same check runs in CI against the vendor writer's bytes with no vendor stack
present.

On Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K: 218 tensors / 25,136 tiles imported,
73 refusals left -- the 24x3 MLP projections, whose section of the .fconst uses a
2,144-byte subvolume stride that no attribute set of the vendor packer emits, and
the final norm.

The q4_1 chunk's bf16 zero point is exact for 48.7 % of the attention projections'
32-column groups (3.3e-3 relative, the same format bound the head already
reported), printed by the tool rather than hidden.

The 96 attention projections of a Ryzen AI OGA NPU pack are now read out of
dd_metastate_Llm_Token_Token_*.fconst instead of being refused. A layer block's
attention payload is a whole number of 64-row tiles of 34,816 B; the layout
(quants / zero points / scales / the writer's bias subvolumes) is the one FACTS
§7.13 established by known-answer write and §7.15 confirmed against shipped
bytes.

The tile -> projection map is measured, not assumed: each tile's bias subvolumes
carry per-row bias values, and on the shipped 0.5B pack they matched the pack's
q/k/v vectors, which identifies the runs as k (2), q (14), v (2), o (14) = the
payload's whole 32 tiles. A pack whose payload is not a whole number of tiles of
its hidden size, or whose q/k/v/o rows do not add up to the measured count, is
refused by name with the numbers, and `npu-pack info` now reports that
measurement instead of the plan.

Every tile is its own check: before anything is written, each tile is decoded and
re-encoded and the encoder has to reproduce the 34,816 bytes it came from, bias
slots included. 768/768 tiles of the shipped 0.5B pack pass. One of those tiles
is committed as tests/golden/oga_attn_tile.bin (copied verbatim from the pack by
tools/oga_attn_tile_golden.py, with the q4_1 tiles it must produce beside it), so
the same check runs in CI against the vendor writer's bytes with no vendor stack
present.

On Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K: 218 tensors / 25,136 tiles imported,
73 refusals left -- the 24x3 MLP projections, whose section of the .fconst uses a
2,144-byte subvolume stride that no attribute set of the vendor packer emits, and
the final norm.

The q4_1 chunk's bf16 zero point is exact for 48.7 % of the attention projections'
32-column groups (3.3e-3 relative, the same format bound the head already
reported), printed by the tool rather than hidden.
… not

The attention checks now hold for any pack the importer reads (every imported tile
survived its round trip, 4 projections per layer when anything was imported) and
assert the 32-tiles/3-biases-per-layer numbers only for the Qwen2.5-0.5B geometry
the tile layout was measured on.
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Preparing review...

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 239015c)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

671 - Partially compliant

Compliant requirements:

  • The PR implements the import of the .fconst attention payload (q/k/v/o + their biases)
  • It parses the pack's quantized weights into the Q4NX container
  • It handles the attention projections from the .fconst file
  • It includes the necessary checks for tile layout and byte-identical re-encoding
  • It provides detailed information about the attention payload in the manifest

Non-compliant requirements:

  • The PR does not include the full ELF generation for the pack model's geometry
  • The PR does not explicitly mention the vendor stack being avoided in the implementation

Requires further human verification:

  • The full ELF generation for the pack model's geometry needs to be verified
  • The vendor stack avoidance needs to be confirmed in the implementation
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Tile Layout Verification

The importer performs a self-check on every attention tile by decoding and re-encoding it to ensure byte-identical results. This is a good safety measure, but the check only verifies the tile's structure against the vendor writer's bytes. If there are subtle differences in how the vendor's packer handles edge cases or specific bit patterns that aren't caught by this check, it could lead to silent correctness issues. The check should be expanded to cover more edge cases or include additional validation steps.

for (int i = 0; i < nt; ++i) {
    const uint8_t* src = at(fc, base + int64_t(sl.first + i) * tile_bytes, tile_bytes);
    tiles.push_back(decode_tile(src, int(H)));
    ++r.attn_tiles_total;
    const std::vector<uint8_t> back = encode_tile(tiles.back());
    if (std::memcmp(back.data(), src, size_t(tile_bytes)) == 0) ++r.attn_tiles_ok;
    else exact = false;
}
const std::string name = "model.layers." + std::to_string(L) + ".self_attn." + sl.proj;
if (!exact) {
    late.push_back(name + ".weight: the .fconst tile does not survive decode->encode, "
                           "so its fields are not understood");
    continue;
}
MLP Projection Refusal

The MLP projections are still refused by name, and the refusal message indicates that the intra-subvolume byte order is not established. While this is a known limitation, it's important to ensure that this refusal is intentional and that there's a clear plan for addressing it in the future. The refusal message should be updated to reflect the current understanding of the MLP layout and any potential workarounds or future plans.

constexpr const char* kRefuseMlp =
    "weights are in dd_metastate_*.fconst, in 4,288-byte subvolumes (64 output rows per column group, "
    "128 rows per unit) carrying one fused gate/up tensor whose rows interleave in 32-row chunks. The unit "
    "size and the interleave are established, the intra-subvolume byte order is not, and the vendor packer "
    "on this box emits only the 2,176-byte instance for K=896, so these tiles cannot be re-encoded and "
    "byte-verified the way the attention tiles are (amd-oss/npu-re docs/FACTS-oga-pack-weight-layout.md "
    "§7.16.4/§7.16.6)";
Attention Bias Handling

The importer handles attention biases by checking if they are all zero and only writing them if they are not. However, there's a specific case where the o_proj bias subvolumes are not empty, but the container has no o_proj bias. This is flagged as a refusal, which is good, but it's important to ensure that this case is handled consistently and that the refusal message is clear about why the bias cannot be imported.

if (std::string(sl.proj) == "o_proj" && any_bias) {
    late.push_back(name + ".weight: o_proj's bias subvolumes are not empty and the "
                           "container has no o_proj bias");
    continue;
}

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 708d106

…ly say

The refusal text claimed a '2,144-byte subvolume stride'; the vendor headers
(ops_common/mladf_matmul_matrix.hpp) give get_in2_size<128,64,128> = 4,288, i.e.
16 subvolumes of 4,288 per 128-row unit, and 2,144 was half of that misread from an
8,576-byte zero run (two all-zero bias subvolumes). matmulbias_weights_gateup.cpp
also shows the MLP section is ONE fused gate/up tensor whose rows interleave in
32-row chunks, so 'gate then up' would be wrong even at the right unit size.

Only the refusal's wording and docs/npu.md change; no behaviour.
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 239015c

@bong-water-water-bong bong-water-water-bong added the tprs-ready TPRS: review verdict READY posted by the standing reviewer; merge gate satisfied label Nov 2, 2026

@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 239015c18 (three commits: import, generic assertions, the MLP-wording correction). Built and run at
this head, not read.

Built and ran it — Release, -G Ninja, gcc-16, a fresh configure with
-DONEBIT_OGA_PACK=/home/bcloud/models/oga/Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K:

ctest -R npu_pack_import          -> 2/2 passed (npu_pack_import 0.00 s, npu_pack_import_oga 3.2 s)
1bit npu-pack import <0.5B pack> -o /var/tmp/tprs829/out2 --allow-incomplete
  imported 218 tensors, 25136 q4_1 tiles
  self-check lm_head vs the pack's fp16 embedding  corr 0.999999  rel_err 0.000700
  controls: nibble-swapped corr -0.0008, no zero-point corr 0.8788 (both must fail)
  container read back through q4nx::dequant_tile  rel_err 3.340e-03  max |diff| 2.441e-04
  attention: 768/768 .fconst tiles survive decode->encode byte for byte; 96 projections + 72 bias tensors
             imported, bf16 zero exact for 48.69% of groups
  fconst: stride 8708288, second header 1115968; 48/48 RMSNorm anchors found
  incomplete: 73 tensors

So every number in the body re-derives: 218 tensors, 25,136 tiles, 73 refusals (= 24 x 3 MLP + the final norm),
768/768 tiles, 96 projections + 72 biases, 48.69 % bf16-exact zeros. The container carries 96 self_attn.*.weight
tensors, 24 q_proj.bias of [896], k_proj/v_proj bias [128] each, and no MLP tensor.

The fixture is the pack's bytes, checked independently of the tool: I read
dd_metastate_Llm_Token_Token_…fconst at 1856 + 2 x 34816 for 34,816 B and compared with
tests/golden/oga_attn_tile.bin — identical; tools/oga_attn_tile_golden.py <pack> --check also says "golden matches
the pack".

The tile check can fail (the only reason a round trip is a gate): I swapped the two nibble halves in
decode_tile() and rebuilt — 5 checks fail, including "encode(decode(tile)) reproduces the pack's bytes exactly"
(first difference at byte 4352), the q4_1 golden tiles, and the real pack's "0/768 tiles". Reverted, rebuilt: 2/2
again. The test's own negative cases (one code nibble, one bf16-ulp scale, one bias slot) are the same idea in CI.

It refuses the packs it cannot read, with the numbers — I ran npu-pack info on the other three pack directories on
this box, and none of them is silently mis-imported:

pack attention_payload
Qwen3-1.7B _npu_4K "attention payload is 4737280 B, not a whole number of 73984-byte tiles", tiles_measured: 0 (281 refusals)
Qwen3-4B _npu_4K "… 8776960 B, not a whole number of 91392-byte tiles", tiles_measured: 0 (361 refusals)
Llama-3.2-1B _npu_4K "layer blocks not located: fewer than 6 RMSNorm epsilon headers in the first GiB" (161 refusals)

The corrected MLP wording (239015c) is the part I can only partly re-derive, and the text is now careful about
exactly that split: I measured the MLP section independently — the 8,576-byte zero runs sit at a 68,608 B period,
three in a row from 1,117,824, and 16 x 4,288 = 68,608; whether the 8,576 B is 2 x 4,288 or 4 x 2,144 is not decidable
from zero runs alone, and the refusal/doc now say it comes from the writer's headers (mladf_matmul_matrix.hpp
get_in2_size<128,64,128>), with the intra-subvolume byte order stated as not established. §7.16.4/§7.16.6 resolve in
amd-oss/npu-re docs/FACTS-oga-pack-weight-layout.md and §7.16.6 carries the quoted header code. Verified at this head:
python3 tools/copyright.py --check rc 0; the only compile warning in the touched TUs is the pre-existing nlohmann
is_trivial one.

CI at this head: build pass 5m30s, CodeQL pass, Analyze (c-cpp|actions|python|rust) pass, hw_gate skipped;
pr_agent was still running when I posted this.

Non-blocking, for whoever reads this next rather than for this PR: the tile→projection map (k 2, q 14, v 2, o 14) is
measured on this pack, and a different pack at hidden 896 with the same tile count would pass the count check and be
mapped by position; the empty-o_proj-bias guard catches one class of mismatch and FACTS §7.16.2/§7.16.5 bound the claim
to this geometry, so nothing here is overclaimed — it is a hardening note for a second pack at the same hidden size.

@bong-water-water-bong
bong-water-water-bong merged commit d0518ed into main Nov 2, 2026
10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/671-oga-attn-payload branch November 2, 2026 16:53
bong-water-water-bong added a commit that referenced this pull request Nov 3, 2026
…he codes (closes #849) (#879)

decode_tile()/encode_tile() read the zero-point nibbles of each 2,176-byte subvolume as
row i / row i+16, while the codes of the same subvolume pair row 2i / 2i+1. The shipped
pack's own bytes say even/odd: a correctly paired zero point sits where its 128 codes
cluster, so per (row, K block) mean(code) must track zp -- +0.9977 under even/odd,
-0.0227 under halves, on tile 2 of layer 0 taken straight out of the pack's .fconst.

Every container imported since the attention payload landed (#829) therefore carried
another row's zero point in half of its rows, for q/k/v/o. w = (code - zp) * scale, so
the affected rows were offset by dzp x scale -- a few int4 steps. Silent: decode ->
encode is invariant under a permutation applied on both sides, and the committed goldens
were generated by this same decoder, so 768/768 attention tiles and 2160/2160 MLP groups
still "passed".

The fix is the two 2-line blocks. What comes with it:
* tools/oga_attn_tile_golden.py carried the same halves reading (it is the independent
  numpy implementation the fixture is checked against, so it did not catch it either);
  re-paired, and tests/golden/oga_attn_tile_q4_1.bin regenerated from the pack. The raw
  tile 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 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). With the pairing reverted, the test fails on both plus the container
  golden; with the fix, all npu-pack checks pass.
* a note in TileW's header comment so the MLP decoder on #842 does not repeat it.

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Review effort 4/5 tprs-ready TPRS: review verdict READY posted by the standing reviewer; merge gate satisfied

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant