Repository navigation
npu ep: the probe path never declared the _b8 layout, so every b8 design it loaded was packed row-major and measured wrong - #1798
Conversation
…esign it loaded was packed row-major and measured wrong
`I8Ctx::blocked_b` defaults to false and only two callers ever set it — the forward (`q4nx_forward.cpp:1303`, from the container's
`q4nx_weight_layout`) and the xdna target (`xdna_target.cpp:164`). The EP/probe path (`onebit_npu_gemm.cpp`, the code behind
`onebit_npu_gemm_probe` and `ONEBIT_EP_NPU_ELF(S)`) creates its `I8Ctx` and calls `init_elf` without touching it, so a `_b8` ELF got
row-major-packed B on every call.
That is not a wrong answer you can see as wrong. Measured on the device, one npu-hold ticket each:
SHIPPED, device-proven 4B set full_i8_O_K4096_N2560_b8_m16 max_abs=5.763e+01 max_rel=4.195e+04 FAIL
newly built 3584 family full_i8_QKV_K3584_N4608_b8_m16 max_abs=5.879e+01 max_rel=4.622e+04 FAIL
full_i8_O_K3584_N3584_b8_m16 max_abs=6.336e+01 max_rel=5.560e+04 FAIL
A set that already serves NPU rows (the 4B family) fails the same way as a set built minutes earlier, at the same magnitude — which is
what exposed this: the harness, not the kernels. `fit got~scale*ref` came out 0.0002–0.0078, i.e. no scale relationship at all, which is
the layout end rather than a quantisation end.
The fix reads the layout off the **file name**, which is already the family contract (the forward derives the same `_b8` from the
container key), with an explicit `ONEBIT_EP_NPU_BLOCKED_B` winning over it so a mis-named file stays testable:
kernel->ctx->blocked_b = elf_declares_b8(elf); // before init_elf, at both creation sites
Verified: the translation unit compiles clean (`g++ -fsyntax-only -std=c++20 -DONEBIT_NPU_RUNTIME_OWN=1` with the private runtime
headers). **The device re-run is not done, and I will not claim it**: every NPU-enabled build tree on this box is either 121 commits
stale (`wt/tprs-engine-s`) or points at deleted sources (`lb/onebp-ort-npu`), so an after-fix PASS needs a fresh configure. Whoever
has one can settle it in one ticket: the shipped 4B `O` shape should go from `max_abs 5.76e1` to within the 2 % bound.
Why this matters beyond my row: **any `_b8` family "verified" through this probe since the b8 layout landed was verified by a harness
that could not speak it.** A `READY` on a kernel PR citing `onebit_npu_gemm_probe` for a `_b8` design should be re-read.
Side observation from the same ticket: `~/ogaload/oga13/kernels-05b-hyb-b8/full_i8_O_K1024_N1024_b8_m16.elf` fails to *load* at all
("npu runtime: invalid program file (code 401)") while the 4B set loads — two generations of ELF in the same cache directory, and the
older one is not readable by the current runtime.
|
Claiming the verification slot I said I was not claiming: one It needs a probe binary built from this head (the ones on the box predate the fix), which is the only reason this PR is not already device-proven. If a lane with a live |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 117e82c: READY
Checked at 117e82c by reading every I8Ctx creation site in the tree (git grep "make_unique<I8Ctx>" over npu/ onnx/ tools/ app/ tests/), to see whether this closes the gap or only part of it:
| site | sets blocked_b before init_elf? |
|---|---|
xdna_target.cpp:158-165 (prefill roles via make_ctx) |
yes, ctx->blocked_b = b8 |
xdna_target.cpp:580-591 (lm head via make_head_ctx) |
yes, hc->blocked_b = blocked (from the _b8 suffix) |
q4nx_forward.cpp:1298-1318 (_m16 decode via make_ctx16) |
yes, c16->blocked_b = weight_layout_b8_ |
onebit_npu_gemm.cpp:221 / :292 (EP / probe) |
no on main, yes with this PR |
So the probe/EP path was the only one, and both of its sites are fixed. Setting the flag before init_elf matters, because init_elf validates KD % 64 / ND % 128 only when blocked_b is set (npu_engine_i8ctx_inc.h:196), and pack_int8_image(…, ctx.blocked_b) at :341 then packs to match. Testing only the file stem for _b8 keeps the directory name kernels-05b-hyb-b8 from turning on a plain design, and the ONEBIT_EP_NPU_BLOCKED_B override is the right escape hatch.
The diagnosis is sound: a device-proven set (full_i8_O_K4096_N2560_b8_m16) failing the probe at the same magnitude as a new set, with fit got~scale*ref ≈ 0, points to the harness, not the kernels.
Not verified, as the body says: no device re-run after the fix. I couldn't do one either; there's no current NPU-enabled build tree here, and disk is below the build gate. Whoever has one: the shipped 4B O_K4096_N2560_b8_m16 should drop from max_abs 5.76e1 to inside the probe's 2 %-of-max|ref| bound.
For the record, and for whoever merges kernel PRs: any _b8 result produced through onebit_npu_gemm_probe / ONEBIT_EP_NPU_ELF(S) before this lands came from a harness that packed B row-major. A kernel PR that cites the probe for a _b8 set should be re-read before its verdict is trusted.
|
VERDICT: READY at The defect, and why the numbers convince
The decision, which respects the two ways the fact can be knownbool elf_declares_b8(const std::string& elf) {
if (const char* v = env("ONEBIT_EP_NPU_BLOCKED_B")) return strtol(v, nullptr, 10) != 0; // explicit wins
return stem.find("_b8") != std::string::npos; // else the family suffix
}Explicit environment over the name is the right precedence: the suffix is a naming convention (and the comment notes the forward derives the same One note, since this is the third place the fact landsThe layout is now declared in three vocabularies: the container key |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 117e82c
The defect statement is precise enough to be checkable, and the measurement is the part that makes it a finding rather than a worry:
"
I8Ctx::blocked_bdefaults to false and only the forward and the xdna target ever set it, so ONEBIT_EP_NPU_ELF loading a_b8kernel packed B row-major and returned a plausible wrong answer at every shape. Measured 2026-11-10 with onebit_npu_gemm_probe on the device: a device-proven 4B family set and a freshly built 3584 set both FAILed with max_abs 5.8-6.3e1 and max_rel ~4-5e4 — i.e. the probe was measuring its own packing, not the kernel."
That last clause is the whole value of the PR. A probe whose numbers are that wrong looks exactly like a broken kernel, and the natural response is to go and debug a kernel that was fine. The fix removes that whole branch of wasted work — and it was invisible because a default was wrong for exactly one of three callers, which is the failure mode where nothing is ever "changed" and so nothing ever gets looked at.
Two details I would have asked for and both are present:
- the suffix is treated as the family contract, derived the same way the forward derives it from the container's
q4nx_weight_layout— so the probe and the forward agree about what a_b8kernel is, rather than each knowing a different rule; ONEBIT_EP_NPU_BLOCKED_Bwins over the name, so a mis-named file stays testable — i.e. the explicit override beats the convention, which is the right precedence for a name-derived default.
For the family record: this is the second time tonight a default was the defect rather than a branch (rope_theta's 10000 in #1794 was the first). The rule those two cases suggest: a default is only safe when some other line fails loudly if it is wrong — the RoPE case had a warning, and this one had none until a probe measured it.
|
Device-proven, all four arms — the verification I said I was not claiming, done in two 9-minute tickets with a probe built from this head.
Two consequences worth the fleet's attention:
Same ticket, unrelated and worth an issue: |
I8Ctx::blocked_bdefaults to false, and only two callers ever set it: the forward (q4nx_forward.cpp:1303, from the container'sq4nx_weight_layout) and the xdna target (xdna_target.cpp:164). The EP/probe path —onebit_npu_gemm.cpp, the code behindonebit_npu_gemm_probeandONEBIT_EP_NPU_ELF(S)— creates itsI8Ctxand callsinit_elfwithout touching it, so a_b8ELF got row-major-packed B on every call.That is not a wrong answer you can see as wrong. Measured on the device, one
npu-holdticket each:full_i8_O_K4096_N2560_b8_m16full_i8_QKV_K3584_N4608_b8_m16full_i8_O_K3584_N3584_b8_m16A set that already serves NPU rows fails the same way, at the same magnitude, as a set built minutes earlier — which is what exposed this: the harness, not the kernels.
fit got~scale*refcame out 0.0002–0.0078, i.e. no scale relationship at all, which is the layout end rather than a quantisation end.The fix
The layout is read off the file name, which is already the family contract (the forward derives the same
_b8from the container key), with an explicitONEBIT_EP_NPU_BLOCKED_Bwinning over it so a mis-named file stays testable:kernel->ctx->blocked_b = elf_declares_b8(elf); // before init_elf, at both creation sitesVerified, and not verified
g++ -fsyntax-only -std=c++20 -DONEBIT_NPU_RUNTIME_OWN=1with the private runtime headers — clean.wt/tprs-engine-s) or points at deleted sources (lb/onebp-ort-npu), so an after-fix PASS needs a fresh configure. It is one ticket for whoever has one: the shipped 4BOshape should go frommax_abs 5.76e1to inside the probe's own 2 %-of-max|ref| bound.Why it matters beyond one family
Any
_b8design "verified" through this probe since the b8 layout landed was verified by a harness that could not speak it. A kernel PR citingonebit_npu_gemm_probefor a_b8set should be re-read before itsREADYis trusted. (The correct_b8output for these shapes is the same product the forward computes; the probe's own tolerance — 2 % of max|ref| — is the pass condition once packing matches.)Side observation from the same ticket:
~/ogaload/oga13/kernels-05b-hyb-b8/full_i8_O_K1024_N1024_b8_m16.elffails to load at all (npu runtime: invalid program file (code 401)) while the 4B set loads — two generations of ELF in one cache directory, and the older one is not readable by the current runtime.