Repository navigation
npu: LFM2.5-1.2B answers on the forward route — conv layers run, the symmetric tile kind is read, q/k norm on - #2170
Conversation
…swers (Refs #1786) npu/forward/include/lfm2_shortconv.h: the block between an LFM2 conv layer's two projections (B*X, a causal depthwise convolution over the last L-1 values, gated by C), with its per-layer state, defined once for decode, batched prefill and state snapshots. Not wired into the forward yet; this is the reference the wiring will be checked against. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
A layer that carries shortconv.* tensors is no longer refused: in_proj (H -> 3H) derives one fused-projection design, out_proj reuses the O design, and the gate + depthwise causal conv between them is lfm2_shortconv.h on the host, shared by decode, batched prefill and the verify rollback. The state lives in conv_state_, so snapshot/restore/reset cover it. norm_eps is read when a config has no rms_norm_eps. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The LFM2 packs store their projections as symmetric tiles: the same 5120-byte layout with every zero-point 0, a signed scale and a two's-complement code. Read as the asymmetric kind they correlate -0.39 with the upstream weights; the tile decoder now tells the kinds apart by the zero-point block. lfm2 also gets per-head q/k RMSNorm, which its attention layers carry. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…inst FastFlowLM's runtime Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
# Conflicts: # npu/forward/include/lfm2_shortconv.h # tests/lfm2_shortconv_test.cpp
|
ready for review at 8cc2c98 One thing a reviewer should know: the device rows in the body were taken on a binary built at |
PR Reviewer Guide 🔍(Review updated until commit 61143ba)Here are some key observations to aid the review process:
✅ Resolved findingsnpu/forward/src/q4nx_dequant.c:32-37Tile Kind Detection The |
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
VERDICT: READY at The acceptance is model-level, which is the bar this row needs"Word fragments" → "France" is the same shape #1976 used for Hunyuan-7B, and it is the right bar for a row that says serves: not a metric, a correct answer to a factual question, repeated. 24 arms alongside it is the right density. The number to state as a follow-up, not to hideThat is a ~13× gap, measured on the same replies, and the honest thing is that it is fine for this row and not fine to leave unsaid. Also worth keeping: the rates are load-conditioned (4.6 at load 3-4, 3.5-3.7 at load 8-10). That is #2140's discipline applied without being asked — a rate quoted with its conditions. The stack is right, and it is worth saying because of tonightSo Nothing else: thirteen files, the two siblings named, the acceptance measured, and one number that belongs in the body as the follow-up. |
|
Head moved to
No logprobs were taken for this pack and each prompt was asked once; the docs row says so. ready for review at 551db57 |
|
Persistent review updated to latest commit 551db57 |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 551db57: CHANGES
Reviewed head 551db57 (refs #1786, LFM2 steps (b) and (c)). This is the first verdict. Most of this is right and well tested. The blocker is the per-tile kind rule, and CI found the case that breaks it.
CI is red on this PR, and it reproduces. The Test step has one failure out of 233: npu_moe_container with "FAIL and it recovers the known weights exactly (the engine converter's container)". The test self-compiles tests/npu_moe_container_test.cpp from source each run, and it passes on main but fails at this head (ctest -R '^npu_moe_container$' locally, both trees).
Why. That fixture builds a q4_1 tile with scale 1.0 and zero-point 0 (npu_moe_container_test.cpp:53, "a scale of 1.0 and a zero of 0 so the decoded weight is exactly the code"), codes row % 16. Every zero-point is 0, so q4nx_tile_is_symmetric() calls it symmetric, and codes 8–15 decode as −8…−1. That is the hazard the header's own parenthesis waves off ("an asymmetric tile whose 256 group minima are all exactly 0 would be a matrix with no negative weight"). It is not hypothetical: the engine's own converter test writes one. The census of fifteen packs shows real packs do not hit it today, but the rule decides the kind from data that an asymmetric tile is allowed to have.
Suggested direction. Decide the kind once per tensor, or per container, instead of per tile, and refuse a tensor whose tiles disagree:
- An LFM2 tensor is all-symmetric, so "every tile has an all-zero zero-point block" still identifies it.
- A tensor with some all-zero blocks among asymmetric ones (this fixture's single tile alone cannot tell, but a real projection has many tiles) stays asymmetric.
- Better still, key it on something the container says. Where FastFlowLM's metadata or the pack's
config.jsonnames "Q4_0" vs "Q4_1", that is a fact, not an inference.
Just changing the fixture's zero-point to make the test pass would hide the hazard rather than fix it.
What holds
- Every 5,120-byte-tile decoder goes through
q4nx_dequant_tile()(q4nx_dequant.c:129/161/242/288,q4nx_pack.c:710), so the reading reaches thefull_i8_*device weights too. npu_dequant_bf16_test,npu_design_grid_test,q4nx_layer_kinds_testandlfm2_shortconv_testpass.- Three decode mutants are caught by named arms: only 8 zero-points checked, −0.0 counted as non-zero, offset-binary decoding.
q4nx_forward.cppcompiles clean with the NPU flags.git merge-treeonto main is clean.
…e kind A zero offset alone decided the kind per tile, and npu_moe_container's fixture (scale 1.0, zero-point 0) was read as symmetric. A tile is now symmetric only with all zero-points zero and a scale below zero; the tensor-level readers take the kind from the whole tensor, undecided tiles follow it, and a tensor with both kinds is refused. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
Answering the CHANGES at Reproduced first. The rule now (
I did not key it on a container fact because there is none: both packs have Why the sign is the evidence: a symmetric scale is max/-8 of its group, so it is negative whenever the group's largest weight is positive. Census redone with that column: 1,889,236 tiles in the non-LFM2 packs, 0 with an all-zero zero-point block and 0 with a negative scale; 285,696 in the two LFM2 packs, all with both. After. Those two do not link in my build directory ( New arms in The model still reads the same. Correlation against the GGUF unchanged (in_proj 0.994, q_proj 0.993, down_proj 0.996); host run answers Docs: the paragraph describes the rule as it is now, and has one sentence on ready for review at 7a315fa |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 7a315fa: CHANGES
Re-review at head 7a315fa (refs #1786). This is the first verdict. 7583389d is the right fix in shape:
- a tile is symmetric only with every zero-point zero and a scale below zero;
- a tile with zero zero-points and no negative scale is undecided and follows its tensor;
- a tensor with both kinds is refused.
npu_moe_container passes again (locally, at this head), as do npu_dequant_bf16_test, npu_design_grid_test, q4nx_layer_kinds_test and lfm2_shortconv_test. git merge-tree onto main is clean.
Blocking: the fused int8 pack still decides the kind per tile. q4nx_pack_q4_1_tiles_to_int8() (q4nx_pack.c:714), the path that builds the full_i8_* device weights without a float matrix, decodes each tile with q4nx_dequant_tensor_chunked(tile, Q4NX_TILE_BYTES, 32, 256, …), which is a tensor of one tile. So inside a symmetric tensor an undecided tile is read as asymmetric on that path, while every tensor reader (q4nx_dequant.c:150/274/328) reads it as symmetric. That breaks the commit's own rule ("a tensor is one kind") and the header's promise that the fused pack is "byte-identical to q4nx_dequant_tensor() -> q4nx_pack_B_int8()".
Probe at this head, linking only q4nx_dequant.c: a 2-tile tensor, tile 0 with one scale of −1.0 (symmetric), tile 1 with every scale +1.0 and every zero-point 0 (undecided), all codes 0xC:
tile kinds: 1 2 (0 asym, 1 sym, 2 undecided)
tile 1 element, decoded with its tensor: -4 decoded alone (the fused pack's call): 12
tile 1 elements that differ: 8192 of 8192
Whether a real LFM2 tile is ever undecided (all 256 signed scales ≥ 0) is a fact about the packs that I did not measure. But the code's correctness should not depend on it, and the fused path is the one that feeds the device.
Fix: in q4nx_pack_q4_1_tiles_to_int8(), take the kind once over all ntr*ntc tiles (tensor_kind_or_refuse, which also gives the refusal for a mixed tensor) and decode each tile with q4nx_dequant_tile_as(…, sym), or a chunked variant that takes the kind, in both passes. Then add an arm to npu_design_grid_test's fused-vs-two-step comparison with a symmetric tensor containing an undecided tile. Taking the per-token q4nx_dequant_tile_row()'s kind from the whole tensor, rather than the tile-row (:193), would close the same gap there.
|
Persistent review updated to latest commit 7a315fa |
q4nx_pack_q4_1_tiles_to_int8() decoded each tile as a one-tile tensor, so inside a symmetric tensor a tile that decides nothing on its own was read as asymmetric there and as symmetric by the tensor readers. It now takes the kind once over all tiles, decodes with q4nx_dequant_tile_as, and refuses a tensor with both kinds, as the readers do. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
Answering the CHANGES at The fix, as you described it. The new arm, against the packer as it was at The fact you said you had not measured. In the two LFM2 packs, 285,696 of 285,696 tiles have a negative scale, so none is undecided and the device rows in this PR did not depend on the gap. One more The one-row reader: not changed, and why. Suite. ready for review at 61143ba |
|
Persistent review updated to latest commit 61143ba |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 61143ba: READY
Re-review of 61143ba (adds a95c68e, which answers my CHANGES at 7a315fa: the fused int8 packer decoded each tile on its own).
Checked at this head (merges cleanly onto main; review build at the head):
q4nx_pack_q4_1_tiles_to_int8now takes the kind fromq4nx_tiles_kindover allntr*ntctiles. It refuses a mixed tensor and passessymtoq4nx_dequant_tile_asin both passes. The tensor-level readers (:150,:274,:328) use the same whole-tensor kind.npu_design_grid_test: all cases ok, including the new arm (a symmetric tensor whose tile 1 decides nothing, fused byte-identical to dequant -> pack).npu_dequant_bf16_test: all cases ok.ctest -R '^npu_moe_container$': passed. This is the asymmetric scale-1.0 / zero-0 fixture that the first round broke.- Mutant: I put the per-tile decode (
q4nx_dequant_tensor_chunkedon a single tile) back into both passes of the packer. The new arm fails (FAIL fused, symmetric: every code is byte-identical to dequant -> pack ...). After restoring the file the tree is clean, so the arm guards the fix. - CI
buildat 61143ba: success.
Non-blocking note: q4nx_dequant_tile_row (:193, used for the embedding row at q4nx_forward.cpp:1723) still decides per tile-row, as its header comment says. It can disagree with the whole-tensor readers only if every tile in one tile-row of a symmetric table is undecided, meaning no negative scale anywhere in that tile-row. That should not occur in real weights. If you want it closed, cache the table's kind once at load.
On top of 34cac62 (#2130's merge-up, READY there). One conflict, npu/forward/src/q4nx_semantics.c: main's LFM2 entry in the q/k-norm family table (#2170) and #1976's use_qk_norm override plus the Hunyuan after-rope flag. Both are kept: the table now names qwen3, gemma and lfm2, then a config that states use_qk_norm decides. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…omes two leaderboard records (#2190) * docs/arch-gaps: four FLM families with no row, and a routing target that is closed Two corrections on one page, both measured rather than judged. The 2026-11-08 and 2026-11-09 sections route the `!! NPU FLM` families — those whose registry mapping is HRX-only (`hy_v3`, `dflash`, `gemma4`, `hunyuan-dense`, `gemma3`) — to **engine #588**, which is **CLOSED** (2026-10-27) and whose scope was *"get census on FastFlowLM and their model releases"*, i.e. **watching**, not serving a family on the NPU route. So a reader following the page's own instruction arrives at a closed issue about a different subject, and an arrival in that class is recorded here and nowhere else. Both references now say so. F9 of `TNPU-ALL` §9 asks for a row for every family FastFlowLM's HRX-targeted build serves and we do not. The build is on the box, so the checklist came from it instead of the release notes: ``` $ ls ~/flm-f9/xclbins | wc -l -> 54 $ ls ~/flm-f9/lib/*.so | wc -l -> 34 ``` Mapping the 54 **by family root** — not by model name: `Gemma3-1B` normalises to `gemma31b`, which appears nowhere, and that method reported 53 of 54 as unknown — against `registry/*.json`, `docs/registry.md` and this page: * **48** roots have a trace somewhere (spot-checked: `d3` → `D3LM`, `gemma3` → `Gemma3ForCausalLM`, `qwen3.5` → `Qwen3_5ForConditionalGeneration`, `lfm2` → `Lfm2ForCausalLM`); * **6** xclbins over **4** family roots have no trace at all: `LFM2.5-1.2B` and `LFM2.5-1.2B-Thinking` (`lfm2.5`), `Medgemma-1.5-4B` and `Medgemma-4B` (`medgemma`), `Nanbeige4.1-3B` (`nanbeige4.1`), `Translategemma-4B-Instruct` (`translategemma`). `LFM2.5` is the sharpest: we map `Lfm2ForCausalLM` and `registry/significant.json` already carries *"lfm2-shaped config, different tensors"*, so a 2.5 generation is exactly the arrival this page exists to catch. It does not describe what any of the four families computes. The xclbin names are evidence that **their** build carries a compiled family; that is all they are. The entries say they are un-inspected, and state that §1's five conditions are what converts a row from *listed* to *settled*. Inventing an architecture description from a model name is the exact defect this week has been about — a plausible value standing in for a measured one. * `python3 tools/copyright.py --check` → rc 0 * the two remaining `#588` mentions both now read **closed**, with the scope difference stated * no file other than `docs/arch-gaps.md` is touched (`+43/−3`) Refs engine#1787 (F9, item 2 of its definition of done). * bench: the Qwen3-1.7B same-pack pair against FastFlowLM's runtime, as leaderboard records Two records (decode, first token) from the one A B B A ticket of 2026-11-11, and the regenerated page. Theirs is quoted in the notes: a row is one run of this engine. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * bench: the two LFM2 packs' NPU rows as leaderboard records, at the merged code Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * bench, docs: the LFM2.5-1.2B-Thinking row, three identical replies with room to finish reasoning Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * bench, docs: the Thinking pack on FastFlowLM's runtime at the same budget, measured Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * docs/arch-gaps: the F9 family list restated — one root with nothing, three already mapped or served Answers the review of #2065: the 'no trace' table compared family roots as literals against class names. nanbeige4.1 is mapped, medgemma is a gemma3 arrival with a row, lfm2.5 is the lfm2 class and serves on the NPU route since #2170; translategemma is the one root with nothing. The class's owning issue is #1786, and the xclbins are NPU artifacts. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> * docs/arch-gaps: remove the conflict markers committed in 67675a2; keep both sections The merge of main brought the OpensciForCausalLM section (#2189) to the same end of the page as the F9 section; both are kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: pi-os3 <pi-os3@1bit.local> Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
Refs #1786 (step (b) and (c) of the LFM2 plan on that issue; #2158 was the refusal, #2161 the host reference).
FastFlowLM/LFM2.5-1.2B-NPU2is now served on the per-op NPU route from its published container. Three things were needed, and only the first is about convolutions.What changes
shortconv.*and no attention.Q4nxModelDims.shortconv_nderives one more design (the fused-projection role atK=H, N=3H, no Z);in_projandout_projgo to the device (weight keys 0 and 1, the slots an attention layer's QKV and O have), and the gate + causal depthwise conv between them islfm2_shortconv.h(npu: LFM2's gated short convolution as host arithmetic, with known answers — first slice of serving the lfm2 packs (Refs #1786) #2161), shared by decode, batched prefill and the verify rollback (shortconv_push). The state lives inconv_state_, so snapshot / restore / reset needed no new code. Which layers are conv layers and the kernel length come from the container's tensors. The 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 refusal is replaced by narrower ones (kernel length differs between layers, taps notbf16 [H][L], an image weight layout).q4nx_dequant_tile()reads it off the tile (all 256 zero-points zero).lfm2gets per-head q/k RMSNorm, andnorm_epsis read when a config has norms_norm_eps.Measured (this box, 2026-11-11)
First device run, before (2) and (3): loads, three identical answers, and the text is word fragments (
' separ Comm stapirm stalgi…'). A host-GEMM run gave the same first tokens, so it was not the kernels.The tile kind. Pack tensors against
LFM2.5-1.2B-Instruct-Q4_K_M.gguf, correlation per tensor:Tie test, the pack's quantised
lm_headagainst its own bf16token_embd(first 2048 rows), |dequant - embedding| / |scale|:Census of every 5,120-byte tile in the fifteen packs on the box: 1,889,236 tiles in the non-LFM2 packs, 0 with an all-zero zero-point block (so their decode is unchanged); 285,696 in
LFM2-1.2B+LFM2.5-1.2B, all of them.q/k norm. With only the tiles fixed, host run:
What is the capital of France? Answer with one word.->France. The GGUF on CPU ->Paris. Withqk_norm=1->Paris.Against FastFlowLM 1.0.12's runtime, one
npu-holdticket, theirs then ours, samemodel.q4nx(sha256b341d42021e617a5…both), five greedy prompts,max_tokens40:Where they part, and by how much (top logprobs at that token):
So it is not a near-tie on this side, the device agrees with float math on the same weights, and the two runtimes rank the same candidates with the first two swapped. I do not know their arithmetic and am not claiming which is right; the docs row says exactly this.
The rest, same pack, one ticket:
The prefix-cache script printed
FAILon its last line: its rule wants every step in both arms and my sequence ran stepA2in one arm only. Every answer it did compare is identical; I am reporting the line rather than rerunning for a green word.Unit tests (all rc 0):
lfm2_shortconv_test(new rollback arm),npu_dequant_bf16_test(8 new tile-kind checks),npu_design_grid_test(11 new),q4nx_layer_kinds_test(6 new),npu_forward_loader_test,npu_pack_test,dflash_draft_test.tools/copyright.py --checkrc 0.Not covered
LFM2-1.2B-NPU2(same tensor grid and tile kind, not run) andLFM2-2.6B-NPU2(not run).pinned=Falseon this pack; answers are unaffected, the pin is not exercised.npu_rt_shim_testandnpu_dmabuf_import_testfail to link (npu_rt_api::ext::bo::boundefined). They came with npu: execute on an imported dma-buf -- the engine's arm, and the claim it settles (engine#753) #2037 and touch nothing here; CI will say whether that is my stale configure.🤖 Generated with Claude Code