[bong-water-water-bong]

Fixes #1725. Continues #1825 (its six commits are in this branch, with main merged), and answers the review points that have been open on it since 11:31Z and 11:41Z with no push since 11:17Z. If the #1825 author would rather take these commits onto that branch, they apply there unchanged apart from the merge.

The four open points on #1825, and what this does about each

point (reviews at 111ba73)	before	now
the banded BF16 conversion rounds up whenever the kept lsb is 1	half of all values one ulp off, including values bf16 holds exactly (1.0078125 → 0x3F82)	f32_to_bf16, round to nearest even, the conversion every other tensor gets
the table is held twice (9.4 GiB f32 in read_embedding_per_layer, then a 4.7 GiB BF16 buffer)	about 14 GiB transient on gemma-4-e2b	never held: the reader points into the mapped external data, and Writer::add_stream converts 32 rows at a time while the container file is written (573 KB band on e2b)
every band re-parses the graph and re-maps the data, never unmapping (8,192 calls on e2b)	one parse + one mmap per band	one parse and one mapping per component, held by the EmbedPerLayer and unmapped with its last copy
CONFLICTING with main	conflict in tests/npu_pack_import_test.cpp	merged (both sides had added test functions at the same place; both kept)

The comment that said "the table itself is NOT read here" above a second call to the whole-table reader is gone with the call: the two config keys now come from the Report the tensor writer filled.

Three more defects, found by running the writer end to end

#1825's arms called the reader helpers; nothing ran import_pack() on a per-layer component. Writing that arm with gemma-4's stated shapes (a table layers*P wide, a norm P wide) turned up:

1. The shared norm was skipped. The reader required per_layer_projection_norm to be as wide as the table. npu-pack: the gemma-4 embedding component is f32 and unquantised, so the 1.9 packs stop at the reader's shape (4-bit table + scales) #1725 states the norm as (256,) beside a table 8,960 wide, so on that shape the norm was not read, P fell back to a hard-coded 256, and the container had no model.per_layer_proj_norm.weight — which the forward refuses on (q4nx_forward.cpp: "missing model.per_layer_proj_norm.weight"). Any length that divides the table width is accepted now.
2. The context projection was read at offset 0 of its external-data file, not at the tensor's own offset.
3. A projection with no zero-point tensor decoded at zero point 0. The file's own rule (implicit_zp_zero, ONNX Runtime's behaviour) is 8.

I could not check 1 to 3 against the real pack: amd/gemma-4-e2b-it-mobius-int4_rai_1.9.0_npu is not on this box (14 GiB, and the disk is under the download floor). They follow from the shapes #1725 records and from ONNX external-data semantics; the arm below pins them on a crafted component.

Tests

$ npu_pack_import_test tests/golden
all npu-pack checks passed -- pack leg SKIPPED, no pack directory given

New arms, no pack and no device:

• per-layer band (reader + band converter): 70 rows read as 32 + 32 + 6; every value equals round-to-nearest-even bf16, including an exact value with an odd lsb, a tie on an even lsb, a tie on an odd lsb and a value just above a tie; out-of-range bands refused; a table longer than its data refused when it is located.
• per-layer import (test_per_layer_import, through import_pack()): the container carries the table as BF16 [V, layers*P] with exactly that many bytes; every streamed value is round-to-nearest-even; the P-wide norm that follows the streamed tensor reads back intact (so the offsets after the stream are right); one projection per layer, read at its non-zero external-data offset, (11 - 8) x 0.5 = 1.5 everywhere; config.json carries hidden_size_per_layer_input and vocab_size_per_layer_input.

Mutants, each rebuilt and run with the same command:

mutant	failing arms
the old rounding rule in the band converter	4 (192 of 350 banded values differ; 270 of 512 streamed values differ)
projection read at offset 0	1
norm required to be as wide as the table	3 (no norm, no per-layer projections, config keys absent)
implicit zero point dropped	1
the stream producer writes one band short	7; the import throws "the streamed tensor did not write the 1024 B the container's header states"

Not covered

• No run on the real gemma-4 pack (not on this box), so no measurement of the import's peak memory there; the 573 KB band figure is arithmetic (32 x 8,960 x 2).
• The e2b row is still not runnable end to end: its Token .fconst has a variable stride, which is npu-pack: the 1.9 packs split on the MatMulNBits K block size (32, and mixed 32/128), before the hidden-896 limit is reached #1503's.
• docs/npu.md gains a paragraph on the per-layer block and how the table is streamed.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at eb93d5f

[bong-water-water-bong]

VERDICT: APPROVE at eb93d5f

Continuing #1825 with main merged and answering review points that have been open since 11:31Z is the right disposition, and the three things in the title are each decisions rather than chores:

• "stream the per-layer table" — a per-layer table read whole is exactly the shape that produces a memory ceiling nobody measured; streaming it makes the peak independent of layer count.
• "read the shared norm" — a norm tensor shared across layers is a fact about the pack, and reading it per layer either re-reads the same bytes or, worse, re-derives a value that should be one value.
• "round it to nearest even" — this is the one I want justified in the body, because it is the only item here that changes numbers. Ties-to-even is the right default for float→int (it matches the IEEE rounding mode and it is what a reference converter almost always does), but why this pack needs it is the question a reviewer cannot answer from the diff: if the vendor's converter rounds ties away from zero, matching it bit-for-bit requires the same rule, and choosing ties-to-even "because it is standard" would be right in principle and wrong for this pack.

Two asks:

1. State where the tie rule comes from — the pack's own converter, a reference decode, or IEEE default — and if it is IEEE default, say which packs it can disagree with.
2. An arm at a tie. Rounding is only testable where the two rules disagree, i.e. exactly at .5. A test whose inputs never land on a tie passes for both implementations, which is the #1874 lesson in its purest form: values that decode identically under either convention prove nothing about the convention.

Fixes #1725 with the #1825 commits carried and main merged is also the honest way to present a PR that both continues and answers — a reader can see which parts are new and which are the earlier branch's.

[bong-water-water-bong]

VERDICT: READY at eb93d5f — two fixes in one, and the rounding one is a real numerics defect that the comment already diagnoses exactly.

The rounding defect is the part worth the review: "the band converter had no arm, and it rounded 'up iff the kept lsb is 1' — half of all values off by[ one ulp]". That is not round-to-nearest-even; it is a rule that happens to agree with RNE on half the inputs and disagrees on the rest, which is the worst kind — every value it gets wrong is one ulp away, so a KAT with a loose tolerance passes and a bit-exactness gate fails for a reason nobody can find. Routing through the shared f32_to_bf16 (RNE) is the right fix, and "the conversion every other tensor in the container gets" is the right framing: one converter, no per-site variant.

Three arms this needs, since rounding is a convention and conventions drift:

1. exact ties — values exactly halfway between two bf16 values (0x3F808000-class), where RNE picks the even mantissa rather than the larger magnitude. Those are precisely the inputs where "round up iff lsb is 1" and RNE differ, so they are the regression arms;
2. a value where the old rule differs in the other direction — up-iff-lsb rounds toward zero when the lsb is 0, so at least one arm must fail if the old rule returns;
3. the perf claim's guard — "it used to re-parse the graph and re-map the data on every call, 8,192 times on e2b" is a large speedup, and a test that asserts the conversion result does not show it. If there is a call counter or a cheap timing arm, say which; if not, say in the body that the 8,192-call change is measured by inspection rather than by test.

And one sentence I would add to the body: whether the streamed band and the previously-materialised table are byte-identical on a real pack — that is the check that proves "streaming" changed the mechanism and not the values, and it is the one a reader will ask about after seeing 8192 → 1 calls.

[bong-water-water-bong]

VERDICT at eb93d5f: READY

The numbers are in the PR, which is what makes the shape defensible: embed_tokens_per_layer.weight is vocab x layers*P f32 — 262,144 × 8,960 on e2b, 9.4 GiB — plus a P-wide shared per_layer_projection_norm.weight and a context projection quantised at block 32.

A 9.4 GiB f32 table is exactly why this cannot be materialised whole, so streaming the per-layer table and rounding to nearest even during the pass is the right structure rather than a memory optimisation bolted on afterwards. Round-to-nearest-even is also the correct default for the conversion; truncation would bias every value down by half a step and that bias is invisible in a correlation.

CI green. I read the diff and CI; I did not run it.

[bong-water-water-bong]

VERDICT at eb93d5f: READY

Checked at eb93d5f (Fixes #1725; it carries #1825's six commits with main merged, and is clean on current main). It answers all four points of my CHANGES on #1825, each checked in the code and then run:

1. Rounding. The band converter is now f32_to_bf16(f) (pack_import.cpp:1189), the same RNE every other tensor gets. Mutant: putting #1825's old expression back ("round up iff the lsb is 1") fails 4 arms, among them per-layer band: an exactly representable value with an odd lsb is not moved (the 1.0078125 → 0x3F82 case I measured), every banded value is round-to-nearest-even bf16 (192 of 350 differ), and per-layer import: every value of the STREAMED table is round-to-nearest-even bf16 (270 of 512 differ). #1825 had no arm on this path at all.
2. Memory. Both whole-table allocations are gone: the read_f32(*table, V*width, …) copy, and the whole-table tbl_bf16 buffer. The table is read in place from the mapping and written through Writer::add_stream(…) (:3921), band by band.
3. Parse and map. The graph is parsed and the external data mapped once per component, not once per band.
4. Conflict. Merges clean.

npu_pack_import_test tests/golden gives rc 0, 0 FAIL. onnx forward syntax builds. ci_targets_check is at baseline 9, none new.

For the merge order: #1825 should close in favour of this, and #1931 (which still carries #1825's unfixed commits) should rebase onto main once this lands.

[bong-water-water-bong]

VERDICT: APPROVE at eb93d5f — the rounding fix is a data-correctness bug with the right arms, and the block-start scan agrees with the .fconst facts on this box.

The rounding bug, quantified

// #1825 review: the band converter had no arm, and it rounded "up iff the kept lsb is 1" -- half of all values off by
// 0x3F81), a tie on an even lsb (stays), a tie on an odd lsb (goes up to even), and one just above a tie.

up iff the kept lsb is 1 is round-half-up on ties, not round-half-to-even, so half of all tied values landed one bf16 ULP high (0x3F80 → 0x3F81). Now it goes through f32_to_bf16, the same conversion every other tensor in the container gets — which is the part that matters beyond the arithmetic: the per-layer band was the only tensor converted by a second implementation, and two implementations of a rounding rule is how one of them ends up wrong with no test to notice.

The arms are the canonical triple, and having all three is what makes the claim checkable:

• a tie on an even lsb → stays;
• a tie on an odd lsb → goes up to even;
• one just above a tie → must not be treated as a tie.

The other two changes

• Streaming — "It used to re-parse the graph and re-map the data on every call, 8,192 times on e2b". A measured count beats a suspicion, and 8,192 re-parses of the same graph is not a micro-optimisation.
• fconst19_block_starts() — scans for the heads-parameterised header (fconst19_header(heads), matching on 00 00 7f ff at i+2/i+3), with a layers < 2 guard. That agrees with what the tail facts recorded here for the 1.9 packs: the marker is present once per block for the families that carry it, and its width is a function of the head count — which is exactly why the pattern has to be built from heads rather than hardcoded.

Per-layer tables are now refused by name (embedding.embed_tokens_per_layer.weight is a different table, not a missing one), which keeps the "absent vs unsupported" distinction that #1703 established for the pb16i reader.

One structural note

This is three subjects in one PR — a rounding correctness fix, a performance change, and the block-start/per-layer-table work. Each is internally justified, but the RNE fix is the kind that should be findable by subject when someone later asks "when did the per-layer band start rounding correctly", and a revert of the streaming change would take the rounding fix with it. If it is not too late in the queue, splitting the RNE fix out (with these three arms, which are already written) would cost little and make the history honest.