Repository navigation
npu: a missing kernel whose only sibling is the _b8 variant names the LAYOUT, not just the file (engine#1503) - #1552
Conversation
… LAYOUT, not just the file (engine#1503) A container imported without --weight-layout b8 is row-major, so it asks for full_i8_<role>_K<K>_N<N>.elf while every per-op ELF set on the box carries the _b8 suffix. The failure was 'missing full ELF <path>', which reads as a missing artifact and sends the reader looking for a kernel that was never supposed to exist; the actual fix is an import flag. Measured on a Ryzen AI 1.9 pack (Qwen2.5-0.5B_rai_1.9.0, hidden 896): the same container answers correctly on the NPU once re-imported with --weight-layout b8 and served against the shipped kernels-05b-hyb-b8 set, and refuses with the message above when imported row-major. Both name-producing sites get the hint - the xdna target layer's prefill path, which is where this container fails, and Q4nxNpuForward::missing_kernel, which serves the plan path. Checked by elf_present() on the sibling name alone, so nothing changes except the wording of a refusal that was already happening. Verified: built (ONEBIT_NPU=ON), reproduced the message against a directory holding only the _b8 names, rc unchanged (4).
PR Reviewer Guide 🔍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: READY at 8e57d0dc.
Two files, +35/-3, and both hunks are error paths adding a diagnosis rather than changing behaviour — which is the right scope for this and the
reason it is safe at this stage of the cycle:
q4nx_forward.cpp return "missing full ELF … - but the b8 variant \"X_b8.elf\" is there: this container's weight layout
is row-major (q4nx_weight_layout absent) … Re-import the pack with `npu-pack import --weight-layout b8`"
xdna_target.cpp hint = " - but the b8 variant …" … appended to the existing "missing" message at :151
Checked the four things that would make a message wrong rather than merely unhelpful:
- the suffix arithmetic:
name.substr(0, size() - 4) + "_b8.elf"for a.elfname —size()-4is exactly the.elflength, both hunks agree, and
thesize() > 4guard keeps a file literally named.elfout of it; - the hint is actually used —
xdna_target.cpp:137declares it,:143fills it,:151appends it to the existing "(missing)" message, so no
branch can set it and drop it; - both probes are real helpers (
file_existsin the forward path,elf_presentin the target) — no new filesystem primitive invented for the
message; - the claim matches the container format I already reviewed:
q4nx_weight_layoutabsent means row-major, and--weight-layout b8is the flag
that makes the pack name the_b8family — so "the missing thing is the layout, not the artifact" is true, and the measured evidence on a Ryzen AI
1.9 pack is the right kind of evidence for a message this specific.
The distinction it draws — "re-fetch the artifact" vs "re-import the pack with a flag" — is the one that cost whoever filed #1503 the most time, and it
is now the first sentence they will read.
|
VERDICT: CHANGES — one measured defect in the probe and one structural point, both small. The idea is right and the 1. The
|
Closes nothing by itself — this is the diagnostic half of #1503, and it is the half I hit twice.
The problem it fixes
A container imported without
--weight-layout b8is row-major, so it asks forfull_i8_<role>_K<K>_N<N>.elf, while every per-op ELF set on this box carries the_b8suffix. The failure was:That reads as a missing artifact. It is not: the artifact present in that directory is
full_i8_QKV_K1024_N1536_b8.elf, and the thing that is actually wrong is the container's declared weight layout. I spent an hour on the wrong hypothesis (renamed the ELFs, served, got garbage) before spotting it, and the same message had already been read as "the fconst decode is broken" once in my own notes.What the PR does
When the plain-named ELF is absent and its
_b8sibling is present in the same directory, the refusal says so:Two sites, because both produce this name:
npu/target/xdna/xdna_target.cpp(the prefill path this container fails on) andQ4nxNpuForward::missing_kernel(the plan path). The check iself_present()on the sibling name and nothing else — no behaviour change, only the wording of a refusal that already happened, and the exit code is unchanged (rc=4).How it was verified
-DONEBIT_NPU=ON -DONEBIT_NPU_RT_DIR=…(CPU-heavy build under~/lb/thermal-run.sh);npu-pack import --allow-incomplete, i.e. without--weight-layout b8) into/tmp/impRM;_b8names → the message above,rc=4;--weight-layout b8at import and the container answers on the NPU — "The first person to walk on the Moon was" → " Neil Armstrong, who was the first to walk on the surface of the Moon. The second person to walk on the Moon…" (24 tokens,npu-target=xdna, full ELF). That is the OOB row foramd/Qwen2.5-0.5B-Instruct_rai_1.9.0_npuon the catalogue board.One
npu-holdticket for the positive control, one for the negative arm; both ended in their own refusal/exit, no server left running.What is still open on #1503 (not this PR)
1bit/1051-longctx-partition, not onmain(q4nx_weight_layout,pack_import.cpp);git merge-treebetween that branch andmainreports 0 conflicts, so landing it is the actual unlock for the rest of the 1.9 queue;importshould default to b8 when only a_b8family exists is a policy question for the owner, not a bug — this PR only makes the failure legible.