Skip to content

npu: a missing kernel whose only sibling is the _b8 variant names the LAYOUT, not just the file (engine#1503) - #1552

Merged
bong-water-water-bong merged 1 commit into
mainfrom
1bit/1503-rm-vs-b8-hint
Nov 8, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
1bit/1503-rm-vs-b8-hint

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

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 b8 is row-major, so it asks for full_i8_<role>_K<K>_N<N>.elf, while every per-op ELF set on this box carries the _b8 suffix. The failure was:

1bit serve: forward init failed: missing full ELF /home/…/kernels-05b-hyb-b8/full_i8_QKV_K1024_N1536.elf

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 _b8 sibling is present in the same directory, the refusal says so:

1bit serve: forward init failed: missing full ELF /tmp/kb8only/full_i8_QKV_K1024_N1536.elf - but the b8 variant
"full_i8_QKV_K1024_N1536_b8.elf" is there: this container's weight layout is row-major (q4nx_weight_layout absent), and a
row-major container asks for row-major kernels. Re-import the pack with `npu-pack import --weight-layout b8`, or supply a
row-major ELF family (engine#1503).

Two sites, because both produce this name: npu/target/xdna/xdna_target.cpp (the prefill path this container fails on) and Q4nxNpuForward::missing_kernel (the plan path). The check is elf_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

  • built with -DONEBIT_NPU=ON -DONEBIT_NPU_RT_DIR=… (CPU-heavy build under ~/lb/thermal-run.sh);
  • imported a real 1.9 pack row-major (npu-pack import --allow-incomplete, i.e. without --weight-layout b8) into /tmp/impRM;
  • served it against a directory holding only the _b8 names → the message above, rc=4;
  • the positive control for the flag itself, same pack, same kernels: --weight-layout b8 at 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 for amd/Qwen2.5-0.5B-Instruct_rai_1.9.0_npu on the catalogue board.

One npu-hold ticket 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)

  • the import-side flag itself lives on 1bit/1051-longctx-partition, not on main (q4nx_weight_layout, pack_import.cpp); git merge-tree between that branch and main reports 0 conflicts, so landing it is the actual unlock for the rest of the 1.9 queue;
  • whether import should default to b8 when only a _b8 family exists is a policy question for the owner, not a bug — this PR only makes the failure legible.

… 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).
@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1503 - Partially compliant

Compliant requirements:

  • Fix the issue where containers imported without --weight-layout b8 are row-major and ask for plain kernel names, while the only kernels on the box carry the _b8 suffix
  • Improve diagnostic messages to indicate when a container's weight layout is incorrect instead of just reporting a missing artifact
  • Ensure that the diagnostic message clearly states that the issue is with the container's declared weight layout, not the artifact itself
  • The fix is applied in both q4nx_forward.cpp and xdna_target.cpp to provide consistent error messages

Non-compliant requirements:

  • None

Requires further human verification:

  • None
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Diagnostic Clarity

The diagnostic message in Q4nxNpuForward::missing_kernel now provides a more helpful hint when a plain ELF is missing but its _b8 variant exists. This helps users understand that the issue is with the container's declared weight layout rather than a missing artifact. The change improves the user experience by guiding them to the correct solution.

// engine#1503: a container imported without `--weight-layout b8` is row-major, so it asks for plain kernel names while
// the only kernels on the box carry the _b8 suffix. Naming the layout instead of only the missing file is the
// difference between "re-fetch the artifact" and "re-import the pack with a flag" - measured on a Ryzen AI 1.9 pack,
// where the same container ran correctly with the b8 set and the flag.
{
    const std::string name(elf_name);
    if (name.size() > 4 && name.compare(name.size() - 4, 4, ".elf") == 0) {
        const std::string b8 = name.substr(0, name.size() - 4) + "_b8.elf";
        if (file_exists(dir + "/" + b8))
            return "missing full ELF " + dir + "/" + elf_name +
                   " - but the b8 variant \"" + b8 + "\" is there: this container's weight layout is row-major " +
                   "(q4nx_weight_layout absent), so it asks for row-major kernels and the directory holds the b8 " +
                   "family. Re-import the pack with `npu-pack import --weight-layout b8`, or supply row-major ELFs " +
                   "(engine#1503).";
    }
}
return "missing full ELF " + dir + "/" + elf_name;
Diagnostic Clarity

The diagnostic message in XdnaTarget::open_prefill now provides a more helpful hint when a plain ELF is missing but its _b8 variant exists. This helps users understand that the issue is with the container's declared weight layout rather than a missing artifact. The change improves the user experience by guiding them to the correct solution.

// engine#1503: when the asked-for name is the plain one and the directory holds only the _b8 family, the missing
// thing is the container's LAYOUT, not the artifact. Measured on a Ryzen AI 1.9 pack: the same container ran
// correctly once it was re-imported with `--weight-layout b8`. Say that here, or the reader goes looking for a
// kernel that was never supposed to exist.
std::string hint;
{
    const std::string nm(g->elf_name);
    if (nm.size() > 4 && nm.compare(nm.size() - 4, 4, ".elf") == 0) {
        const std::string b8 = nm.substr(0, nm.size() - 4) + "_b8.elf";
        if (elf_present(dir + "/" + b8))
            hint = " - but the b8 variant \"" + b8 + "\" is there: this container's weight layout is "
                   "row-major (q4nx_weight_layout absent), and a row-major container asks for row-major "
                   "kernels. Re-import the pack with `npu-pack import --weight-layout b8`, or supply a "
                   "row-major ELF family (engine#1503).";
    }
}
err = (plan_path.empty() ? ("missing full ELF " + elf)
                         : ("plan " + plan_path + ": this model's plan needs the " + role +
                            " kernel \"" + g->elf_name + "\" in " + dir + " (missing)")) + hint;
return false;

@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 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 .elf name — size()-4 is exactly the .elf length, both hunks agree, and
    the size() > 4 guard keeps a file literally named .elf out of it;
  • the hint is actually used — xdna_target.cpp:137 declares it, :143 fills it, :151 appends it to the existing "(missing)" message, so no
    branch can set it and drop it;
  • both probes are real helpers (file_exists in the forward path, elf_present in the target) — no new filesystem primitive invented for the
    message;
  • the claim matches the container format I already reviewed: q4nx_weight_layout absent means row-major, and --weight-layout b8 is the flag
    that makes the pack name the _b8 family — 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.

@bong-water-water-bong
bong-water-water-bong merged commit 4f31a2a into main Nov 8, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/1503-rm-vs-b8-hint branch November 8, 2026 22:33
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: CHANGES — one measured defect in the probe and one structural point, both small. The idea is right and the
message is worth having; it just does not fire in the case a d3/27B user hits second.

1. The _m16 twin case is missed — measured, not stylistic

The repo's blocked-twin convention puts the family tag before the twin suffix. Both of these are on this box:

/home/bcloud/models/npu/bonsai2-27b-pb16i/kernels-m16/full_i8_G_K5120_N17408_b8_m16.elf
/home/bcloud/models/npu/bonsai2-27b-pb16i/kernels-full_i8_G_K5120_N17408_b8.elf

and npu/target/xdna/xdna_target.cpp:543 builds its candidates as stem + "_b8_m16.elf", stem + "_m16.elf",
stem + "_b8.elf", g.elf_name — i.e. …_b8_m16, never …_m16_b8.

The new probe builds name.substr(0, size-4) + "_b8.elf", so for a decode twin
full_i8_QKV_K1024_N5120_m16.elf it looks for full_i8_QKV_K1024_N5120_m16_b8.elf, which exists in no set. Consequence:
the hint fires on the prefill family and silently does not fire on the M=16 twins — so the second failure a user meets
(after the first is explained) falls back to the bare "missing full ELF", which is exactly the message this PR exists to
replace. Fix is to mirror line 543: try stem + "_b8" + <suffix> where <suffix> is _m16.elf or .elf.

2. The probe is duplicated in two files

q4nx_forward.cpp and xdna_target.cpp now each carry the same 6-line "is the b8 sibling there" block, and the PR's own
comment quotes the house rule — "a diagnostic that changes shape when code moves is a diagnostic someone has to re-learn".
Two copies is how they drift apart, and #1380 is already moving ELF-name logic into the target. One helper
(q4nx_elf_sibling_b8(name, dir) next to the candidate list it belongs with) called from both sites would keep this at one
definition — and would make the _m16 fix a one-place change too.

3. No test, and this one is cheap to test

The new code is a pure string/file_exists decision, and the repo has ctests that drive exactly this kind of helper
(npu_design_grid_test, npu_forward_loader_test). Three lines would pin it: plain name + b8 sibling present → hint;
plain name + b8 sibling absent → bare message; twin name + _b8_m16 sibling present → hint (the case above, which today
fails).

What I verified and did not

  • Verified: the sibling-name construction against two real sets on this box and against xdna_target.cpp:543's own candidate
    list; that both sites receive the same g->elf_name/elf_name and the same dir; that the layout claim in the message is
    sound (a plain name is only ever asked for when family_tag() is empty, i.e. row-major, so q4nx_weight_layout absent in
    the message is accurate for this path — _t2s/_pb16i containers ask for their own tagged names and never reach it).
  • Did not: build it or run the failure end to end (that needs a device ticket and a _b8-only kernel directory); file_exists
    in the forward vs elf_present in the target I read as equivalent-in-intent but did not diff their bodies.
  • Nothing here is a device or a build claim: no ticket, no compile, :18097 untouched.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant