From 452f304cef45203ef68affbdc41d82c989afcf99 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 10 Nov 2026 06:50:39 -0400 Subject: [PATCH] npu-pack: read the pack's use_qk_norm declaration and refuse by name when the gains are missing (#1889) Hunyuan-7B-Instruct states `use_qk_norm: true` in config.json, and the importer decided qk_norm by layout arithmetic (pack_import.cpp:2846) plus an ONNX-graph check -- neither matched that pack's block, so the flag landed on false and the import reported attention 5120/5120 tiles and MLP 9216/9216 groups byte-exact. The forward then normalised nothing and the model answered ' Trease/ft////////////////////' deterministically, which reads like a decode problem rather than a missing normalisation. grep -r use_qk_norm npu/ tools/ returned nothing: the declaration was never read. This takes the first of the three steps #1889 lists, the one that turns silence into a named refusal: declared_qk_norm(pack_dir) reads use_qk_norm / qk_norm, top level or under text_config, from config.json or model_config.json -- exported so it can be pinned by a test plan() when the pack declares it and NEITHER source found the gains, throws: "this pack declares use_qk_norm: true, so its per-head q/k RMSNorm gains must be applied, but they were found in neither place this importer looks: ..." and says why importing without them is worse than refusing (byte-exact weights, quiet nonsense) Not taken here, deliberately: relocating the gains by measurement (steps 2-3 of the issue). That is the tail/envelope area another lane owns (#1740 W1a/W1b), and a wrong relocation mis-rotates every Hunyuan pack silently -- the same class of failure this commit is trying to end. Tested: npu_pack_import_test gains test_declared_qk_norm, which pins four cases (top-level true, true under text_config, explicit false, absent) against crafted configs; the suite reports "qk norm declaration: top-level, text_config, false and absent all read" and "all npu-pack checks passed". The refusal path itself needs a real declaring pack to exercise -- the Hunyuan-7B pack is a 9.3 GB download, so it is verified by construction (one `if` around an exported predicate that is tested) rather than by a device run; the issue records what the pack does today. --- npu/pack_import.cpp | 31 +++++++++++++++++++++++++++++++ npu/pack_import.h | 4 ++++ tests/npu_pack_import_test.cpp | 20 ++++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/npu/pack_import.cpp b/npu/pack_import.cpp index 400e4fe29..285438227 100644 --- a/npu/pack_import.cpp +++ b/npu/pack_import.cpp @@ -315,6 +315,24 @@ OnnxModel parse_onnx(const std::string& path) { return m; } +// The pack's own declaration of a per-head q/k RMSNorm (engine#1889). Hunyuan-7B states `use_qk_norm: true` in +// config.json; the block arithmetic below then failed to find the gains, the flag landed on false, the import reported +// byte-exact tiles, and the forward normalised nothing -- so the model answered gibberish with no error anywhere. The +// declaration is read so that case becomes a named refusal instead of a silent omission. +bool pack_declares_qk_norm(const std::string& pack_dir) { + for (const char* fn : {"config.json", "model_config.json"}) { + std::ifstream f(pack_dir + "/" + fn); + if (!f) continue; + json j; + try { j = json::parse(f); } catch (...) { continue; } + const json* c = &j; + if (j.contains("text_config") && j["text_config"].is_object()) c = &j["text_config"]; + for (const char* k : {"use_qk_norm", "qk_norm"}) + if (c->contains(k) && (*c)[k].is_boolean() && (*c)[k].get()) return true; + } + return false; +} + // ── the .fconst files: layer block geometry (observed, byte-exact) ─────────── // amd-oss/npu-re docs/FACTS-oga-pack-weight-layout.md §2.1. Every layer block opens // with bf16 1e-6 (the RMSNorm epsilon, little-endian 86 35), which is what locates a @@ -1862,6 +1880,9 @@ bool fconst_scratch_ready(const std::string& dir) { return fs::is_directory(dir, ec) && !ec; } +// engine#1889: the public spelling of the declaration check above (the internal one lives in the anonymous namespace). +bool declared_qk_norm(const std::string& pack_dir) { return pack_declares_qk_norm(pack_dir); } + std::vector fconst_scratch_dirs() { // Where the canonical .fconst copy may live, in preference order. A function because the ORDER is the fix from engine#1801 and // the reviewer's ask was a test that pins it: on strixhalo /tmp is tmpfs, so the first choice has to be disk, and a version of @@ -3054,6 +3075,16 @@ Plan plan(const std::string& pack_dir) { // source from the .fconst's two per-head sections but the same tensor in the container. const bool graph_qk_norm = graph_weights && !onnx.index.empty() && onnx.index.count("model.layers.0.attn.q_norm.layernorm.weight"); + // engine#1889: the pack says it has per-head q/k norms and neither source found them. Refuse by name -- importing + // without them is a wrong answer that reads like a clean run. + if (declared_qk_norm(pack_dir) && !p.knorm_off && !graph_qk_norm) + throw std::runtime_error( + "this pack declares use_qk_norm: true, so its per-head q/k RMSNorm gains must be applied, but they were found " + "in neither place this importer looks: the layer block's two per-head sections (hidden " + std::to_string(d.hidden) + + ", " + std::to_string(d.heads) + " heads, so the arithmetic expects a gain section after the q/k/v bundle) and the " + "graph's model.layers.0.attn.q_norm.layernorm.weight. Importing without them would normalise nothing and answer " + "quiet nonsense from byte-exact weights, which is the failure this refusal exists to prevent (engine#1889)."); + if (p.knorm_off || graph_qk_norm) for (int L = 0; L < d.layers; ++L) for (const char* nm : {"self_attn.q_norm.weight", "self_attn.k_norm.weight"}) { diff --git a/npu/pack_import.h b/npu/pack_import.h index 838513fbb..5fcd6743e 100644 --- a/npu/pack_import.h +++ b/npu/pack_import.h @@ -462,6 +462,10 @@ const Fconst19Rule* select_fconst19_rule(int hidden, int heads, int layers, std: // weights into the wrong projections while every anchor still lines up), and a row that a merge silently drops (the // h4096 row was lost twice that way, which un-measured a whole model class with nothing failing). const Fconst19Rule* fconst19_rules(int* count); +// True when the pack's own config declares a per-head q/k RMSNorm (`use_qk_norm`/`qk_norm`, top level or under +// `text_config`). Exposed so a test can pin the declaration reading: engine#1889 is the case where the declaration was +// ignored and the import silently ran without the norm. +bool declared_qk_norm(const std::string& pack_dir); // The preference order for the 1.9 canonical .fconst scratch copy (engine#1801): the explicit override, then the on-disk cache // directory, then the temp directory and the cwd. Exposed so a test can pin the order -- the bug that shipped once was a cache // directory that was never created, so every pack silently fell through to tmpfs. diff --git a/tests/npu_pack_import_test.cpp b/tests/npu_pack_import_test.cpp index 019347612..0b257f2eb 100644 --- a/tests/npu_pack_import_test.cpp +++ b/tests/npu_pack_import_test.cpp @@ -2707,6 +2707,25 @@ void test_fconst_scratch_order() { std::printf("fconst scratch: order is override -> disk cache -> temp/cwd, pinned\n"); } +// engine#1889: the pack's own use_qk_norm declaration has to be read, because the importer's layout arithmetic can fail to +// find the gains and then silently import without them (Hunyuan-7B answered gibberish on byte-exact weights). +void test_declared_qk_norm() { + const auto with_config = [](const char* body) { + const std::string dir = "/tmp/1bit-qkn-" + std::to_string(::getpid()); + std::system(("rm -rf " + dir).c_str()); + std::filesystem::create_directories(dir); + std::ofstream(dir + "/config.json") << body; + const bool got = pi::declared_qk_norm(dir); + std::system(("rm -rf " + dir).c_str()); + return got; + }; + check(with_config(R"({"use_qk_norm": true})"), "qk norm declared: top-level use_qk_norm: true is read"); + check(with_config(R"({"text_config": {"use_qk_norm": true}})"), "qk norm declared: read under text_config too"); + check(!with_config(R"({"use_qk_norm": false})"), "qk norm declared: false is false"); + check(!with_config(R"({"hidden_size": 4096})"), "qk norm declared: an absent key is false, not an error"); + std::printf("qk norm declaration: top-level, text_config, false and absent all read\n"); +} + void test_qwen3_attn_sections() { struct Pack { const char* name; int64_t hidden, q_out, kv_out, hd, h0, h1, h2; }; const Pack packs[] = {{"Qwen3-1.7B", 2048, 2048, 1024, 128, 4739136, 4739456, 7107264}, @@ -3408,6 +3427,7 @@ int main(int argc, char** argv) { test_embedding_component(golden_dir.empty() ? "." : golden_dir); test_fconst19_rule_table(); test_fconst_scratch_order(); + test_declared_qk_norm(); test_selfcheck_reads_bf16(); if (const char* rc = std::getenv("NPU_EMBED_COMPONENT")) dump_real_component(rc); test_model_config_geometry(golden_dir.empty() ? "." : golden_dir);