Repository navigation
GGUF-on-NPU: serve all six target arches on --device npu - #185
bong-water-water-bong wants to merge 9 commits into
Conversation
The converter's 'k_b to the MLA math grid' transpose (FLM_Q4NX_Converter 62594563f) is inverted. Measured directly with the converter's own _pack_2d_float on GLM's blk.0.attn_k_b.weight: the dequantised per-head slice is [512][192] (kv_lora x nope) and packing it AS-IS gives the 12 tiles/head the forward reads (load_heads3 with kb_rows_ = qk_nope_ = 192, cols = kv_lora_ = 512), while transposing it to [192][512] gives 16 tiles/head. The forward then reads past the slice and '1bit serve --device npu' segfaults on the first request. A repack via scripts/repack_gguf.py therefore emits k_b [20][16][5120] while the Q4NX the GLM capture was verified against (/tmp/glm_v3) has [20][12][5120]. The engine's repack now runs the converter from a patched copy with that gate disabled, the same way it already forces Q4_1, and prints why. Verified: the patched repack writes k_b [20][12][5120] (v_b unchanged at [20][16][5120]).
|
Docs7 for 1bit-monster/engine
Commit |
PR Reviewer Guide 🔍(Review updated until commit b6b5864)Here are some key observations to aid the review process:
|
|
Review: please fix this at the source rather than in the engine.
Suggest: settle the orientation with the converter's author, fix it there, and drop the patch here. If a stopgap is needed meanwhile, gate it on a converter version or config field, not on source text, and fail loudly when neither form matches. |
|
The converter now reverts the inverted k_b transpose at the source (5fc52149b, "converter deepseek2: revert the inverted k_b transpose (62594563f)"). This PR's source-text patch can be dropped: close it, or reduce it to a load-time check in the forward that k_b is 12 tiles per head. |
…grid) The MLA/MoE path in this re-synced forward was missing two canonical npu-infer fixes, so `1bit serve -m GLM-4.7-Flash-Q4_K_M.gguf --device npu` decoded " jp" instead of " Paris": 1. mla_expert() transposed every routed expert. Its tile decode already reproduces the GGUF's flat order, which IS the logical [N][K] weight, so the transpose made every routed expert ~10-20x wrong and flipped the top-k selection (canonical f336dfc72). Now a plain std::copy. 2. v_b was indexed in-major (wv[k*vh+n]); the Q4NX's logical index is out-major (h*256+n)*512+k (canonical 3d1b252ec). Now wv[n*kv_lora+k]. Verified on the NPU: /v1/completions "The capital of France is" -> " Paris" (4m4s). Q4NX is byte-identical to the verified glm_npu2 dir; the two bugs were the only functional deltas from canonical.
…-device npu) Qwen3-Coder-30B-A3B (qwen3moe) is the first NON-MLA MoE arch: the FFN is a 128-expert top-8 softmax router with no shared expert and no router bias. Four changes make `1bit serve -m <gguf> --device npu` answer " Paris": 1. forward: parse the MoE params (n_expert/top_k/expert_im/dense_im/ leading_dense/gating/scale) INDEPENDENTLY of mla_, so a non-MLA MoE arch gets its expert geometry (they were read only inside `if (mla_)`). 2. forward: load the MoE tensors (bf16 router + pooled gate/up/down experts) in the standard path when n_expert_>0, instead of the dense gate/up/down; the runner bias and shared expert stay optional (moe_mlp already handles both). 3. forward: dispatch moe_mlp in the standard path for n_expert_>0 (deepseek2 still reaches moe_mlp from the MLA branch, which continues first). 4. forward: `dense_weights_cached` must NOT skip load_layer for a MoE arch -- moe_router_ is a HOST vector reloaded per layer, so skipping it left every layer using the last-loaded layer's router on tokens >= 1 (pos 0 was fine, which is why it looked plausible but decoded "."). scripts/repack_gguf.py: write the MoE config keys for qwen3moe/qwen35moe -- the per-EXPERT intermediate (768) must replace the dense feed_forward_length (5472) so the G/U/D designs ARE the expert MLP, plus n_expert/top_k/softmax gating. Verified: anchor -> " Paris" (3m54s); experts/q_proj/o_proj/router all decode corr >= 0.9989 vs the GGUF; GLM (deepseek2) still answers " Paris" (regression).
The device exposes one NPU hardware context; a second concurrent '1bit serve --device npu' dies inside DRM_IOCTL_AMDXDNA_CREATE_HWCTX (EINVAL) and exits 1. npu_engine_universal has always held this flock; the serve route did not, so siblings could not serialise (and my qwen3moe run hit exactly this when a leftover GLM serve held the context).
Qwen3.6-35B-A3B is a hybrid: 30 GDN (gated delta-net linear attention) layers interleaved with 10 full-attention layers, each FFN a 256-expert top-8 softmax router plus a gated shared expert. Extends the qwen3moe work: - forward: load_gdn_layer now takes the MoE branch (router + pooled experts + the optional shared expert) when n_expert_ > 0, instead of the dense gate/up/down; the GDN layers' FFN was already dispatched by the common MLP section (it sees n_expert_ > 0 and calls moe_mlp). - forward: the shared expert (share_gate/up/down_exps_proj) and its [H] gate (share_router) load in both MoE branches; moe_mlp applies sigmoid(x . g) to the shared expert output (qwen35moe), and leaves it unscaled when absent (deepseek2 has no such gate). - repack: write the GDN geometry (linear_num_key_heads/key_head_dim/ value_head_dim/num_value_heads/conv_kernel_dim from the GGUF ssm.* fields), attn_output_gate (the q_proj emits q|gate) and the per-layer layer_types list the forward dispatches on. Kernels generated for the six new geometries (QKV K2048 N9216 for the gated full-attn, G/U K2048 N512, D K512 N2048, QKV K2048 N8192 (GDN qkv), Z K2048 N4096). Q4NX repacked (21 GB) + the GDN config keys verified.
The stamp records converter_path/commit; --check-stamp re-resolves the converter from the dir's config. It used model_type, which for qwen35moe is the HF name 'qwen3_5_moe' (no q4nx/models/qwen3_5_moe.py), so it fell back to the canonical checkout while the dir was built by the iso-build one -> always 'stale' -> the serve discarded and re-repacked the 21 GB dir on every start (and twice hit ENOSPC doing it). config.json now carries "arch" (the GGUF architecture) and the check prefers it.
…er.json
Two defects on the SentencePiece path (MiniCPM4-8B, the only SentencePiece model of
the four blocked targets), both found by testing against oracle ids rather than by
reading the code.
1. Every SPM decode gained a spurious leading space. decode() mapped every metaspace
to ' ', including the one spm() synthesises as SentencePiece's add_dummy_prefix
marker, so 'The capital of France is' -> ' The capital of France is' and
'Paris' -> ' Paris'. Strip exactly that one leading space: stripping one rather
than all also preserves a genuinely space-leading input, since ' Paris' encodes to
the double metaspace [1345, 3837, 1360] and must decode back to ' Paris'.
2. A correctly-typed tokenizer.json could not be loaded at all. model.at("merges")
throws [json.exception.out_of_range.403] when the key is absent, and a
SentencePiece/Unigram tokenizer.json has no such key -- the throw aborted the
constructor before the SPM detection, so the unigram path was unreachable and
every SPM model fell back to raw bytes. Also accept the Unigram list-form vocab
([[token, score], ...], id = position) as well as the BPE object form. Verified
with a negative control: the old expression throws on a Unigram file, the new one
returns an empty merge set.
Test: tests/data/minicpm4_tokenizer_golden.json, whose ids come from llama.cpp's own
tokenizer (llama-server /tokenize on MiniCPM4-8B-Q4_K_M.gguf), not from this
tokenizer. Results: MiniCPM4 as repacked 5/5 (was 0/3), MiniCPM4 as a proper Unigram
file 5/5 (previously unloadable), Qwen3 BPE golden 14/14 unchanged.
|
Persistent review updated to latest commit aa9b7f9 |
| if (!std::getenv("NPU_NO_DEVICE_LOCK")) { | ||
| const char* lk = std::getenv("NPU_DEVICE_LOCK"); | ||
| if (!lk || !lk[0]) lk = "/tmp/1bit-npu-device.lock"; | ||
| int lfd = ::open(lk, O_CREAT | O_RDWR, 0666); |
| if (!std::getenv("NPU_NO_DEVICE_LOCK")) { | ||
| const char* lk = std::getenv("NPU_DEVICE_LOCK"); | ||
| if (!lk || !lk[0]) lk = "/tmp/1bit-npu-device.lock"; | ||
| int lfd = ::open(lk, O_CREAT | O_RDWR, 0666); |
- tools/registry_check.py: an npu .gguf row resolves under --models and reads its arch from the GGUF header (the GGUF-on-NPU route repacks it itself), instead of being treated as a Q4NX model directory. - registry/check_models.tsv: add the six npu GGUF rows (qwen2, qwen3moe, qwen35moe, deepseek2/glm, minicpm, llama). - registry/checked.json: refresh the Qwen3-0.6B npu-directory entry to the current HEAD. - tests/anchor_rerun.sh: harness to re-run the anchor completion across a set of served models and capture the results.
aa9b7f9 to
4784b06
Compare
|
Persistent review updated to latest commit 4784b06 |
|
Still not mergeable:
|
The GGUF-on-NPU route serves six arches, each verified by a captured `--device npu` run (goal muhz0s39-8c9l9o). Record them so the registry reflects what the NPU actually carries — text decoders only: - qwen2 (Qwen2.5-7B), qwen3moe (Qwen3-Coder-30B), qwen3_5_moe (Qwen3.6-35B), deepseek2 (GLM-4.7-Flash), minicpm (MiniCPM4-8B), llama (MiniCPM5-1B). Regenerates registry/architectures.json (npu count 1 -> 7).
|
Persistent review updated to latest commit b6b5864 |
|
Closing as superseded: all nine commits landed through #209 (squash 4c8e861), which is why this branch now conflicts. I checked file by file against main (fd5662d): five files are identical, and in the other seven every line only this branch has is an older version that main replaced on purpose:
|
Summary
1bit serve -m <model.gguf> --device npunow repacks a GGUF into the Q4NX int4tile format and runs the NPU lane end-to-end for six target architectures. Each
answers the anchor prompt ("The capital of France is", greedy, max_tokens=1):
Captured runs:
evidence/gguf-npu-six/rerun_*.json+task4_READY.md(goal
muhz0s39-8c9l9o). The vulkan and hrx lanes are unchanged (serve_e2ePASS), and the existing Q4NX-directory fast lane stays bit-identical on its
golden check (24/24).
Changes
k_bper-head transpose isinverted; the engine repack disables that gate in its patched converter copy
(analysis below), and the two missing GLM MLA fixes are resynced (expert
transpose, v_b grid).
attn_output_gate, the hybrid forward, and six new kernels.1bit servetakes/tmp/1bit-npu-device.locklikethe ELF runner, so concurrent serves don't fight over the one device.
tokenizer.jsonaccepted(MiniCPM4).
provenance-stamp check re-resolves the converter.
check_models.tsv+registry_check.pynpu-
.ggufsupport) and the anchor re-run harness (tests/anchor_rerun.sh).Verification
--device npuruns:evidence/gguf-npu-six/rerun_{q2,q3m,q35,glm,m4,m5}.json.tests/serve_e2e.shvulkan PASS and hrx PASS (MiniCPM5-1B).tests/npu_lane_e2e.sh24/24 bit-identical (Qwen3-0.6B).GLM k_b transpose (original analysis)
FLM_Q4NX_Convertercommit62594563f("transpose k_b's per-head slice to theMLA math grid") is inverted. Measured with the converter's own
_pack_2d_floaton GLM's
blk.0.attn_k_b.weight:[512][192](kv_lora x nope)[12][5120][192][512](after transpose)[16][5120]The forward reads
k_b_projwanting the 12-tile grid (its own comment says so),and the pre-
62594563fQ4NX the GLM capture was verified against hask_b [20][12][5120]. With the transpose in place the repack emittedk_b [20][16][5120], the forward read past the slice, and the serve segfaultedon the first request. The engine repack now runs the converter from a patched
copy with that gate disabled (the same way it already forces Q4_1) and prints
why; the converter repo is untouched.
Performance (measured 2026-09-28,
--device npu)8-token greedy continuation of "The capital of France is" (load = serve start →
/health200):Correctness across all six arches is the milestone. Decode is slow (0.006–0.17 tok/s)
because this is the model-generic forward (~100+ synchronous NPU dispatches/token,
correctness-first), not the fused fast lane (~96 tok/s on Qwen3-0.6B). Full run
details:
evidence/gguf-npu-six/benchmark-2026-09-28.md.