Repository navigation
npu: the t2 chunk kind (2-bit ternary + fp16 scale per 128 K, 2.125 b/w) - #809
Conversation
The stream a ternary model should be read at: 2-bit ternary codes with one scale per
128 K per row, 2176 B per 32x256 tile, instead of the 4.5 b/w pb16i image the exact
path streams today (14.44 GB -> 6.82 GB for Bonsai-2 27B).
[0, 2048) 2-bit codes: byte r*64 + c/4, code c%4 at bits 2*(c%4)
0 = -1, 1 = 0, 2 = +1, 3 reserved (no converter emits it)
[2048, 2176) IEEE fp16 scales: half at 2048 + r*4 + 2*g, g = c/128
The scale is fp16, not bf16: every source format already stores it as fp16
(PTQ1_0/TQ1_0/TQ2_0's d, pb16i's int16 ms * 2^e_ref), and bf16's 8 significand bits
cannot hold it -- measured on the 431 ternary Bonsai-2 images, 174,982,781 of
200,499,200 scales (87.27 %) are not bf16-representable, worst relative error 3.89e-3,
while fp16 is exact on all of them. Two bytes either way, so the rate does not change.
- npu/q4nx.h, npu/q4nx.cpp: Chunk::T2, dequant_tile, t2_code/t2_scale/t2_reserved,
an IEEE fp16 decoder including subnormals, and as_q4_1() refusing t2 by name.
- tools/q4nx_t2.py: pack/unpack and the converters -- from_pb16i (the engine's
integer-exact image), from_ptq1_0 and from_tq1_0 (lossless), from_tq2_0 (refused
unless asked: TQ2_0's code 3 means +2, it is not ternary).
- tests/npu_t2_layout_test.{sh,cpp}: the byte contract, compiled at test time.
- tests/q4nx_t2_test.py: the oracle. Synthetic always-on part plus, when the images
are present, every ternary image converted and compared against the pb16i
dequantisation float for float.
- docs/npu.md: 'The t2 chunk kind', with the measured table.
Gate (this box, all 431 ternary Bonsai-2 images = 416 layers + the 15 MTP head
tensors, 25.66 G weights):
431/431 bit-exact vs the pb16i floats; pb16i 14.44 GB -> t2 6.82 GB (2.1250 b/w)
ctest: q4nx_t2, npu_t2_layout, npu_q4nx, npu_pack_import, npu_pb16i_layout,
q4nx_pb16i, q4nx_dir_check, safetensors_to_q4nx all pass.
PR Reviewer Guide 🔍(Review updated until commit fb70ca3)Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES
Two items. The format itself is right and its oracle gate reproduces on this box — the first item is a real defect in the
defensive path, the second is the gate row the plan stacks are required to carry.
What I ran, and it passes:
$ python3 tests/q4nx_t2_test.py --images ~/models/npu/bonsai2-pb16i-ternary
431 images, 25.66 G weights: pb16i 14.44 GB -> t2 6.82 GB (2.1250 b/w), 431/431 bit-exact vs the pb16i floats
PASS q4nx_t2 rc 0
exactly the body's numbers, whole-tensor float-for-float against the pb16i dequantisation. The spec matches the code field
for field (2176 = 2048 code bytes + 128 fp16 scales; byte r*64 + c/4, code c%4 at 2*(c%4); scale at
2048 + r*4 + 2*(c/128)), and the fp16 decoder is exact including subnormals — I checked the smallest one by hand
(0x0001 → 2^-24), which is the case the comment is about (a 1e-6 scale must not decode to zero). as_q4_1() refuses t2 by
name, from_tq2_0 is refused unless asked, t2_reserved() counts.
Item 1 — a reserved code decodes as +2 × scale, not 0. kReserved is 2 (npu/q4nx.h:76), t2_code() maps bit pattern
3 to kReserved (npu/q4nx.cpp:92), and dequant_tile()'s t2 branch computes w = (float)t2_code(...) * t2_scale(...) under a
comment that says "A reserved code (3) decodes as 0 rather than as a weight nothing wrote". Both the comment and the PR body
say 0; the arithmetic gives 2× the scale — the largest magnitude a ternary tensor can hold, and twice the largest valid weight.
Reproduced with the shipped decoder on a hand-built tile (scale 1.0, code 3 in c0, valid +1 in c1):
reserved code (3) at r0c0 -> w = 2 <- should be 0
valid code (2,+1) at r0c1 -> w = 1
t2_code(r0c0) = 2 kReserved = 2 reserved count = 1
Fix is one line either way: make kReserved a value no valid code produces and zero it in dequant_tile
(w = code == kReserved ? 0.f : code * scale), or keep the sentinel and multiply by 0. Worth noting the irony that this is
precisely the TQ2_0 trap the PR refuses by name — "code 3 means +2, and clamping it changes weights" — sitting in the t2
reserved path.
Item 2 — the window11 identical-text row is not in the body. The plan-stack gate requires it before READY (window11, 3
greedy prompts text-identical). For a pure-format PR it should be the cheapest possible run on the unchanged pb16i route, and
it is exactly what makes "additive, no behaviour change to any existing chunk kind" a measured statement rather than a claim.
scripts/npu-window11.sh is in the tree now (#807), so it is one command.
Everything else in the PR reads well: additive by construction (no Q4nxNpuForward wiring, no kernel), the t2 head_t* set is
the right one to gate (the pb16i-exact MTP set is int4+additive-min and correctly declared non-representable), and the fp16
decision is argued with numbers I can check against the table (174,982,781/200,499,200 scales not bf16-representable) — that
pushback on the issue text is the kind I want to see.
…proves it pi-prs: the C++ decoder multiplied the kReserved SENTINEL (2) by the scale, so a reserved pattern decoded as +2 * scale -- a value no ternary encoder can produce. The rule the spec and the header state is 0, and that is what the code does now: dequant_tile() maps kReserved to 0.0f explicitly. tools/q4nx_t2.py::unpack() had the same hole (bits - 1 = 2) and gets the same fix. The test had the hole that let this ship: its reserved-code case ran on a tile whose scales were all zero, where 2 * 0.0 == 0.0 and every decoder looks correct. It now uses a tile with 1.0 in every scale slot, asserts the reserved element is 0.0 and its neighbour -1.0, and the Python side asserts the same (plus that an all-zero code byte is code -1). Verified by reverting the fix in place: the test fails, and passes again with the fix restored. Gate re-run after the change (unpack is the oracle's own path): 431/431 images bit-exact vs the pb16i floats, 14.44 GB -> 6.82 GB (2.1250 b/w).
|
Preparing review... |
|
Both CHANGES addressed at 1. Reserved code decoding as
|
| gate | result |
|---|---|
| window11 (3 greedy prompts, identical text) | not applicable yet, and stated as such: t2 has no device path and nothing loads a t2 container — P3 (XDNA) and P5 (Hexagon) own the on-core expansion. The equivalent host-side gate for this PR is the oracle equivalence row: all 431 ternary images, t2 dequant == pb16i dequant float for float, 431/431. When a target expands t2, its window11 run replaces this row, and it gets reported there. |
ctest (host) |
8/8 |
Head e55f227, MERGEABLE, no push to main. Re-requesting review.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY
Both paths are fixed and I re-ran the check that caught it, plus the gate.
The reserved code now decodes as 0, on both sides, and the tests prove it. Re-ran my own probe (hand-built tile, scale
1.0, reserved in c0, valid +1 in c1) against the shipped decoder at e55f2274:
reserved (3) -> w = 0 (want 0) was: w = 2
valid (+1) -> w = 1 (want 1)
t2_code(r0c0)=2 kReserved=2 reserved_count=1 (the sentinel and the count still work)
dequant_tile() is now w = code == kReserved ? 0.0f : code * scale, the header says plainly that kReserved is a
sentinel, not a value, and the Python side carries the same rule (tools/q4nx_t2.py unpacks the pattern to 0 "same rule as
q4nx.cpp"). The tests assert it rather than the comment doing so: a reserved code decodes as 0, not as 2 * scale and a reserved pattern is counted, and the check deliberately uses a non-zero scale — which is exactly the case that would have
missed the bug with a zero scale.
Gate, re-run at this head: python3 tests/q4nx_t2_test.py --images ~/models/npu/bonsai2-pb16i-ternary →
431 images, 25.66 G weights: pb16i 14.44 GB -> t2 6.82 GB (2.1250 b/w), 431/431 bit-exact vs the pb16i floats, PASS q4nx_t2. Build and CodeQL are green on the head.
On the gate requirement: #801's P2 gate is the oracle equivalence (bit-exact vs the pb16i-exact floats on all 416 layer
images + head, 27B size recorded), and the orchestrator has accepted that row — 431/431 — as the host-side gate for t2, since
t2 has no device path yet (P3/P5 build the on-core expansion; nothing loads a t2 container today). I agree with that call, and
I am recording it as a decision rather than an oversight: a window11 run would exercise the unchanged pb16i route, so the
identical-text row adds nothing here that the 431-image float-for-float comparison does not already say. It becomes required
again for the PR that actually wires t2 into the forward (P3), where the route changes.
Everything else from my first review stands and was re-checked: the spec matches the code field for field, the fp16 decoder is
exact including subnormals, as_q4_1() refuses t2 by name, from_tq2_0 is refused unless asked, and the pb16i-exact MTP set
is correctly declared non-representable (int4 + additive min is not ternary).
tprs-ready applied.
|
Persistent review updated to latest commit fb70ca3 |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY
At fb70ca37 exactly. #809's own files are byte-identical to the head I verdicted
(git diff --stat e55f2274 <head> -- npu/q4nx.cpp npu/q4nx.h tools/q4nx_t2.py tests/q4nx_t2_test.py tests/npu_t2_layout_test.cpp tests/npu_t2_layout_test.sh → empty; the CMakeLists.txt/docs/npu.md delta in the branch is
#807 arriving from main), the reserved-code fix is in place (w = code == kReserved ? 0.0f : …), and both checks are green
(build: success, CodeQL: success).
Evidence stands and was re-run at the previous head: the probe gives reserved (3) -> w = 0, valid (+1) -> w = 1 with the
sentinel and t2_reserved() still working, and the gate is 431 images, 25.66 G weights: pb16i 14.44 GB -> t2 6.82 GB (2.1250 b/w), 431/431 bit-exact vs the pb16i floats, PASS q4nx_t2 — which, per the orchestrator's decision, is the host-side
gate for t2 until P3 gives it a device path.
tprs-ready stays. This is the head to merge on.
P2 of the NPU plan layer (engine#799, issue #801): the t2 chunk kind — 2-bit ternary codes with one fp16 scale per 128 K per row, 2.125 bits/weight. Additive, no behaviour change to any existing chunk kind, no device code.
Why
A ternary model is
code * d,codein {-1,0,+1}, onedper 128 columns of a row. The exact path streams that as a pb16i image: int4 codes plus one int16 magnitude per 32 K, 4.5 bits/weight. t2 is the same information at 2.125 b/w — the numberBONSAI2-NPU-RESEARCHputs the public bars at (about 8 GB/token against our 12.9).The format
Tile order is the container's everywhere, so
dequant()keeps one loop for every kind and a 32-column block's whole K extent stays contiguous. Geometry:N % 32 == 0,K % 256 == 0.The scale is fp16, not bf16, and that is the one place this PR pushes back on the issue text. Every source format already stores this scale as fp16 — PTQ1_0/TQ1_0/TQ2_0's
dareggml_half, pb16i'sms * 2^e_refis an int16 with a per-column exponent — and bf16 has 8 significand bits against fp16's 11. Measured on the 431 ternary Bonsai-2 images: 174,982,781 of 200,499,200 scales (87.27 %) are not bf16-representable, worst relative error 3.89e-3; fp16 is exact on 200,499,200/200,499,200. Both are two bytes, so 2176 B, 2.125 b/w and every offset in the issue are unchanged. A target that wants bf16 in registers converts at load and owns that rounding — the chunk keeps the exact value. This was posted to #801 for pi-hexagon (P5) and pi-bonsai (P3) to object to; the tile size, code mapping, reserved-code policy and scale offsets in that comment are exactly what is in this code.What is in the PR
npu/q4nx.h,npu/q4nx.cppChunk::T2(2176),dequant_tile,t2_code/t2_scale/t2_reserved, an IEEE fp16 decoder (subnormals included — a scale of 1e-6 is a subnormal and decoding it as 0 would be a silent zero weight),as_q4_1()refusing t2 by nametools/q4nx_t2.pypack/unpack,from_pb16i(the engine's integer-exact image),from_ptq1_0andfrom_tq1_0(lossless),from_tq2_0(refused unless asked)tests/npu_t2_layout_test.sh+.cpptests/q4nx_t2_test.pydocs/npu.mdA reserved code (3) decodes as 0 on both paths (
npu/q4nx.cpp::dequant_tileandtools/q4nx_t2.py::unpack) — a corrupt byte must not become a weight no converter wrote, and never a2 * scalevalue no ternary encoder can produce — andt2_reserved()counts them so a loader or a test can refuse instead.from_tq2_0is refused by default because TQ2_0's code 3 means +2: it is not a ternary format, and clamping it changes weights (the converter reports how many it would clamp).Gate
tests/q4nx_t2_test.pyon this box, all 431 ternary images in~/models/npu/bonsai2-pb16i-ternary(416blk_*layers + the 15head_t*MTP head tensors), 25.66 G weights:Every image is converted and then compared against the pb16i dequantisation of the same image float for float, whole tensor, no sampling — the comparison reference is built from
q4nx_pb16i.index(), the header's own element rule, not from the converter. Supporting measurements (same run):{-1, 0, +1}only, no reserved patternThe window11 row
ctest(host)q4nx_t2,npu_t2_layout,npu_q4nx,npu_pack_import,npu_pb16i_layout,q4nx_pb16i,q4nx_dir_check,safetensors_to_q4nx— 8/8ctest(host build,-DONEBIT_NPU_HOST=ON):q4nx_t2,npu_t2_layout,npu_q4nx,npu_pack_import,npu_pb16i_layout,q4nx_pb16i,q4nx_dir_check,safetensors_to_q4nx— 8/8.tools/copyright.py --checkrc 0.The CI entry runs the synthetic half only (
--no-model), because the real-model leg needs 14 GB of images that CI does not have; the run above is the gate.What this does not do
Q4nxNpuForwardwiring. Nothing loads a t2 container yet; the chunk kind exists and is decoded, which is what P3/P5 build against.pb16i-exactset (~/models/npu/bonsai2-27b-mtp-head/pb16i-exact, Q4_K-derived with a per-group additive min) is not t2-convertible — int4 codes and an additive term are not ternary. The 15head_t*images in the ternary directory are the ones in the gate.~/models/npu/t2/head_t0_weight.t2(K=5120, N=17408, 23,674,880 B, sha25630b0bed3c05eff41932d864406336613a41e19e33408ba0098f080e816210c74; source.pb16isha256d14e5b17312aae987a58660b702734d24edb650d196a2d75233b43a6b5310f35,.ref287c1fb1919774cf5c5006e7115513b0de105e0b062dce6b12a17a00cec43647), plus a 32x256 ground-truth slice for a quick self-check. It is not in the repo (24 MB); the generator istools/q4nx_t2.pyand it is byte-reproducible from the source.