Skip to content

npu ep: the probe path never declared the _b8 layout, so every b8 design it loaded was packed row-major and measured wrong - #1798

Merged
bong-water-water-bong merged 1 commit into
mainfrom
fix/npu-probe-b8-layout
Nov 10, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
fix/npu-probe-b8-layout

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

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:

design max_abs max_rel result
shipped, device-proven 4B set full_i8_O_K4096_N2560_b8_m16 5.763e+01 4.195e+04 FAIL
newly built 3584 full_i8_QKV_K3584_N4608_b8_m16 5.879e+01 4.622e+04 FAIL
newly built 3584 full_i8_O_K3584_N3584_b8_m16 6.336e+01 5.560e+04 FAIL

A 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*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

The layout is read 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, and not verified

  • Compiles: g++ -fsyntax-only -std=c++20 -DONEBIT_NPU_RUNTIME_OWN=1 with the private runtime headers — clean.
  • Device re-run: NOT done, and I am not claiming 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. It is one ticket for whoever has one: the shipped 4B O shape should go from max_abs 5.76e1 to inside the probe's own 2 %-of-max|ref| bound.

Why it matters beyond one family

Any _b8 design "verified" through this probe since the b8 layout landed was verified by a harness that could not speak it. A kernel PR citing onebit_npu_gemm_probe for a _b8 set should be re-read before its READY is trusted. (The correct _b8 output 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.elf fails 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.

…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.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Claiming the verification slot I said I was not claiming: one npu-hold ticket settles it, and it is cheap. The check is the shipped 4B O shape, which this PR changes from max_abs 5.76e1 / FAIL to a pass under the probe's own 2 %-of-max|ref| bound:

ONEBIT_EP_NPU_ELF=~/ogaload/oga11/kernels-4b-b8/full_i8_O_K4096_N2560_b8_m16.elf \
ONEBIT_EP_NPU_M=16 ONEBIT_EP_NPU_K=4096 ONEBIT_EP_NPU_N=2560 \
  npu-hold -t 9 -l "b8-probe-gate" -- <build>/ep/onebit_npu_gemm_probe 4096 2560 16 3

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 ONEBIT_EP_NPU=ON build tree picks it up, the arms are: (1) shipped 4B O → PASS, (2) my 3584 QKV/O → PASS, (3) ONEBIT_EP_NPU_BLOCKED_B=0 on the same _b8 file → FAIL again (the negative control, which is what makes the first two mean something).

@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 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.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 117e82c — the probe path was judging _b8 kernels as row-major, and this declares the layout where every other consumer already gets it. The measured symptom is what makes it credible.

The defect, and why the numbers convince

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 packing, not the kernel.

I8Ctx::blocked_b defaults to false and only the forward and the xdna target set it; ONEBIT_EP_NPU_ELF loading a _b8 kernel therefore packed B row-major. A max_rel ~5e4 on a kernel that is known good on the device is the signature of a wrong layout rather than a wrong computation — and the PR says so rather than "the numbers were bad".

The decision, which respects the two ways the fact can be known

bool 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 _b8 from the container's q4nx_weight_layout, so the two agree by construction), while the variable is the only way to test a mis-named file. Same shape as #1793 — the layout is a declared fact, and every consumer should read the same declaration.

One note, since this is the third place the fact lands

The layout is now declared in three vocabularies: the container key q4nx_weight_layout (engine#1793), the ELF suffix _b8, and this env override. They agree today; the risk is a fourth consumer reading a fourth thing. A one-line comment here naming the other two (the container key and where the forward derives it) would let the next reader see the whole set instead of the one in front of them.

@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 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_b defaults to false and only the forward and the xdna target ever set it, so ONEBIT_EP_NPU_ELF loading a _b8 kernel 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:

  1. 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 _b8 kernel is, rather than each knowing a different rule;
  2. ONEBIT_EP_NPU_BLOCKED_B wins 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.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

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.

g++ -std=c++20 -O2 -DONEBIT_EP_NPU=1 -DONEBIT_NPU_RUNTIME_OWN=1 … onebit_npu_gemm_probe.cpp onebit_npu_gemm.cpp q4nx_pack.c q4nx_pack_blocked.cpp q4nx_dequant.c npu_rt.cpp npu_rt_api.cpp -lpthread — the exact source list npu_sources.cmake gives the EP target, so the binary is this PR, not a rebuilt stale tree.

arm design max_abs fit scale ms/call result
A control shipped device-proven 4B O 4096×2560 _b8_m16 2.617e-01 1.0000 25.2 PASS (was 5.763e+01, scale 0.0002)
B mine QKV 3584×4608 _b8_m16 2.353e-01 1.0000 55.4 PASS
C mine O 3584×3584 _b8_m16 2.430e-01 1.0000 56.9 PASS
D negative control same _b8 file, ONEBIT_EP_NPU_BLOCKED_B=0 6.336e+01 0.0002 93.8 FAIL (reproduces the reported signature)
E mine GU 3584×37888 _b8_m16 2.701e-01 1.0000 215.8 PASS
F mine D 18944×3584 _b8_m16 5.381e-01 1.0000 90.9 PASS
G mine QKV 3584×4608 plain tier M=128 2.806e-01 1.0000 42.1 PASS

fit scale = 1.0000 in every passing arm and 0.0002 in the control: that is the number that separates a layout mistake from a quantisation one, and it moves exactly as predicted. max_abs ≤ 0.54 against max|ref| 45–100 is ≤ 0.5 %, inside the probe's own 2 % bound, on 3 repeats per arm.

Two consequences worth the fleet's attention:

  1. This PR turns a red gate green for a device-proven set (arm A), which is the strongest available evidence that the diagnosis was the harness and not the kernels.
  2. Every _b8 design probed before this fix was packed row-major. Any kernel row that cited onebit_npu_gemm_probe for a _b8 set since the b8 layout landed measured the packing, not the kernel — arm D is what that looked like (scale 0.0002, max_abs two orders of magnitude out).

Same ticket, unrelated and worth an issue: ~/ogaload/oga13/kernels-05b-hyb-b8/full_i8_O_K1024_N1024_b8_m16.elf fails to load (npu runtime: invalid program file (code 401)) while ~/ogaload/oga11/kernels-4b-b8 loads fine — two ELF generations in one cache directory, and the runtime refuses the older one. I have not filed it because I do not know which lane owns that cache; it is a one-line check for whoever does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant