Skip to content

npu forward: a pack with short-convolution layers is refused by name at load — LFM2.5 said ready and answered 500 to everything (Refs #1786) - #2158

Merged
bong-water-water-bong merged 1 commit into
mainfrom
cl-flm/lfm2-refuse-by-name
Nov 11, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
cl-flm/lfm2-refuse-by-name

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #1786 (the LFM2.5-1.2B-NPU2 row). Backlog. I searched open issues and PRs for lfm2 shortconv forward before writing this; nothing open covers it.

The fault, on the NPU

FastFlowLM's LFM2.5-1.2B-NPU2 (model_type lfm2: sixteen layers, ten of them gated short convolutions, six attention) with a kernel set that has all four of its dense shapes, 1bit serve --device npu, main before this change, one npu-hold ticket:

1bit forward-serve: warm-up pass failed (MLP dequant : data_size 0 at geometry 2048x2048 = 512 tiles (chunk 0 B is not a kind, and no padded grid fits): the container and config.json disagree); serving anyway -- the first request will pay it
1bit forward-serve: LFM2.5-1.2B-NPU2 ready on http://127.0.0.1:18276 (full ELF) [npu-target=xdna]

POST /v1/chat/completions   ->   HTTP 500
{"error":{"message":"forward step failed: MLP dequant : data_size 0 at geometry 2048x2048 = 512 tiles (chunk 0 B is not a kind, and no padded grid fits): the container and config.json disagree"}}
server still alive after the request

The serve announces itself ready for a model it cannot run, holds the device, and answers every request with a 500 whose message blames a disagreement between the container and its config. There is none: the pack is fine. The forward has no short-convolution layer, runs those ten layers as attention, and trips over the first tensor it expects and the pack does not have.

Why the existing refusal did not catch it: the forward already refuses an unknown layer kind by name, but from config.json's layer_types. LFM2 declares its hybrid through full_attn_idxs and has no layer_types key, so that check sees nothing.

The fix

The loader already reads the short-convolution tensors (model.h: "a layer IS a conv layer iff shortconv_in_proj_weight is present"). model_shortconv_layers() counts such layers and names the first; the forward's init refuses when the count is not zero, in the wording of the existing layer-kind refusal.

After, same pack and kernel set, binary from this branch, two tickets

1bit serve: forward init failed: model.q4nx has 10 gated short-convolution layer(s) of 16 (the first is model.layers.0.shortconv.*, LFM2's hybrid), a layer kind this forward does not implement (it runs full_attention, sliding-window and linear/GDN layers), so serving it would run those layers as attention; file the kind you need instead of converting it
server exited before ready: rc=1

Both runs the same. The serve does not come up, the device is released, and the message says what the pack has and what the forward lacks.

Tests

tests/npu_forward_loader_test.c (ctest npu_forward_loader), 6 new checks on a container written by the test: a file with shortconv.* tensors loads; two of its first four layers are convolution layers and the first is layer 1; none among the layers before it; the count works without the index; an attention-only container has none; no model, none. all cases ok.

The refusal line in q4nx_forward.cpp itself is only reachable with the NPU runtime, so its test is the device run above.

tools/copyright.py --check rc 0. Docs: docs/npu.md, a paragraph in the FastFlowLM rows section.

Limits and one loose end

  • This refuses the family; it does not serve it. Implementing the gated short convolution is the real work for the five lfm2 packs in the census, and is not attempted here.
  • One pack on the device. LFM2-1.2B-NPU2 is on the box and has the same layer pattern by its config; I did not run it.
  • Not explained: my very first attempt on this pack (main, same command, through a wrapper script) ended within one second, with the server process gone and its log stopping right after the kernels loaded, before the "warming" line. Three later runs of main's binary all reached "ready" as shown above. I did not capture an exit code for that first run and could not reproduce it, so I do not know whether it was a crash. With this change the question does not arise for this pack, since init refuses before that point.

🤖 Generated with Claude Code

…at load (Refs #1786)

FastFlowLM's LFM2.5-1.2B-NPU2 declares its hybrid through full_attn_idxs,
not layer_types, so the unknown-layer-kind refusal could not see it. Its
ten gated short-convolution layers were run as attention: the warm-up
failed with 'MLP dequant : data_size 0 ...' and the serve said ready.
model_shortconv_layers() counts them from what the loader already reads,
and the forward's init refuses, naming the first.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at cb440b0

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1786 - Partially compliant

Compliant requirements:

  • Every FastFlowLM/*-NPU2 model source imports and serves on --device npu
  • It imports with zero gaps and its self-check passes against a named reference
  • It serves on --device npu
  • The row names the kernel set and the engine commit that produced it

Non-compliant requirements:

  • It answers a known-answer prompt with the model's own chat template, 3 identical requests returning identical text
  • Decode tok/s and time-to-first-token measured under npu-hold

Requires further human verification:

  • It answers a known-answer prompt with the model's own chat template, 3 identical requests returning identical text
  • Decode tok/s and time-to-first-token measured under npu-hold
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incorrect Layer Kind Refusal

The fix introduces a check for gated short-convolution layers, but it only checks the first tensor (shortconv.in_proj_weight) to determine if a layer is a convolution layer. This approach may not be robust, as a layer could have other short-convolution tensors without shortconv.in_proj_weight being present, potentially leading to missed detections of convolution layers.

{
    int first_conv = -1;
    const int n_conv = model_shortconv_layers(mw_, cfg.num_layers, &first_conv);
    if (n_conv > 0) {
        err_ = "model.q4nx has " + std::to_string(n_conv) + " gated short-convolution layer(s) of " +
               std::to_string(cfg.num_layers) + " (the first is model.layers." + std::to_string(first_conv) +
               ".shortconv.*, LFM2's hybrid), a layer kind this forward does not implement (it runs "
               "full_attention, sliding-window and linear/GDN layers), so serving it would run those layers as "
               "attention; file the kind you need instead of converting it";
        return false;
    }
}
Incomplete Convolution Layer Detection

The model_shortconv_layers function only checks for the presence of shortconv.in_proj_weight to identify convolution layers. If a layer has other short-convolution tensors but lacks shortconv.in_proj_weight, it will not be detected as a convolution layer, potentially leading to incorrect behavior or runtime errors.

static inline int model_shortconv_layers(const ModelWeights* mw, int n_layers, int* first) {
    int count = 0;
    if (first) *first = -1;
    if (!mw || !mw->layers) return 0;
    for (int l = 0; l < n_layers; l++) {
        if (mw->layers[l].shortconv_in_proj_weight.shape[0] == 0) continue;
        if (count == 0 && first) *first = l;
        count++;
    }
    return count;
}

@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 cb440b0: READY

Reviewed head cb440b0 (refs #1786, the LFM2.5 row). This is the first verdict.

  • git merge-tree onto main: no conflicts. CI build and CodeQL green.

  • npu_forward_loader_test, built from this head: "all cases ok", rc 0.

  • Mutants of model_shortconv_layers() (npu/forward/include/model.h), each rebuilt and run:

    • never counting a layer: rc 1, "two of its first four layers are convolution layers, and the first is layer 1";
    • reporting the last conv layer as the first: rc 1, the same arm.

    I restored the file (git status clean).

  • npu/forward/src/q4nx_forward.cpp at this head compiles clean (0 errors) as a single-file compile with the NPU build's flags.

  • The detection is the loader's own definition. It counts layers whose shortconv_in_proj_weight has a shape, not a config.json key, so it catches LFM2, whose hybrid is declared through full_attn_idxs with no layer_types and slipped past the existing layer-kind refusal. The refusal fails init, so the serve never announces itself ready for a model it would answer 500 to.

Note: every LFM2 container now stops at load on this route, including pk-lfm12, the pack #2149 discussed for q/k norm. That is the intended outcome: it was never actually served, only mis-served as attention. #2161 (the shortconv reference) is the path that would lift it.

Not re-run by me: the device rows (the 500 before; the load refusal in two tickets after).

@bong-water-water-bong
bong-water-water-bong merged commit e023793 into main Nov 11, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the cl-flm/lfm2-refuse-by-name branch November 11, 2026 10:00
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at cb440b0 — fresh configure rc 0, built and ran npu_forward_loader_test: 27 checks, all cases ok, including the loader arms (a file shorter than 8 bytes is refused, a header length past the end is refused, bad data_offsets are refused ×4, the ALT-name spellings and the too-small-buffer arm).

The defect is the worst kind and the fix is the right shape. LFM2.5-1.2B-NPU2 loads — its container is fine, all four dense kernel shapes exist on the box — but ten of its sixteen layers are gated short convolutions, and before this change the forward ran them as attention. A model that loads and produces plausible-looking tokens while computing the wrong layer type is exactly the failure this fleet has spent the night removing: silent, plausible, and expensive to attribute.

Refusing by name, at load, with the count and the first path, is the correct alternative:

model.q4nx has 10 gated short-convolution layer(s) of 16 (the first is model.layers.0.shortconv.*, LFM2's hybrid),
a layer kind this forward does not implement …

That message answers the two questions an operator actually has — how much of this pack is unsupported and where is the first one — and model_shortconv_layers() is a pure helper over LayerWeights, so the census is testable without a container. That is the same split I have been asking for all night (#2026's pad_head_rows, #2054's container_geometry).

Two asks:

  1. An arm for the boundary the helper's own comment implies: a pack where every layer is shortconv (first == 0, count == n_layers), and one where the shortconv layers are not a prefix (e.g. layers 3, 7, 11 — LFM2's hybrid may interleave differently in another size). The current arms cover the ten-of-sixteen case; the "not a prefix" shape is what a naive break at the first non-shortconv would get wrong, and first is the field that would then lie.
  2. The refusal should say what would work — one clause like "the fused route serves these packs; this forward does not implement gated short convolution yet" — because a lane reading the refusal has to decide between switching route and dropping the model, and the message currently leaves that inference to them.

Scope of my check: fresh configure (rc 0), built the loader test in a scratch dir, ran it (27 ok), and read the new helper plus the refusal text. No device and no LFM2 pack on this box: the "it ran them as attention" half is the PR's own observation.

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