From 0e2392a6f3613ad91e01bcf7d668636fe07a2adc Mon Sep 17 00:00:00 2001 From: t Date: Wed, 11 Nov 2026 15:10:47 -0400 Subject: [PATCH 01/10] wip: classifier on 3b --- npu/pack_import.cpp | 19 +++++++++++++- tests/npu_pack_import_test.cpp | 45 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index c144eeb3f..d27e29848 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3432,6 +3432,19 @@ ContainerRoles container_roles(const std::vector& tensors) { {"gate_proj", "gate_proj"}, {"gate_proj", "ffn_gate"}, {"up_proj", "up_proj"}, {"up_proj", "ffn_up"}, {"down_proj", "down_proj"}, {"down_proj", "ffn_down"}, + // engine#1788 acceptance (F1 slice 3a gap): the MoE families MiniCPM-V 4.7 names, mapped to the roles the NPU + // forward ALREADY reads -- npu/forward/include/model.h carries up_exps_weight / gate_exps_weight / + // down_exps_weight / share_{up,gate,down}_exps_weight / moe_router_weight / shared_expert_gate_weight with + // those exact container names in their comments. So this is translation to an existing vocabulary, not a new + // one. The `share_*` spellings come first because `share_gate_exps_proj` contains `gate_exps_proj`. + {"share_gate_exps", "share_gate_exps_proj"}, + {"share_up_exps", "share_up_exps_proj"}, + {"share_down_exps", "share_down_exps_proj"}, + {"gate_exps", "gate_exps_proj"}, + {"up_exps", "up_exps_proj"}, + {"down_exps", "down_exps_proj"}, + {"moe_router", "moe_router"}, + {"shared_expert_gate","shared_expert_gate"}, {"input_norm", "input_layernorm"},{"input_norm", "attn_norm"}, {"input_norm", "attention_norm"}, {"post_norm", "post_attention_layernorm"}, {"post_norm", "ffn_norm"}, {"final_norm", "final_layernorm"}, @@ -3458,7 +3471,11 @@ ContainerRoles container_roles(const std::vector& tensors) { matched = true; break; } - if (!matched && n.find("norm.weight") != std::string::npos && n.find("layers.") == std::string::npos) + // "never inside a layer" has to be spelled positively enough to survive the container's own naming: + // MiniCPM-V 4.7 spells its blocks `model.layer.N.` (singular), so a guard against "layers." alone filed + // 30 per-layer linear_attn.ssm_norm tensors as final_norm -- ahead of the kSsm loop below (engine#1788). + if (!matched && n.find("norm.weight") != std::string::npos && n.find("layers.") == std::string::npos && + n.find("layer.") == std::string::npos) role = "final_norm"; // `model.norm.weight`: the top-level norm, never inside a layer if (!matched && role.empty()) { for (const char* s : kSsm) diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index f61788927..754cd1688 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -5094,6 +5094,50 @@ void test_container_plan(const std::string& root) { // pack that classifies completely, the alternate spellings that must classify the same way, and the two residues (SSM // and MTP) which must be RECOGNISED rather than reported as unknown, since `unassigned` is only useful if it means // "nothing knows this". +// engine#1788 acceptance, in the shape of a unit arm: the MoE vocabulary and the singular `layer.` spelling that the +// MiniCPM-V 4.7 container uses. Measured on the real 736-name pack, this pair of rules takes +// "733 names -> 413 named, 320 unassigned" to "733 names -> 733 named, 0 unassigned"; the names here are a +// representative subset so the arm runs without a 700-line fixture. +void test_container_roles_moe_and_singular_layer() { + const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; return x; }; + const auto roles = [&](std::initializer_list names) { + std::vector v; + for (const char* n : names) v.push_back(t(n)); + return pi::container_roles(v); + }; + + // 1. the eight MoE families, spelled as the container spells them + { + const auto r = roles({"model.layer.0.moe_router.weight", + "model.layer.0.shared_expert_gate.weight", + "model.layer.0.mlp.gate_exps_proj.weight", + "model.layer.0.mlp.up_exps_proj.weight", + "model.layer.0.mlp.down_exps_proj.weight", + "model.layer.0.mlp.share_gate_exps_proj.weight", + "model.layer.0.mlp.share_up_exps_proj.weight", + "model.layer.0.mlp.share_down_exps_proj.weight"}); + check(r.unassigned.empty(), "35b: the MoE families are named, not unassigned"); + check(r.role_of.at("model.layer.0.moe_router.weight") == "moe_router", "35b: moe_router"); + check(r.role_of.at("model.layer.0.shared_expert_gate.weight") == "shared_expert_gate", + "35b: shared_expert_gate is its own role, not a projection"); + check(r.role_of.at("model.layer.0.mlp.gate_exps_proj.weight") == "gate_exps", "35b: gate_exps_proj"); + check(r.role_of.at("model.layer.0.mlp.share_gate_exps_proj.weight") == "share_gate_exps", + "35b: share_gate_exps_proj is the shared expert, not the routed gate"); + check(r.counts.at("gate_exps") == 1 && r.counts.at("share_gate_exps") == 1, + "35b: the shared spelling does not also count as the routed one"); + } + + // 2. a per-layer norm under the singular block spelling stays a layer norm: it must NOT become final_norm + { + const auto r = roles({"model.norm.weight", "model.layer.0.linear_attn.ssm_norm.weight", + "model.layer.0.input_layernorm.weight"}); + check(r.role_of.at("model.norm.weight") == "final_norm", "35b: the top-level norm is final_norm"); + check(r.counts.at("final_norm") == 1, "35b: exactly one final_norm, even with `model.layer.N.` blocks"); + check(r.role_of.at("model.layer.0.linear_attn.ssm_norm.weight") == "ssm", + "35b: a per-layer ssm_norm is ssm, which is what kSsm says it is"); + } +} + void test_container_roles() { const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; return x; }; const auto roles = [&](std::initializer_list names) { @@ -6134,6 +6178,7 @@ int main(int argc, char** argv) { test_q4nx_container(golden_dir.empty() ? "." : golden_dir); test_container_geometry(); test_container_roles(); + test_container_roles_moe_and_singular_layer(); test_container_plan(golden_dir.empty() ? "." : golden_dir); test_hybrid_pack_import(golden_dir.empty() ? "." : golden_dir); test_rope_parameters_theta(golden_dir.empty() ? "." : golden_dir); From 18e74a8d8779e168889147cec9798d75654840f9 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 16:09:19 -0400 Subject: [PATCH 02/10] npu-pack: classify the MoE families and the singular `layer.` spelling, on top of slice 3b (engine#1788) Rebased onto #2099 (slice 3b) and reduced to what 3b does not already do: 3b fills the role fields and writes a prose note, so this drops the duplicate fields and keeps - the two classifier fixes: the final_norm guard tested `layers.` only, so the container's singular `model.layer.N.` spelling filed 30 per-layer linear_attn.ssm_norm tensors as final_norm (31 -> 1, ssm 180 -> 210); and the eight MoE needles mapped to the roles model.h already reads (320 unassigned -> 0). - a machine-readable `roles` block in `npu-pack info`, with `named` derived from tensor count minus the unassigned list so the two cannot disagree; - the vision container report (vision_weight.q4nx, 347 tensors, read by npu/vision_pack.cpp); Measured: MiniCPM-V-4.7-35B-A3B-NPU2 733 named / 0 unassigned, 1B 357 / 0, both with a 347-tensor vision container. The plan-level arms are written against 3b's fields and fail when a classifier rule is removed. --- npu/pack_import.cpp | 29 +++++++++++++++ npu/pack_import.h | 8 ++++ tests/npu_pack_import_test.cpp | 67 ++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index d27e29848..49549716b 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3306,6 +3306,20 @@ std::string Plan::manifest() const { c["tensors"] = container_tensors; c["payload_bytes"] = container_payload_bytes; c["dtypes"] = container_dtypes; + // engine#1786 item F1 slice 3b fills the two fields on the plan; this is what makes them readable from `info` + // without a caller linking the plan struct. `named` is derived so it cannot disagree with the list. + const int64_t unnamed = int64_t(container_roles_unassigned.size()); + c["roles"] = {{"named", container_tensors - unnamed}, + {"unassigned", unnamed}, + {"counts", container_role_counts}}; + if (!container_roles_unassigned.empty()) c["roles"]["unassigned_names"] = container_roles_unassigned; + if (container_vision_tensors >= 0) + c["vision"] = {{"tensors", container_vision_tensors}, + {"payload_bytes", container_vision_payload_bytes}, + {"roles_named", container_vision_roles_named}, + {"role_counts", container_vision_role_counts}, + {"note", "a second container (vision_weight.q4nx); it has its own reader " + "(npu/vision_pack.cpp), so this row reports what is there rather than planning it"}}; c["note"] = container_note; j["container"] = c; } @@ -3617,6 +3631,21 @@ Plan plan(const std::string& pack_dir) { geom += "; no source states "; for (size_t i = 0; i < g.missing.size(); ++i) geom += (i ? ", " : "") + g.missing[i]; } + // engine#1788: the vision tower is a SECOND container with its own reader (npu/vision_pack.cpp), so this + // reports what is there for a human reading `info` rather than planning it. 347 tensors of a 1080-tensor + // pack were invisible here, which is how a pack looks importable when half of it has never been looked at. + { + std::error_code ecv; + const std::string vision_path = pack_dir + "/vision_weight.q4nx"; + if (fs::exists(vision_path, ecv)) { + const Q4nxContainer v = read_q4nx_container(vision_path); + const ContainerRoles vr = container_roles(v.tensors); + p.container_vision_tensors = int64_t(v.tensors.size()); + p.container_vision_payload_bytes = v.payload_bytes; + p.container_vision_roles_named = int64_t(vr.role_of.size()); + for (const auto& kv : vr.counts) p.container_vision_role_counts[kv.first] = int64_t(kv.second); + } + } p.container_note = c.why.empty() ? "the pack is a safetensors container (model.q4nx): " + std::to_string(c.tensors.size()) + " tensors, " + std::to_string(c.payload_bytes) + " B of payload, no graph; " + geom + diff --git a/npu/pack_import.h b/npu/pack_import.h index ade20f81d..63ece2c2e 100644 --- a/npu/pack_import.h +++ b/npu/pack_import.h @@ -266,6 +266,14 @@ struct Plan { int64_t container_payload_bytes = 0; std::map container_dtypes; std::string container_note; // what the container genre still needs, empty for a non-container pack + // engine#1788: MiniCPM-V's vision tower is a SECOND container beside model.q4nx (vision_weight.q4nx, 1.14 GB / + // 347 tensors on the 35B). Reported, not interpreted: the vision tower has its own reader (npu/vision_pack.cpp), + // so this is for a human reading `info`, not for a plan -- and 347 tensors absent from it is how a pack looks + // importable when half of it has never been looked at. -1 when the pack has no such file. + int64_t container_vision_tensors = -1; + int64_t container_vision_payload_bytes = 0; + int64_t container_vision_roles_named = -1; + std::map container_vision_role_counts; // engine#1786 (item F1), slice 2: WHERE the container's geometry came from, and what it could not reach. // A bare number in `dims` cannot say whether the pack declared it or a file beside it did, and those two are free to // disagree (13 of the 29 packs measured carry a `dims` block; all 29 carry config.json). Empty for a non-container pack. diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 754cd1688..0633dfb4e 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -5138,6 +5138,72 @@ void test_container_roles_moe_and_singular_layer() { } } +// engine#1786 item F1 slice 3b / engine#1788: the acceptance a container-genre plan is judged by, on the fields slice 3b +// now fills. `plan()` must say how many of a container's names it can place, and the number has to be all of them -- a +// pack whose MoE vocabulary or block spelling the classifier does not know is NOT importable, and the plan states that +// instead of leaving it to a caller with a scratch harness. Measured on the real packs: MiniCPM-V 35B-A3B 733/733, 1B 357/357. +static void write_container(const std::string& path, const std::vector& names) { + json hdr = json::object(); + size_t off = 0; + for (const auto& n : names) { + hdr[n] = {{"dtype", "F32"}, {"shape", {1}}, {"data_offsets", {off, off + 4}}}; + off += 4; + } + std::string h = hdr.dump(); + while (h.size() % 8) h.push_back(' '); + std::ofstream f(path, std::ios::binary | std::ios::trunc); + const uint64_t hlen = uint64_t(h.size()); + f.write(reinterpret_cast(&hlen), 8); + f.write(h.data(), std::streamsize(h.size())); + std::vector zeros(names.size(), 0.0f); + f.write(reinterpret_cast(zeros.data()), std::streamsize(zeros.size() * sizeof(float))); +} + +void test_container_plan_reports_roles() { + const auto dir_for = [](const std::string& tag) { + fs::path d = fs::temp_directory_path() / ("plan-roles-" + tag + "-" + std::to_string(::getpid())); + fs::remove_all(d); + fs::create_directories(d); + put_file((d / "config.json").string(), + "{\"hidden_size\": 64, \"num_hidden_layers\": 2, \"num_attention_heads\": 4," + " \"num_key_value_heads\": 2, \"vocab_size\": 128, \"intermediate_size\": 128," + " \"model_type\": \"qwen3\", \"architectures\": [\"Qwen3ForCausalLM\"]}"); + put_file((d / "tokenizer.json").string(), "{}"); + return d; + }; + const std::vector known = { + "model.embed_tokens.weight", + "model.layer.0.input_layernorm.weight", + "model.layer.0.self_attn.q_proj.weight", + "model.layer.0.self_attn.k_proj.weight", + "model.layer.0.self_attn.v_proj.weight", + "model.layer.0.self_attn.o_proj.weight", + "model.layer.0.moe_router.weight", + "model.layer.0.mlp.gate_exps_proj.weight", + "model.layer.0.mlp.share_up_exps_proj.weight", + "model.norm.weight"}; + { + const fs::path d = dir_for("known"); + write_container((d / "model.q4nx").string(), known); + const pi::Plan p = pi::plan(d.string()); + check(p.container_tensors == int64_t(known.size()), "plan: the container's tensor count reaches the plan"); + check(p.container_roles_unassigned.empty(), "plan: every name of a container it accepts is placed (the F1 acceptance)"); + check(int64_t(p.container_role_counts.size()) > 0, "plan: the role counts reach the plan"); + fs::remove_all(d); + } + { + const fs::path d = dir_for("unknown"); + std::vector with_unknown = known; + with_unknown.push_back("model.layer.0.mystery_proj.weight"); + write_container((d / "model.q4nx").string(), with_unknown); + const pi::Plan p = pi::plan(d.string()); + check(p.container_roles_unassigned.size() == 1 && + p.container_roles_unassigned.front() == "model.layer.0.mystery_proj.weight", + "plan: a name nothing knows is listed as unassigned, by name"); + fs::remove_all(d); + } +} + void test_container_roles() { const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; return x; }; const auto roles = [&](std::initializer_list names) { @@ -6178,6 +6244,7 @@ int main(int argc, char** argv) { test_q4nx_container(golden_dir.empty() ? "." : golden_dir); test_container_geometry(); test_container_roles(); + test_container_plan_reports_roles(); test_container_roles_moe_and_singular_layer(); test_container_plan(golden_dir.empty() ? "." : golden_dir); test_hybrid_pack_import(golden_dir.empty() ? "." : golden_dir); From b0d268e79415f79f1d0dbf563664ce9f668292d0 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 17:32:45 -0400 Subject: [PATCH 03/10] npu-pack: a container pack refuses import by name, not with 'cannot open model.onnx' A container-genre pack has no model.onnx by design, so import_pack fell through to the ONNX reader and reported a missing file about a pack that has no graph at all -- the exact diagnostic the plan() comment says the genre detection exists to avoid. Refused here the way the external-data genre already is: the message names the tensor count, how many names nothing knows, and the missing piece (the container-genre writer, engine#1786 item F1), and points at `npu-pack info` for what the container holds. Measured on MiniCPM-V-4.7-35B-A3B-NPU2: 733 tensor(s), 0 name(s) nothing knows -- so the reader half of that genre is finished and only the writer is missing. --- npu/pack_import.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index 49549716b..ab8fdedb7 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -4718,6 +4718,17 @@ Report import_pack(const std::string& pack_dir, const std::string& out_dir, cons // Census yes, writer no (see the note where graph_weights is decided): this genre's weights are 1-D external-data // blocks, and until the repacker exists an import would write a container nothing can read. Refused here rather // than in plan(), because `info` must keep reporting the genre it can now see. + // engine#1786 (item F1) / engine#1788: a container-genre pack has no `model.onnx` by design, so without a branch here + // the first byte fails with "cannot open /model.onnx" -- a missing-file message about a pack that has no graph + // at all. Refused by name, the same way the external-data genre below is, until the container-genre writer exists; + // `info` keeps reporting what the container holds, which is how the roles get to anyone building that writer. + if (p.container_tensors >= 0) { + throw std::runtime_error( + "this pack is a safetensors container (model.q4nx): " + std::to_string(p.container_tensors) + + " tensor(s), " + std::to_string(p.container_roles_unassigned.size()) + + " name(s) nothing knows; importing it needs the container-genre writer (engine#1786, item F1), which the " + "importer does not have yet. `1bit npu-pack info` reports what the container holds"); + } if (p.graph_packed_siblings > 0) { // The genre has a writer now (engine#1503): decode -> convert -> split by role -> the same I8 tile chunks, // with its own function because import_pack is built around the .fconst envelope. From e632a02c53a0d24260d68693cf99e2ed23b991f0 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 17:51:05 -0400 Subject: [PATCH 04/10] npu-pack: an attention output gate is not the FFN gate (engine#1788) MiniCPM-V 4.7 sets `attn_output_gate: true`, so every layer carries `self_attn.gate_proj.weight` -- and every one of them was filed under the dense-MLP role `gate_proj`, because that needle matches the substring. The forward already keeps them apart (`model.h`: `gate_proj_weight` vs `self_attn_gate_proj_weight`), so this uses its name and puts the rule ahead of the `gate_proj` needle. Measured on both packs on this box: MiniCPM-V-4.7-35B-A3B: gate_proj 30 -> 0, self_attn_gate_proj 30 (that pack has no dense gate at all) MiniCPM-V-4.7-1B: gate_proj 42 -> 24 real + 18 self_attn_gate_proj 733/733 and 357/357 still named, 0 unassigned; suite green. --- npu/pack_import.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index ab8fdedb7..eae02fb00 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3443,6 +3443,12 @@ ContainerRoles container_roles(const std::vector& tensors) { {"k_proj", "k_proj"}, {"k_proj", "attn_k"}, {"k_proj", "wk."}, {"v_proj", "v_proj"}, {"v_proj", "attn_v"}, {"v_proj", "wv."}, {"o_proj", "o_proj"}, {"o_proj", "attn_output"}, {"o_proj", "wo."}, + // engine#1788: an ATTENTION output gate is not the FFN gate. MiniCPM-V 4.7 carries `attn_output_gate: true`, + // so every one of its 30 `self_attn.gate_proj.weight` tensors was filed under `gate_proj` -- the dense-MLP role + // -- and a consumer would have wired an attention gate into the FFN path. The forward already keeps them apart + // (model.h: `gate_proj_weight` vs `self_attn_gate_proj_weight`), so this uses its name and must precede the + // `gate_proj` needle. + {"self_attn_gate_proj", "self_attn.gate_proj"}, {"gate_proj", "gate_proj"}, {"gate_proj", "ffn_gate"}, {"up_proj", "up_proj"}, {"up_proj", "ffn_up"}, {"down_proj", "down_proj"}, {"down_proj", "ffn_down"}, From f3a5146324776a77bea32ac950ed775351039c40 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 18:12:02 -0400 Subject: [PATCH 05/10] npu-pack: a role names a forward field, and two roles may not name the same one (engine#1788, 3c step 2) The writer's second step is role -> destination tensor, and the role vocabulary is not a safe key on its own: the gate_proj defect fixed in the previous commit is exactly one needle serving two different tensors, and a container can carry both. So the mapping is a table with a refusal rather than a switch the writer may get wrong: - container_role_field(role) names the forward field the role fills (npu/forward/include/model.h), or "" when it is not a per-layer weight the loader reads -- fused pairs (qkv_proj, gate_up_proj), and ssm/mtp/vision, which are their own containers or their own code. - container_role_field_conflicts(counts) returns every destination claimed by more than one role, naming all of them. Arms: the two gate senses are different fields; a fused pair and an unknown role say ""; and a table-integrity arm runs every known role at once and requires no conflict -- mutation-checked by pointing up_exps at gate_exps_weight, which fails naming both roles and the field. --- npu/pack_import.cpp | 39 ++++++++++++++++++++++++++++++++++ npu/pack_import.h | 12 +++++++++++ tests/npu_pack_import_test.cpp | 27 +++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index eae02fb00..34fff4b88 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3423,6 +3423,45 @@ std::string header_witness_refusal(int vote_hdrs, bool declared_qk) { // engine#1786 (item F1), slice 3a: roles from names. The rule is ordered -- the first match wins -- so `q_proj.bias` // classifies as a bias of q_proj rather than as an unknown tensor, and a name that matches nothing lands in `unassigned` // with its own name rather than being guessed from its shape. +// engine#1788 / slice 3c step 2: the role -> forward-field table, and the refusal that makes it a check. +const char* container_role_field(const std::string& role) { + static const std::pair kMap[] = { + {"embed", "embed_table"}, {"lm_head", "lm_head_weight"}, + {"input_norm", "input_layernorm_weight"}, {"post_norm", "post_attention_layernorm_weight"}, + {"q_norm", "q_norm_weight"}, {"k_norm", "k_norm_weight"}, + {"q_proj", "q_proj_weight"}, {"k_proj", "k_proj_weight"}, + {"v_proj", "v_proj_weight"}, {"o_proj", "o_proj_weight"}, + {"gate_proj", "gate_proj_weight"}, {"up_proj", "up_proj_weight"}, + {"down_proj", "down_proj_weight"}, {"self_attn_gate_proj", "self_attn_gate_proj_weight"}, + {"up_exps", "up_exps_weight"}, {"gate_exps", "gate_exps_weight"}, + {"down_exps", "down_exps_weight"}, {"share_up_exps", "share_up_exps_weight"}, + {"share_gate_exps", "share_gate_exps_weight"}, {"share_down_exps", "share_down_exps_weight"}, + {"moe_router", "moe_router_weight"}, {"shared_expert_gate", "shared_expert_gate_weight"}, + {"shortconv_in_proj", "shortconv_in_proj_weight"}, {"shortconv_conv", "shortconv_conv_weight"}, + {"shortconv_out_proj", "shortconv_out_proj_weight"}, + }; + for (const auto& kv : kMap) + if (role == kv.first) return kv.second; + return ""; +} + +std::vector container_role_field_conflicts(const std::map& counts) { + std::map> by_field; + for (const auto& kv : counts) { + const char* f = container_role_field(kv.first); + if (f == nullptr || *f == '\0') continue; + by_field[f].push_back(kv.first); + } + std::vector out; + for (const auto& kv : by_field) + if (kv.second.size() > 1) { + std::string s = std::string(kv.first) + " <- "; + for (size_t i = 0; i < kv.second.size(); ++i) s += (i ? ", " : "") + kv.second[i]; + out.push_back(s); + } + return out; +} + ContainerRoles container_roles(const std::vector& tensors) { struct Rule { const char* role; const char* needle; }; // Longest/most specific spellings first: `qkv_proj` before `q_proj`, `gate_up_proj` before `gate_proj`. diff --git a/npu/pack_import.h b/npu/pack_import.h index 63ece2c2e..a7bf53276 100644 --- a/npu/pack_import.h +++ b/npu/pack_import.h @@ -395,6 +395,18 @@ struct ContainerRoles { }; ContainerRoles container_roles(const std::vector& tensors); +// engine#1786 (item F1) / engine#1788, slice 3c step 2: what each role fills in the forward, as a CHECK rather than a +// mapping the writer can quietly get wrong. The role vocabulary is not a safe key on its own -- `gate_proj` and +// `self_attn_gate_proj` are different tensors that one needle used to share, and a container can carry both -- so two +// roles that name the same destination field are refused instead of being written in some order that happens to work. +// Returns the forward's field name (npu/forward/include/model.h), or "" when the role is not a per-layer weight the +// forward's loader reads: fused pairs (`qkv_proj`, `gate_up_proj`) are consumed by a fused path, `ssm`/`mtp`/`vision` +// are their own containers or their own code, and `shared_expert_gate` is read only by the MoE block. +const char* container_role_field(const std::string& role); +// The destination fields claimed by more than one role in `counts` (each with the roles that claim it). Empty is the +// only state a writer may start from. +std::vector container_role_field_conflicts(const std::map& counts); + // Reads the pack (model.onnx, its external data, both .fconst headers, config.json). // Throws only when the pack is structurally unreadable. // engine#1889 (review of #2063): plan()'s wide-q branch (q_out != hidden), as one function so its effect on the plan is diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 0633dfb4e..d2f03a840 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -5204,6 +5204,32 @@ void test_container_plan_reports_roles() { } } +// engine#1788 / slice 3c step 2: the role -> forward-field check. The defect this exists for is that `gate_proj` and +// `self_attn_gate_proj` are different tensors one needle used to share; the table must keep them apart, and no two roles +// may ever name the same destination field. +void test_container_role_fields() { + check(std::string(pi::container_role_field("gate_proj")) == "gate_proj_weight" && + std::string(pi::container_role_field("self_attn_gate_proj")) == "self_attn_gate_proj_weight", + "role fields: the FFN gate and the attention gate are different fields"); + check(pi::container_role_field("qkv_proj") == std::string(""), + "role fields: a fused pair is not one field (the writer must handle it, not guess)"); + check(pi::container_role_field("ssm") == std::string("") && pi::container_role_field("no_such_role") == std::string(""), + "role fields: an unmapped role says so instead of defaulting"); + + // table integrity: every role the table knows, in one counts map, must produce no conflict. This is the arm that + // fails if someone later adds a second role pointing at a field that is already taken. + const std::vector all = { + "embed", "lm_head", "input_norm", "post_norm", "q_norm", "k_norm", "q_proj", "k_proj", "v_proj", "o_proj", + "gate_proj", "up_proj", "down_proj", "self_attn_gate_proj", "up_exps", "gate_exps", "down_exps", + "share_up_exps", "share_gate_exps", "share_down_exps", "moe_router", "shared_expert_gate", + "shortconv_in_proj", "shortconv_conv", "shortconv_out_proj"}; + std::map counts; + for (const char* r : all) counts[r] = 1; + const std::vector conflicts = pi::container_role_field_conflicts(counts); + check(conflicts.empty(), "role fields: no two roles claim the same destination field" + + (conflicts.empty() ? std::string() : " (" + conflicts.front() + ")")); +} + void test_container_roles() { const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; return x; }; const auto roles = [&](std::initializer_list names) { @@ -6244,6 +6270,7 @@ int main(int argc, char** argv) { test_q4nx_container(golden_dir.empty() ? "." : golden_dir); test_container_geometry(); test_container_roles(); + test_container_role_fields(); test_container_plan_reports_roles(); test_container_roles_moe_and_singular_layer(); test_container_plan(golden_dir.empty() ? "." : golden_dir); From fe14927874674447a6d7921e08205d5f2521c934 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 19:54:34 -0400 Subject: [PATCH 06/10] npu-pack: report a container's I8 payload widths, and refuse only what cannot be a payload (engine#1788, 3c) The row footprint of this genre is `2*K + 640` bytes, which the writer needs and which decides copy-vs-re-tile. My first version judged each row against the widths the geometry declares and was WRONG on both packs on this box: both containers' `lm_head` implies K = 4032, a width neither model's config contains, and 37 of the 1B's rows with it. So the check now reports the distribution as fact and refuses only the unambiguous case: - `container_payload_rows` row_bytes -> count, in `info`: 35B {4736: 370, 8704: 1}, 1B {4736: 150, 8704: 37} -- two widths per container, the second being the head. - `container_payload_anomalies` only rows shorter than the 640-byte overhead or with an odd remainder, reported by name with implied_k = -1. Arms: the distribution, the malformed pair, and that a BF16 table is not an I8 row. Full suite green. --- npu/pack_import.cpp | 27 +++++++++++++++++++++++++++ npu/pack_import.h | 24 ++++++++++++++++++++++++ tests/npu_pack_import_test.cpp | 31 +++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index 34fff4b88..98b59675e 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3313,6 +3313,13 @@ std::string Plan::manifest() const { {"unassigned", unnamed}, {"counts", container_role_counts}}; if (!container_roles_unassigned.empty()) c["roles"]["unassigned_names"] = container_roles_unassigned; + c["payload_rows"] = container_payload_rows; + if (!container_payload_anomalies.empty()) { + json a = json::array(); + for (const auto& x : container_payload_anomalies) + a.push_back({{"name", x.name}, {"row_bytes", x.row_bytes}}); + c["payload_anomalies"] = a; + } if (container_vision_tensors >= 0) c["vision"] = {{"tensors", container_vision_tensors}, {"payload_bytes", container_vision_payload_bytes}, @@ -3445,6 +3452,23 @@ const char* container_role_field(const std::string& role) { return ""; } +std::map container_payload_rows(const std::vector& tensors) { + std::map out; + for (const auto& t : tensors) + if (t.dtype == "I8" && t.shape.size() == 3) out[t.shape[2]]++; + return out; +} + +std::vector container_payload_anomalies(const std::vector& tensors) { + std::vector out; + for (const auto& t : tensors) { + if (t.dtype != "I8" || t.shape.size() != 3) continue; + const int64_t row_bytes = t.shape[2]; + if (row_bytes < 640 || (row_bytes - 640) % 2 != 0) out.push_back({t.name, row_bytes, -1}); + } + return out; +} + std::vector container_role_field_conflicts(const std::map& counts) { std::map> by_field; for (const auto& kv : counts) { @@ -3691,6 +3715,9 @@ Plan plan(const std::string& pack_dir) { for (const auto& kv : vr.counts) p.container_vision_role_counts[kv.first] = int64_t(kv.second); } } + // engine#1788, 3c: what the writer needs to know about the payload, as fact rather than judgement. + p.container_payload_rows = container_payload_rows(c.tensors); + p.container_payload_anomalies = container_payload_anomalies(c.tensors); p.container_note = c.why.empty() ? "the pack is a safetensors container (model.q4nx): " + std::to_string(c.tensors.size()) + " tensors, " + std::to_string(c.payload_bytes) + " B of payload, no graph; " + geom + diff --git a/npu/pack_import.h b/npu/pack_import.h index a7bf53276..5acba5a41 100644 --- a/npu/pack_import.h +++ b/npu/pack_import.h @@ -199,6 +199,11 @@ int64_t read_per_layer_table_band(const EmbedPerLayer& meta, int64_t first_row, std::string* why); // One tensor the pack describes, and where its values would come from. +struct ContainerPayloadAnomaly { + std::string name; + int64_t row_bytes = 0; + int64_t implied_k = 0; +}; struct Planned { std::string name; // Q4NX tensor name (model.layers.N.<...>.weight) std::string source; // "model.onnx.data", "fconst:Llm_Token_Token", "fconst:Llm_Prefill", "config.json" @@ -274,6 +279,10 @@ struct Plan { int64_t container_vision_payload_bytes = 0; int64_t container_vision_roles_named = -1; std::map container_vision_role_counts; + // engine#1788, 3c: every I8 row whose implied K is none of the widths the geometry declares. Reported for the + // writer's benefit -- a row that implies a K the model does not have is a re-tile, not a copy. + std::map container_payload_rows; // row_bytes -> how many I8 tensors use it + std::vector container_payload_anomalies; // engine#1786 (item F1), slice 2: WHERE the container's geometry came from, and what it could not reach. // A bare number in `dims` cannot say whether the pack declared it or a file beside it did, and those two are free to // disagree (13 of the 29 packs measured carry a `dims` block; all 29 carry config.json). Empty for a non-container pack. @@ -403,6 +412,21 @@ ContainerRoles container_roles(const std::vector& tensors); // forward's loader reads: fused pairs (`qkv_proj`, `gate_up_proj`) are consumed by a fused path, `ssm`/`mtp`/`vision` // are their own containers or their own code, and `shared_expert_gate` is read only by the MoE block. const char* container_role_field(const std::string& role); +// engine#1788, slice 3c step 3: does a container tensor's payload agree with the geometry the pack declares? An I8 row +// in this genre is `2*K + 640` bytes (K the projection's input width, 640 the fixed per-group scale/zero overhead) -- +// measured on both MiniCPM-V packs on this box (35B: uniform 4736 with K = H = 2048; 1B: 4736 and 8704, K = 2048/4032). +// A row that implies a K the model's geometry does not contain is a re-tile the writer must not copy blind, so it is +// reported by name rather than written. + +// `plausible_k` is NOT used to judge: running it against the two packs on this box produced false positives (both +// containers' `lm_head` rows imply K = 4032, a width no geometry field of either model declares, and 37 of the 1B's +// rows with it). So the check reports what it can prove and stops there: +// - `container_payload_rows` the distribution of row widths, which is the fact a writer needs (one width = a copy, +// several = a re-tile per shape); +// - `container_payload_anomalies` only the rows that cannot be one of these at all (shorter than the 640-byte +// overhead, or an odd remainder), with implied_k = -1. +std::map container_payload_rows(const std::vector& tensors); +std::vector container_payload_anomalies(const std::vector& tensors); // The destination fields claimed by more than one role in `counts` (each with the roles that claim it). Empty is the // only state a writer may start from. std::vector container_role_field_conflicts(const std::map& counts); diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index d2f03a840..304942955 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -5230,6 +5230,36 @@ void test_container_role_fields() { (conflicts.empty() ? std::string() : " (" + conflicts.front() + ")")); } +// engine#1788 / 3c step 3: what a container's I8 payload looks like, as fact. Measured on both packs on this box: +// 35B {4736: 370, 8704: 1}, 1B {4736: 150, 8704: 37} -- two widths per container, the second being the head. The earlier +// version of this check judged each row against the geometry's widths and produced false positives (both containers' +// `lm_head` implies K = 4032, which neither model declares), so it reports the distribution and only *refuses* the rows +// that cannot be a payload at all. +void test_container_payload_rule() { + const auto t = [](const std::string& n, std::vector shape) { + pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; x.shape = std::move(shape); return x; + }; + { + const std::vector v = {t("a", {4096, 8, 4736}), t("b", {16384, 2, 4736}), + t("lm_head.weight", {7755, 8, 8704})}; + const std::map rows = pi::container_payload_rows(v); + check(rows.size() == 2 && rows.at(4736) == 2 && rows.at(8704) == 1, + "payload rows: the distribution is reported, so a writer can see one width or several"); + check(pi::container_payload_anomalies(v).empty(), "payload rows: ordinary rows are not anomalies"); + } + { + const std::vector v = {t("short", {4, 2, 512}), t("odd", {4, 2, 4737})}; + const std::vector a = pi::container_payload_anomalies(v); + check(a.size() == 2 && a[0].name == "short" && a[0].implied_k == -1, + "payload rows: a row below the 640-byte overhead, or with an odd remainder, is reported"); + } + { + pi::Q4nxTensor bf16; bf16.name = "model.embed_tokens.weight"; bf16.dtype = "BF16"; bf16.shape = {248144, 2048}; + check(pi::container_payload_rows({bf16}).empty() && pi::container_payload_anomalies({bf16}).empty(), + "payload rows: a BF16 table is not an I8 row"); + } +} + void test_container_roles() { const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; return x; }; const auto roles = [&](std::initializer_list names) { @@ -6270,6 +6300,7 @@ int main(int argc, char** argv) { test_q4nx_container(golden_dir.empty() ? "." : golden_dir); test_container_geometry(); test_container_roles(); + test_container_payload_rule(); test_container_role_fields(); test_container_plan_reports_roles(); test_container_roles_moe_and_singular_layer(); From 4022807a367c74be1a2fe9b7c89187405b27a6d9 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 21:52:47 -0400 Subject: [PATCH 07/10] npu-pack: report the shapes each role appears in, which is the writer's input (engine#1788, 3c) After the plausible-K attempt produced false positives, this reports the fact the writer needs rather than judging it: for every role, the distinct (shape, dtype) forms it appears in, with counts. On MiniCPM-V 4.7 35B: q_proj [256,8,4736] I8 x10 qkv_proj [256,8,4736] I8 x30 gate_exps [4096,8,4736] I8 x40 down_exps [16384,2,4736] I8 x40 moe_router [2048,256] BF16 x40 embed [248144,2048] BF16 x1 lm_head [7755,8,8704] I8 x1 final_norm [2048] BF16 x1 Every role carries exactly ONE shape, which is what makes the writer a mapping rather than a per-shape decision -- and a role that ever appears in two shapes is reported as two, because that is where the writer must decide rather than copy. Arms: count-per-shape, the dtype in the shape text (a BF16 table is not an I8 row), a two-shape role, and empty input. Full suite green. --- npu/pack_import.cpp | 20 ++++++++++++++++++++ npu/pack_import.h | 7 +++++++ tests/npu_pack_import_test.cpp | 30 ++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index 98b59675e..de91131f0 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3314,6 +3314,7 @@ std::string Plan::manifest() const { {"counts", container_role_counts}}; if (!container_roles_unassigned.empty()) c["roles"]["unassigned_names"] = container_roles_unassigned; c["payload_rows"] = container_payload_rows; + c["role_shapes"] = container_role_shapes; if (!container_payload_anomalies.empty()) { json a = json::array(); for (const auto& x : container_payload_anomalies) @@ -3459,6 +3460,24 @@ std::map container_payload_rows(const std::vector& return out; } +std::map> container_role_shapes(const std::vector& tensors) { + std::map> out; + const ContainerRoles roles = container_roles(tensors); + std::map> by_name; + for (const auto& t : tensors) { + std::string s = "["; + for (size_t i = 0; i < t.shape.size(); ++i) s += (i ? "," : "") + std::to_string(t.shape[i]); + s += "]"; + by_name[t.name][s + " " + t.dtype]++; + } + for (const auto& kv : roles.role_of) { + const auto it = by_name.find(kv.first); + if (it == by_name.end()) continue; + for (const auto& shape : it->second) out[kv.second][shape.first] += shape.second; + } + return out; +} + std::vector container_payload_anomalies(const std::vector& tensors) { std::vector out; for (const auto& t : tensors) { @@ -3717,6 +3736,7 @@ Plan plan(const std::string& pack_dir) { } // engine#1788, 3c: what the writer needs to know about the payload, as fact rather than judgement. p.container_payload_rows = container_payload_rows(c.tensors); + p.container_role_shapes = container_role_shapes(c.tensors); p.container_payload_anomalies = container_payload_anomalies(c.tensors); p.container_note = c.why.empty() ? "the pack is a safetensors container (model.q4nx): " + std::to_string(c.tensors.size()) + diff --git a/npu/pack_import.h b/npu/pack_import.h index 5acba5a41..adc5ac36b 100644 --- a/npu/pack_import.h +++ b/npu/pack_import.h @@ -282,6 +282,8 @@ struct Plan { // engine#1788, 3c: every I8 row whose implied K is none of the widths the geometry declares. Reported for the // writer's benefit -- a row that implies a K the model does not have is a re-tile, not a copy. std::map container_payload_rows; // row_bytes -> how many I8 tensors use it + // engine#1788, 3c: per role, the shapes the container carries -- the fact the writer's shape matching needs. + std::map> container_role_shapes; std::vector container_payload_anomalies; // engine#1786 (item F1), slice 2: WHERE the container's geometry came from, and what it could not reach. // A bare number in `dims` cannot say whether the pack declared it or a file beside it did, and those two are free to @@ -426,6 +428,11 @@ const char* container_role_field(const std::string& role); // - `container_payload_anomalies` only the rows that cannot be one of these at all (shorter than the 640-byte // overhead, or an odd remainder), with implied_k = -1. std::map container_payload_rows(const std::vector& tensors); +// engine#1788, slice 3c step 4: per role, the shapes a container actually carries. The writer's job is "for this +// destination field, are the source shapes what that field holds?", and after the plausible-K attempt produced false +// positives on both packs on this box, the useful thing to hand it is the FACT: which shapes each role appears in. +// A role with one shape is a mapping; a role with several is a decision the writer must make per shape. +std::map> container_role_shapes(const std::vector& tensors); std::vector container_payload_anomalies(const std::vector& tensors); // The destination fields claimed by more than one role in `counts` (each with the roles that claim it). Empty is the // only state a writer may start from. diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 304942955..14e066c2b 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -5260,6 +5260,35 @@ void test_container_payload_rule() { } } +// engine#1788 / 3c step 4: per role, the shapes a container carries. Measured on the 35B: every role has exactly ONE +// shape (q_proj [256,8,4736] x10, qkv_proj [256,8,4736] x30, gate_exps [4096,8,4736] x40, moe_router [2048,256] BF16 +// x40, ...), which is what makes the writer a mapping rather than a per-shape decision. A role that appears in two +// shapes must be visible as two, not averaged. +void test_container_role_shapes() { + const auto t = [](const std::string& n, std::vector shape, const char* dt) { + pi::Q4nxTensor x; x.name = n; x.dtype = dt; x.shape = std::move(shape); return x; + }; + { + const std::vector v = { + t("model.layers.0.self_attn.q_proj.weight", {256, 8, 4736}, "I8"), + t("model.layers.1.self_attn.q_proj.weight", {256, 8, 4736}, "I8"), + t("model.layers.0.moe_router.weight", {2048, 256}, "BF16")}; + const auto rs = pi::container_role_shapes(v); + check(rs.count("q_proj") && rs.at("q_proj").size() == 1 && rs.at("q_proj").begin()->second == 2, + "role shapes: two tensors of one role and one shape are one entry with a count"); + check(rs.count("moe_router") && rs.at("moe_router").begin()->first == "[2048,256] BF16", + "role shapes: the shape text carries the dtype, so a BF16 table cannot be read as an I8 row"); + } + { + // a role in two shapes must report both: that is the case the writer has to decide about + const std::vector v = { + t("a.q_proj.weight", {256, 8, 4736}, "I8"), t("b.q_proj.weight", {128, 8, 4736}, "I8")}; + const auto rs = pi::container_role_shapes(v); + check(rs.at("q_proj").size() == 2, "role shapes: a role carried in two shapes reports both, not an average"); + } + check(pi::container_role_shapes({}).empty(), "role shapes: no tensors, no roles"); +} + void test_container_roles() { const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; return x; }; const auto roles = [&](std::initializer_list names) { @@ -6300,6 +6329,7 @@ int main(int argc, char** argv) { test_q4nx_container(golden_dir.empty() ? "." : golden_dir); test_container_geometry(); test_container_roles(); + test_container_role_shapes(); test_container_payload_rule(); test_container_role_fields(); test_container_plan_reports_roles(); From 7c7fa26629da55b4b43610b50bb5d80975022d2d Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 22:44:50 -0400 Subject: [PATCH 08/10] npu-pack: the writer's mapping as a plan it can be wrong about (engine#1788, 3c step 5) For every role that names a forward field: which source tensor fills it, in what shape, and how many tensors carry that role. On MiniCPM-V 4.7 35B that is 19 fields, 0 ambiguous, and `no_source` listing only what a MoE model lacks by design (dense gate/up/down, shortconv) -- so "this model does not have it" and "something is missing" are different lines instead of one. Three refusals rather than guesses: - a role present in TWO shapes is `ambiguous` and is NOT mapped: that is where a writer must decide, and picking the first would have been the plausible-K mistake again; - a fused pair (`qkv_proj`, `gate_up_proj`) is not one destination field, so it is neither an item nor missing; - `source` is a REPRESENTATIVE with `of: N`, because a role carried per layer (k_proj of 10, moe_router of 40) must not read as a single tensor. Arms: per-layer count, two-shape ambiguity, fused-not-a-field, empty input. Full suite green. --- npu/pack_import.cpp | 52 ++++++++++++++++++++++++++++++++++ npu/pack_import.h | 18 ++++++++++++ tests/npu_pack_import_test.cpp | 38 +++++++++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index de91131f0..f3caae522 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3315,6 +3315,17 @@ std::string Plan::manifest() const { if (!container_roles_unassigned.empty()) c["roles"]["unassigned_names"] = container_roles_unassigned; c["payload_rows"] = container_payload_rows; c["role_shapes"] = container_role_shapes; + { + json wp; + json items = json::object(); + for (const auto& it : container_write_plan.items) + items[it.field] = {{"source", it.source}, {"of", it.count}, {"shape", it.shape}}; + wp["items"] = items; + wp["count"] = container_write_plan.items.size(); + if (!container_write_plan.ambiguous.empty()) wp["ambiguous"] = container_write_plan.ambiguous; + if (!container_write_plan.no_source.empty()) wp["no_source"] = container_write_plan.no_source; + c["write_plan"] = wp; + } if (!container_payload_anomalies.empty()) { json a = json::array(); for (const auto& x : container_payload_anomalies) @@ -3460,6 +3471,46 @@ std::map container_payload_rows(const std::vector& return out; } +ContainerWritePlan container_write_plan(const std::vector& tensors) { + ContainerWritePlan out; + const ContainerRoles roles = container_roles(tensors); + // role -> the tensors that carry it + std::map> by_role; + for (const auto& t : tensors) { + const auto it = roles.role_of.find(t.name); + if (it != roles.role_of.end()) by_role[it->second].push_back(&t); + } + const auto shape_text = [](const Q4nxTensor& t) { + std::string s = "["; + for (size_t i = 0; i < t.shape.size(); ++i) s += (i ? "," : "") + std::to_string(t.shape[i]); + return s + "] " + t.dtype; + }; + // every field the table knows, in table order: does a role that names it have a source? + static const char* kRoles[] = { + "embed", "lm_head", "input_norm", "post_norm", "q_norm", "k_norm", "q_proj", "k_proj", "v_proj", "o_proj", + "gate_proj", "up_proj", "down_proj", "self_attn_gate_proj", "up_exps", "gate_exps", "down_exps", + "share_up_exps", "share_gate_exps", "share_down_exps", "moe_router", "shared_expert_gate", + "shortconv_in_proj", "shortconv_conv", "shortconv_out_proj"}; + for (const char* role : kRoles) { + const auto it = by_role.find(role); + if (it == by_role.end()) { + out.no_source.push_back(std::string(container_role_field(role)) + " (role `" + role + "` absent)"); + continue; + } + // distinct shapes for this role + std::map first; + for (const Q4nxTensor* t : it->second) first.emplace(shape_text(*t), t); + if (first.size() > 1) { + out.ambiguous.push_back(std::string(container_role_field(role)) + ": role `" + role + "` appears in " + + std::to_string(first.size()) + " shapes"); + continue; + } + out.items.push_back({container_role_field(role), first.begin()->second->name, first.begin()->first, + int64_t(it->second.size())}); + } + return out; +} + std::map> container_role_shapes(const std::vector& tensors) { std::map> out; const ContainerRoles roles = container_roles(tensors); @@ -3737,6 +3788,7 @@ Plan plan(const std::string& pack_dir) { // engine#1788, 3c: what the writer needs to know about the payload, as fact rather than judgement. p.container_payload_rows = container_payload_rows(c.tensors); p.container_role_shapes = container_role_shapes(c.tensors); + p.container_write_plan = container_write_plan(c.tensors); p.container_payload_anomalies = container_payload_anomalies(c.tensors); p.container_note = c.why.empty() ? "the pack is a safetensors container (model.q4nx): " + std::to_string(c.tensors.size()) + diff --git a/npu/pack_import.h b/npu/pack_import.h index adc5ac36b..3e56e2e2d 100644 --- a/npu/pack_import.h +++ b/npu/pack_import.h @@ -204,6 +204,18 @@ struct ContainerPayloadAnomaly { int64_t row_bytes = 0; int64_t implied_k = 0; }; +struct ContainerWriteItem { + std::string field; // the forward field (npu/forward/include/model.h) + std::string source; // ONE container tensor that fills it -- a representative, not the only one + std::string shape; // its shape+dtype text, as reported by container_role_shapes() + int64_t count = 0; // how many tensors carry this role (e.g. one per layer), so a writer does not read + // `source` as "the only one" +}; +struct ContainerWritePlan { + std::vector items; + std::vector ambiguous; // ": appears in N shapes" + std::vector no_source; // fields the table knows whose role is absent from this pack +}; struct Planned { std::string name; // Q4NX tensor name (model.layers.N.<...>.weight) std::string source; // "model.onnx.data", "fconst:Llm_Token_Token", "fconst:Llm_Prefill", "config.json" @@ -284,6 +296,7 @@ struct Plan { std::map container_payload_rows; // row_bytes -> how many I8 tensors use it // engine#1788, 3c: per role, the shapes the container carries -- the fact the writer's shape matching needs. std::map> container_role_shapes; + ContainerWritePlan container_write_plan; // role -> forward field -> source tensor (slice 3c step 5) std::vector container_payload_anomalies; // engine#1786 (item F1), slice 2: WHERE the container's geometry came from, and what it could not reach. // A bare number in `dims` cannot say whether the pack declared it or a file beside it did, and those two are free to @@ -433,6 +446,11 @@ std::map container_payload_rows(const std::vector& // positives on both packs on this box, the useful thing to hand it is the FACT: which shapes each role appears in. // A role with one shape is a mapping; a role with several is a decision the writer must make per shape. std::map> container_role_shapes(const std::vector& tensors); +// engine#1788, slice 3c step 5: the writer's mapping, as a plan it can be wrong about. For every role that names a +// forward field, which source tensor supplies it and in what shape; a role present in two shapes is AMBIGUOUS (the +// writer must decide, not pick), and a field the table knows whose role is absent is listed separately rather than +// called missing -- whether a model needs `up_proj` or `up_exps` is a per-model question, not a table one. +ContainerWritePlan container_write_plan(const std::vector& tensors); std::vector container_payload_anomalies(const std::vector& tensors); // The destination fields claimed by more than one role in `counts` (each with the roles that claim it). Empty is the // only state a writer may start from. diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 14e066c2b..902d97675 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -5289,6 +5289,43 @@ void test_container_role_shapes() { check(pi::container_role_shapes({}).empty(), "role shapes: no tensors, no roles"); } +// engine#1788 / 3c step 5: the writer's mapping as a plan it can be wrong about. On the 35B: 19 fields mapped, 0 +// ambiguous, and `no_source` correctly lists only what a MoE model lacks by design (dense gate/up/down, shortconv). +void test_container_write_plan() { + const auto t = [](const std::string& n, std::vector shape, const char* dt) { + pi::Q4nxTensor x; x.name = n; x.dtype = dt; x.shape = std::move(shape); return x; + }; + { + const std::vector v = {t("model.layers.0.moe_router.weight", {2048, 256}, "BF16"), + t("model.layers.1.moe_router.weight", {2048, 256}, "BF16")}; + const pi::ContainerWritePlan p = pi::container_write_plan(v); + bool found = false; + for (const auto& it : p.items) + if (it.field == "moe_router_weight") { + found = true; + check(it.count == 2 && it.shape == "[2048,256] BF16", "write plan: a role carried per layer counts them"); + } + check(found, "write plan: the router maps to its forward field"); + check(p.ambiguous.empty(), "write plan: one shape is not ambiguous"); + } + { + // a role in two shapes must be AMBIGUOUS, not silently one of them + const std::vector v = {t("a.moe_router.weight", {2048, 256}, "BF16"), + t("b.moe_router.weight", {1024, 256}, "BF16")}; + const pi::ContainerWritePlan p = pi::container_write_plan(v); + check(p.ambiguous.size() == 1, "write plan: two shapes for one role is reported as ambiguous"); + for (const auto& it : p.items) check(it.field != "moe_router_weight", "write plan: an ambiguous role is not mapped"); + } + { + // a fused pair is not one field, so it is neither an item nor 'missing' + const std::vector v = {t("model.layers.0.attn_qkv.weight", {64, 8, 4736}, "I8")}; + const pi::ContainerWritePlan p = pi::container_write_plan(v); + for (const auto& it : p.items) check(it.field != "qkv_proj", "write plan: a fused pair is not one destination field"); + check(!p.no_source.empty(), "write plan: the fields this pack does not carry are listed separately"); + } + check(pi::container_write_plan({}).items.empty(), "write plan: no tensors, no items"); +} + void test_container_roles() { const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; return x; }; const auto roles = [&](std::initializer_list names) { @@ -6329,6 +6366,7 @@ int main(int argc, char** argv) { test_q4nx_container(golden_dir.empty() ? "." : golden_dir); test_container_geometry(); test_container_roles(); + test_container_write_plan(); test_container_role_shapes(); test_container_payload_rule(); test_container_role_fields(); From aa7ed0afa341798c1a468eb415682615b181cb20 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 23:41:29 -0400 Subject: [PATCH 09/10] npu-pack: recognise the gemma3/gemma4/bolt families this forward does not read (engine#1788) Measured across every container on this box, three packs could not be finished and two of them shared the SAME six families: `pre_/post_feedforward_layernorm`, `inp_gate` (and its `_prefill` sibling), `per_layer_projection` (and its `_prefill`) and `layer_output_scale`. None has a field in npu/forward/include/model.h, so they are RECOGNISED under one role (`gemma_local`) rather than mapped -- `unassigned` has to keep meaning "nothing knows this", and these are known. gemma3-270m-npu2 36 -> 0 unassigned (36 gemma_local) gemma4-e2b-npu2 319 -> 74 (245 gemma_local) bolt-e4b 382 -> 88 (294 gemma_local) MiniCPM-V 35B 733 -> 733 / 0 (unchanged, no family of this kind) The recogniser runs AFTER the rule table, so a norm the forward does read still wins -- the second arm pins exactly that (`post_attention_layernorm` stays `post_norm`, `input_layernorm` stays `input_norm`). What is left on gemma4/bolt is NOT covered on purpose: `model.layers.N.post_layernorm.weight` (35) and `model.per_layer_model_proj.weight_layerN` (35) need a semantic decision -- is gemma4's `post_layernorm` this forward's post-attention norm? -- and guessing it would be the plausible-K mistake again. --- npu/pack_import.cpp | 11 +++++++++++ tests/npu_pack_import_test.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index f3caae522..43b596e39 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3607,6 +3607,13 @@ ContainerRoles container_roles(const std::vector& tensors) { // and `model.mtp.*` is a draft head rather than a model. static const char* kSsm[] = {"linear_attn.", "ssm_alpha_proj", "ssm_beta_proj", "ssm_conv1d", "ssm_dt", "ssm_norm", "ssm_out_proj"}; + // engine#1788: gemma3/gemma4/bolt carry five tensor families this engine's forward does NOT read -- sandwich norms + // (`pre_/post_feedforward_layernorm`), an input gate, a per-layer projection (and its `_prefill` sibling) and a layer + // output scale. Nothing here has a field in npu/forward/include/model.h, so they are RECOGNISED rather than mapped: + // `unassigned` has to keep meaning "nothing knows this", and these are known -- measured on gemma4-e2b (319 of 709) + // and bolt-e4b (382 of 849), which were the two packs the classifier could not finish. + static const char* kGemmaLocal[] = {"pre_feedforward_layernorm", "post_feedforward_layernorm", "inp_gate", + "layer_output_scale", "per_layer_projection"}; static const char* kMtp[] = {"model.mtp."}; ContainerRoles out; for (const auto& t : tensors) { @@ -3634,6 +3641,10 @@ ContainerRoles container_roles(const std::vector& tensors) { for (const char* s : kSsm) if (n.find(s) != std::string::npos) { role = "ssm"; matched = true; break; } } + if (!matched && role.empty()) { + for (const char* g : kGemmaLocal) + if (n.find(g) != std::string::npos) { role = "gemma_local"; matched = true; break; } + } if (!matched && role.empty()) { for (const char* s : kMtp) if (n.find(s) != std::string::npos) { role = "mtp"; matched = true; break; } diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 902d97675..3362ad261 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -5326,6 +5326,32 @@ void test_container_write_plan() { check(pi::container_write_plan({}).items.empty(), "write plan: no tensors, no items"); } +// engine#1788: the gemma3/gemma4/bolt families this forward does NOT read are recognised, not left unassigned -- and the +// rules that DO name a forward field still win, so a sandwich norm cannot shadow the input or post-attention norm. +void test_container_roles_gemma_local() { + const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "BF16"; return x; }; + const auto roles = [&](std::initializer_list names) { + std::vector v; + for (const char* n : names) v.push_back(t(n)); + return pi::container_roles(v); + }; + { + const auto r = roles({"model.layers.0.pre_feedforward_layernorm.weight", + "model.layers.0.inp_gate.weight", "model.layers.0.inp_gate.weight_prefill", + "model.layers.0.per_layer_projection.weight", "model.layers.0.layer_output_scale.weight"}); + check(r.unassigned.empty(), "gemma local: the families this forward does not read are recognised"); + check(r.counts.at("gemma_local") == 5, "gemma local: all five land on one role"); + } + { + // the forward's own norms must still win: this is the shadowing the ordering could cause + const auto r = roles({"model.layers.0.post_attention_layernorm.weight", + "model.layers.0.input_layernorm.weight"}); + check(r.role_of.at("model.layers.0.post_attention_layernorm.weight") == "post_norm" && + r.role_of.at("model.layers.0.input_layernorm.weight") == "input_norm", + "gemma local: a norm the forward reads is not swallowed by the recogniser"); + } +} + void test_container_roles() { const auto t = [](const std::string& n) { pi::Q4nxTensor x; x.name = n; x.dtype = "I8"; return x; }; const auto roles = [&](std::initializer_list names) { @@ -6366,6 +6392,7 @@ int main(int argc, char** argv) { test_q4nx_container(golden_dir.empty() ? "." : golden_dir); test_container_geometry(); test_container_roles(); + test_container_roles_gemma_local(); test_container_write_plan(); test_container_role_shapes(); test_container_payload_rule(); From 24ded5a64b733204df5bafe36acb3e4a61c08ae6 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Thu, 12 Nov 2026 00:02:10 -0400 Subject: [PATCH 10/10] npu-pack: the last three families on gemma4/bolt, and every container on the box now classifies completely `post_layernorm` and `per_layer_model_proj` are gemma-local for a MEASURED reason: gemma4-e2b carries BOTH `post_attention_layernorm` (35, classified post_norm) and `post_layernorm` (35), so the second cannot be the forward's post-attention norm; gemma3-270m has no `post_layernorm` at all, which is why only gemma4/bolt were left with it. `rope_freqs` and `per_layer_token_embd` (+ its `.scale`) get their own recognised role, `container_extra`, because they are known for a different reason: the forward only FLAGS a rope-freqs file (model_config.h has_rope_freqs_file) and the per-layer token embedding is read by our ONNX path as `embed_tokens_per_layer` (engine#1725) with no container-path field yet. Recognised, not mapped -- and not "unassigned", which would say nothing knows them. Result, every container on this box: gemma3-270m 237 / 0 gemma4-e2b 709 / 0 bolt-e4b 849 / 0 MiniCPM-V-35B 733 / 0 MiniCPM-V-1B 357 / 0 Suite green. --- npu/pack_import.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index 43b596e39..2946c2e1d 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -3612,8 +3612,19 @@ ContainerRoles container_roles(const std::vector& tensors) { // output scale. Nothing here has a field in npu/forward/include/model.h, so they are RECOGNISED rather than mapped: // `unassigned` has to keep meaning "nothing knows this", and these are known -- measured on gemma4-e2b (319 of 709) // and bolt-e4b (382 of 849), which were the two packs the classifier could not finish. + // `post_layernorm` and `per_layer_model_proj` are here for a measured reason, not a guess: gemma4-e2b carries + // BOTH `post_attention_layernorm` (35, classified `post_norm`) and `post_layernorm` (35), so the second cannot be + // the forward's post-attention norm -- they are different tensors in the same model. gemma3-270m has no + // `post_layernorm` at all, which is why only gemma4/bolt were left with it. static const char* kGemmaLocal[] = {"pre_feedforward_layernorm", "post_feedforward_layernorm", "inp_gate", - "layer_output_scale", "per_layer_projection"}; + "layer_output_scale", "per_layer_projection", "post_layernorm", + "per_layer_model_proj"}; + // engine#1788: the last three on gemma4/bolt are recognised for a different reason than the gemma ones, so they get + // their own role. `rope_freqs.weight` is a file the forward only FLAGS (`model_config.h`: has_rope_freqs_file), and + // `per_layer_token_embd` (with its `.scale` sibling) is the per-layer token embedding that our ONNX path reads as + // `embed_tokens_per_layer` (engine#1725) but the container path has no field for. Recognised, not mapped: neither + // has a field to fill, and calling them "unassigned" would say nothing knows them when the opposite is true. + static const char* kContainerExtras[] = {"rope_freqs", "per_layer_token_embd"}; static const char* kMtp[] = {"model.mtp."}; ContainerRoles out; for (const auto& t : tensors) { @@ -3641,6 +3652,10 @@ ContainerRoles container_roles(const std::vector& tensors) { for (const char* s : kSsm) if (n.find(s) != std::string::npos) { role = "ssm"; matched = true; break; } } + if (!matched && role.empty()) { + for (const char* e : kContainerExtras) + if (n.find(e) != std::string::npos) { role = "container_extra"; matched = true; break; } + } if (!matched && role.empty()) { for (const char* g : kGemmaLocal) if (n.find(g) != std::string::npos) { role = "gemma_local"; matched = true; break; }