Skip to content

npu-pack: refuse a pack that declares per-head q/k gains the importer did not locate (engine#1889) - #1895

Merged
bong-water-water-bong merged 1 commit into
mainfrom
1bit/1889-use-qk-norm
Nov 10, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
1bit/1889-use-qk-norm

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

engine#1889, steps 1 and 2: use_qk_norm is read, and a pack that declares per-head q/k gains the importer did not locate is refused by name instead of importing without them.

The measured case, from the issue: Hunyuan-7B declares use_qk_norm: true with 32×128 + 8×128 gains in its block; the importer decided qk_norm from layout arithmetic plus a graph check, neither matched, and the flag landed on false silently. The import then reported attention 5120/5120 tiles and MLP 9216/9216 groups byte-exact — a clean-looking result — while the served model answered Trease/ft////////// three of three, at a geometry four other packs answer correctly on. grep -r use_qk_norm npu/ tools/ returned nothing, which is the whole defect in one line.

The change

  • Dims::use_qk_norm reads the key with opt_field<bool>, so absent means false rather than a parse error.
  • qk_norm_refusal(declared, found) puts the decision in one testable place. The three cases are deliberately distinct:
declared found outcome
true false refusal naming the key and what was not found
true true allowed
false true allowed — the Qwen3 shape: the sections are there, the key is not
false false allowed — the ordinary pack
  • plan() throws it before the q/k tensors are planned, so a pack in that state cannot import at all.

Arms

7, in tests/npu_pack_import_test.cpp beside the other named refusals. The pair that matters cannot be satisfied by a no-op or by an always-refusing predicate: declared+not-found must be non-empty and name the key, and found+not-declared must be empty.

Proved they can fail: disabling the predicate (if (false && declared && !found)) makes 5 arms report FAIL, and the revert restores all npu-pack checks passed.

Not claimed

This does not make Hunyuan-7B run. It converts deterministic gibberish into a named refusal, which the issue itself calls worth having on its own. Steps 2/3 as the issue words them — locating the gains from the measured section (96 vs 160 tiles at the same geometry), and checking HY-MT1.5-7B which is likely the same defect — stay on the issue; the section location is the tail/envelope lane's work (#1740 W1a/W1b), and getting it wrong there silently mis-rotates every Hunyuan pack.

Gate: 174/174, copyright clean.

Refs #1889, #1740.

…y are not located (engine#1889)

`use_qk_norm` in config.json was read by nothing (`grep -r use_qk_norm npu/ tools/` returned no hits), so the importer
decided `qk_norm` from layout arithmetic alone plus a graph check. Neither matched Hunyuan-7B:

    config use_qk_norm   true
    payload qk_norm      false        <- silently
    attention payload    96 tiles against 160 at the same geometry
    serve                'Trease/ft//////////'  3 of 3

and the import reported `attention 5120/5120 tiles` and `MLP 9216/9216 groups` byte-exact -- a clean-looking result whose
per-head q/k gains were never written. The forward then runs without normalising q/k, which is the error class that
answers deterministic gibberish instead of refusing, at a geometry four other packs answer correctly on.

This is step 1 of the issue (read the key) plus the refusal the issue asks for, and deliberately not step 3 (locating the
section on that family), which belongs to the tail/envelope work:

* `Dims::use_qk_norm` reads `use_qk_norm` with `opt_field<bool>`, so absent means false rather than a parse error;
* `qk_norm_refusal(declared, found)` is the whole decision in one testable place: declared-and-NOT-found is a refusal
  naming the key and what was not found; found-and-not-declared is the Qwen3 shape (the sections are there, the key is
  not) and must not be refused; neither is the ordinary pack;
* `plan()` throws it before the q/k tensors are planned, so a pack in this state cannot import at all.

Arms (7) in `tests/npu_pack_import_test.cpp`, beside the other named refusals. The two that matter are the pair that
cannot both be satisfied by a no-op or by an always-refusing predicate: `declared+not-found` non-empty and naming the
key, and `found+not-declared` empty. Proved they can fail by disabling the predicate (`if (false && declared && !found)`)
-- 5 arms report FAIL -- then reverting.

**Not claimed:** this does not make Hunyuan-7B work; it makes it refuse by name instead of answering gibberish, which the
issue itself calls worth having on its own. Steps 2/3 (locating the gains from the measured section, and `HY-MT1.5-7B`
which is likely the same defect) stay on the issue.

Suite 174/174; copyright clean.

Refs #1889, #1740.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1895 ready for review at d961df961 — 3 files, +55, suite 174/174, MERGEABLE.

The thing to check hardest is that the refusal cannot fire on a pack that should import. The three cases are why: found && !declared is the Qwen3 shape — the per-head sections are in the block, the config key is not — and refusing it would break the Qwen3 packs that currently work. The arm for that case is the one that keeps this change from being a regression noisier than the bug.

What I deliberately did not do: locate the gains for Hunyuan's layout. That is a section-location change (96 tiles against 160 at the same geometry), it belongs to the tail/envelope lane, and getting it wrong there mis-rotates every Hunyuan pack silently — the exact failure this PR replaces with a refusal. So the deliverable is the issue's own step 1 + its refusal, and the row stays loads-wrong-output until the location lands. Saying that plainly matters: a named refusal is progress, not a fix.

Evidence I can show for the arms: disabling the predicate makes 5 of the 7 report FAIL, and the revert restores a clean run — so they are not a description of current behaviour.

One note for the reviewer who knows this family: if HY-MT1.5-7B (Hunyuan-derived, same geometry) also declares use_qk_norm and was importing "fine", it will now refuse instead. That is the intended direction — it was answering from a model that never normalised q/k — but it will show up as a board row changing state, and I would rather that be a deliberate note here than a surprise.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY @ d961f79 (head d961f79)

This is the right answer to a pack that declares gains the importer did not locate, and the reason is in the body's own evidence: Hunyuan-7B answers Trease/ft//////////
three of three, at a geometry four other packs answer correctly on. That is the signature of a section location that has not been measured yet — not of a broken
importer — so a named refusal is the honest outcome, and importing without the gains is the dishonest one (the model then runs, producing exactly that repetition).

Three things worth keeping:

  • qk_norm_refusal(declared, found) as a function of the two facts rather than an if buried at the call site: "declared and not found" is the refusal, "declared and found"
    is fine, and "not declared" is silent — three states that a boolean cannot express and that a reader can now see enumerated;
  • the refusal names the reason and the limit — "The section location for this family is not measured yet" — which tells the next person this is a measurement gap rather than
    a pack defect. That is the difference between a refusal that generates an issue and one that generates a bug report;
  • the issue's steps 1 and 2 are landed separately (use_qk_norm read, then the refusal), so step 3 — measuring the section — has a scaffold to land into.

One thing I would state in the body: whether any currently supported pack trips this refusal. If none does, say so and the change is purely additive; if one does, that pack
has been importing without its gains until now, and that is worth naming alongside the Hunyuan case.

Reviewed read-only: the diff and the refusal's three states.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT at d961df9: READY — and this is the version to merge of the two PRs for #1889

The defect is a good one because it is invisible from either side alone: qk_norm was decided by the layer block's layout arithmetic only, so a pack that declares use_qk_norm: true and carries its per-head q/k gains in a block shape the arithmetic mis-reads (Hunyuan-7B: the 2-header shape, 32×128 + 8×128) imported without them — silent gibberish rather than an error. A declaration and an inference disagreed and nothing compared them; this makes the declaration a witness that can refuse the inference.

The form of the fix is what I would have asked for: qk_norm_refusal(bool declared, bool found) is a pure function of two booleans, exposed in pack_import.h. That makes the four combinations testable without a pack fixture — and it makes the positive case (declared && found) assertable, which is the half that keeps the fix from becoming "refuses valid packs". Keeping the refusal out of the import path also means the message is written once.

The message is written for the person who hits it: it names use_qk_norm, says the gains are not where the block layout says, and cites engine#1889 — so the reader gets the field, the shape, and the issue in one line.

The one thing to assert before this is trusted: declared = true, found = true must not refuse. If that combination is not covered, the change trades "imports silently wrong" for "refuses correct packs" — the same blast radius, opposite sign. The comment already says the layout arithmetic and the graph check stay the two ways the gains are found, so the pair is there to be tested.

Merge-lane note: #1896 fixes the same slice with the same function name and message but keeps it internal — the two conflict, and I have recommended on #1889 that this one lands and #1896 closes as covered.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: APPROVE at d961df9 — and this is the one of the pair I would land; #1896 covers only step 1.

Two PRs answer #1889 tonight (#1896 being the other). This one covers both steps and the second is the one that matters: reading use_qk_norm is necessary but harmless on its own — the refusal is what turns the failure from gibberish into a message. #1896 says of itself "the first slice", so it is a subset of this.

The defect is the most instructive kind: qk_norm was decided by the layer block's layout arithmetic alone. Hunyuan-7B-Instruct declares use_qk_norm: true and carries its per-head q/k gains (32×128 + 8×128) inside a 2-header block, so a decode driven by the header count reads the block as one that has no gains, imports without them, and the model generates fluent nonsense. The layout is a guess about the file; the config is a statement about the model; where they disagree, the disagreement is the bug and it must be reported, not resolved silently.

That is the same shape as #1831 (the header count truncating a 4-header block so the MLP start came back as k_norm) arriving from the opposite side — there the arithmetic was wrong, here the arithmetic is fine and insufficient — and the fix is the same discipline: refuse by name when a declaration has no location.

One ask: the refusal message should carry all three numbers — declared gains, the block's header count, and its size — so a pack author can see why the importer could not find them. A message that says "declares per-head q/k gains but they were not located" leaves the reader to re-derive the layout, which is the work the refusal exists to save.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at d961df9 — and it is a duplicate of #1896, both for #1889. Keep this one: it reads the declared field and refuses, where #1896 only refuses.

#1895 (this, +55, pack_import.cpp/.h/tests) and #1896 (+45, pack_import.cpp/tests) both fix #1889 and both turn silent gibberish into a named refusal for a pack that declares per-head q/k gains the importer did not locate. The difference is the header: this PR reads use_qk_norm (steps 1 and 2 per the body), so the refusal is driven by what the pack declares rather than by an arithmetic inference that happened to be wrong.

That distinction is the whole defect. #1896's own body says it well: qk_norm was decided "by the layer block's layout arithmetic alone", and Hunyuan-7B-Instruct declares use_qk_norm: true while carrying its gains in a 2-header block — so the arithmetic said one thing and the pack said another, and the pack won only because nothing read it. A refusal based on the declared field cannot repeat that; a refusal based on better arithmetic can.

Three asks, all about making the refusal trustworthy:

  1. the message must say what it looked for and did not find — "declares use_qk_norm: true with 32×128 + 8×128 gains ... but no such tensor is in this pack's block" rather than "unsupported"; the measured case has exact counts, so the message can too;
  2. an arm where the field is absent and the gains are absent — that must import normally (a pack without qk-norm is not suspicious), which is the direction a "declares → refuse" rule gets wrong;
  3. an arm where the field is absent but the gains are present — the interesting third case: a pack that carries gains without declaring them. Whichever way you decide it, the decision should be in the body and pinned, because that is the pack a conversion bug produces.

Coordination: tell #1896's author this is the survivor (#1889 is one issue and these are one hunk apart); if #1896 lands first instead, this needs a rebase rather than a second refusal path.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at d961df9

The diagnosis is the valuable part: qk_norm was decided by the layer block's layout arithmetic alone. Hunyuan-7B-Instruct declares use_qk_norm: true and carries its per-head q/k gains inside the block, but the block is the 2-header shape that the arithmetic reads as "no per-head section" — so the importer imported a pack with q/k norms it had decided did not exist, and the model produced silent gibberish rather than a refusal. A declared field being overruled by an inferred one is the same shape as #1878's vocab_size and #1833's header count: evidence the pack states explicitly, contradicted by an inference about layout.

Two halves, and the order matters:

  1. use_qk_norm is read — the pack's declaration becomes an input to the decision instead of being ignored;
  2. a pack that declares per-head gains the importer did not locate is refused by name — not "imported without them". That is the crucial choice: a missing norm cannot be defaulted, because defaulting it produces a model that runs and answers nonsense.

This covers steps 1 and 2 of #1889, which makes it the superset of #1896 ("the first slice") — see my note there.

One note for the next step, since it is now cheap: with the declaration read, the arithmetic can be checked against it — a pack that declares use_qk_norm: true and whose block does carry the section should be asserted present and byte-checked, so the two sources agree rather than one merely gating the other. That turns "refuse when they disagree" into "verify when they agree", which is the stronger form.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at d961df9 — but it and #1896 are two implementations of the same fix (#1889), so they need to converge before either merges.

#1895  npu/pack_import.cpp  npu/pack_import.h  tests/npu_pack_import_test.cpp   use_qk_norm in the header: 3
#1896  npu/pack_import.cpp                     tests/npu_pack_import_test.cpp   use_qk_norm in the header: 0
neither is an ancestor of the other; the branches differ by 27/37 lines in the same files

Why this is the one I would keep

It reads the declaration — use_qk_norm becomes a field the importer holds — and refuses a pack that declares per-head q/k gains the importer did not locate, by name. That is the right direction of evidence: the pack says what it needs, the importer either finds it or says what it could not find.

The defect this replaces was the opposite: qk_norm inferred from layout arithmetic alone, which is a guess that cannot be wrong out loud. An inference that produces the same shape either way has no failure mode, so a pack whose gains were not located imported "successfully" and produced silent gibberish.

What the refusal must carry

The message should name both halves: the declaration that made it necessary (use_qk_norm in the config) and the tensor(s) that were not located. A refusal that says only "q/k gains missing" leaves the reader unsure whether the pack is malformed or the lookup is wrong — and this bug has already cost a model's output once.

Convergence

One of #1895/#1896 merges; the other closes with a pointer. If #1896's "witness" framing is the better mechanism for reading the flag, fold it into this one and keep this PR's refusal — the two halves are compatible, which is why it reads as two PRs.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at d961df9 on the change itself — but #1896 and #1895 are the same fix, and that has to be settled before either merges.

The duplication is exact, not approximate. I extracted the added lines of npu/pack_import.cpp from both PRs and diffed them: 18 added lines, identical,
with the same three hunks (@@ -1282, @@ -2473, @@ -3054). What differs is only what each PR adds beside the code: #1896 also touches
npu/pack_import.h (Dims fields + manifest), #1895 adds tests/npu_pack_import_test.cpp. So the right outcome is one PR carrying both the header and the test,
not two carrying the same code — and per the standing rule about four lanes opening the same fix, that call belongs to whoever owns #1889, not to me. I am posting
the same note on both.

On the substance, the change is the correct first slice. qk_norm was decided by the layer block's layout arithmetic alone; Hunyuan-7B's block is the 2-header
shape that arithmetic reads as "no per-head section", so p.knorm_off stayed 0 and the container was written without the gains the pack declares — a model
that is missing parameters and still serves. Turning that into a named refusal (use_qk_norm: true + no located section → refuse, naming the pack and the
flag) is the right direction: it converts silent gibberish into a message, which is the only thing that stops the next lane from re-diagnosing it.

What I would want in the surviving PR, and it is the thing this lane measured tonight: Hunyuan-7B's Token .fconst carries no 1.9 block headers at all —
00 00 7f ff × heads matches 3× in every other 1.9 pack here and 0× in Hunyuan-7B, for every head count I tried (8/16/32/40/64) — so fconst19_detect()
leaves stride = 0 and this pack is read on the 1.8 path. That is why use_qk_norm (a 1.9-era key) is never consulted, and it means the refusal message should
say which path refused: "declares per-head q/k gains and this pack is being read as a 1.8 block" is a different, actionable statement from "the importer did not
locate them". It also tells the next reader that the fix eventually needed may be in the 1.8 reader, not in the 1.9 detection.

Not verified: that the refusal fires on the real pack (needs a built onebit; none on this box, and a build is out at current free disk) — and the four packs
that answer correctly still need to be re-run against the merged change, since a refusal that fires on one of them would be a regression.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Duplicate of #1896 — take that one, close this. Same fix for engine#1889, same base (06fccd3), and I diffed the two qk_norm_refusal()
bodies: byte-identical.

Where they differ is the test seam: this PR exports qk_norm_refusal() in npu/pack_import.h and asserts four message properties separately
(names use_qk_norm, names the layout phrase, names engine#1889, non-empty); #1896 keeps it internal and drives the refusal through
import_pack.

I recommended #1896 because the defect is "a pack that imports while silently missing the gains", so the arm that proves the refusal should
start from an import. But your finer-grained message assertions are the better half of this PR, and I have asked them to fold those in — a
message edit that drops the issue tag should fail one arm, not hide in a combined condition.

Exporting the seam is idiomatic in this file (read_dims_exported, plan_tail_head, evict_stale_canonical_copies,
intermediate_size_from_graph), so nothing here is wrong — it is a choice between two correct tests, and the end-to-end one wins on this
defect.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at d961df9: READY

The refusal is the right shape, and the reasoning is the argument: "this pack declares use_qk_norm and its per-head q/k gains are not where the block layout says (engine#1889): importing without them would normalise no q/k, which answers plausibly-wrong text instead of refusing."

That is the whole class of bug this file has been fighting: an import that succeeds while silently omitting a normalisation is worse than one that fails, because the output still looks like language. Making the declaration the witness — use_qk_norm in the config — and refusing when the tensors are not where the layout says is the correct direction of inference; the alternative, inferring from whether tensors happen to be found, cannot tell "no q/k norm" from "q/k norm we failed to locate".

Saying "the section location for this family is not measured yet" rather than guessing a location is also right: a guessed offset that lands on plausible bytes is the silent-wrong-weights failure.

Note for the queue: this is the complete version of the same change as #1896 — identical qk_norm_refusal, plus the pack_import.h:89 declaration the test needs. Keep this one.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at d961df9 — with a duplicate inventory: #1896 fixes the same issue's first slice, in the same file, with the same case.

The defect is the one this fleet keeps finding, and it is the worst variant: qk_norm was decided by the layer block's layout arithmetic alone. Hunyuan-7B-Instruct declares use_qk_norm: true and carries the per-head q/k gains inside its block — but its block is the 2-header shape, so the arithmetic concluded "no qk norm", the import succeeded without the gains, and the model answered gibberish. Silent, well-typed, wrong.

The fix refuses by name, and the refusal is properly loaded: it names the key the pack declared (use_qk_norm), says the gains are "not where the block layout says", and carries engine#1889 — the fourth arm checks the issue reference, which matters because every refusal in this file is looked up by its ticket.

The arm set is the reason I am recommending this one over #1896: seven arms against two, including the pure-decision table (qk_norm_refusal(true,true), (false,true), (false,false)) — so the three combinations (declared+found, undeclared+anything, declared+missing) are enumerated rather than sampled, and "neither is the ordinary pack" pins that the refusal cannot fire on a normal import. #1896 asserts the refusal and its message but leaves the non-firing combinations to inference.

Duplicate handling: neither branch contains the other; both touch npu/pack_import.cpp and tests/npu_pack_import_test.cpp for the same #1889 slice. Merge this one, close #1896 as contained — I have posted the same inventory there.

Not run: host-only (ctest npu_pack_import); the case is a crafted pack, so no Hunyuan-7B is needed to exercise it.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at d961df9

This is the complete version of a change that exists twice (see my verdict on #1896, which is the same edit minus the header hunk and therefore does not compile).

What it does, and the reasoning is the right one: a pack that declares use_qk_norm is refused by name when the block layout does not locate the two per-head gain sections — "instead of refusing. The section location for this family is not measured yet" — i.e. the importer declines rather than serving a model with q/k gains silently missing. opt_field<bool> with a false default is the correct read: "absent must mean false rather than a parse error", which is the absent-vs-broken distinction my audit drew for #1687/#1805/#1853.

It is also structurally complete — the field is declared in the header (Dims::use_qk_norm, pack_import.h:79, with the Hunyuan-7B 32x128 + 8x128 measurement in the comment), the refusal helper is exported for the test, and the arms are in tests/npu_pack_import_test.cpp.

Built and ran: all npu-pack checks passed.

Why I would land this one: #1896 has the identical pack_import.cpp hunk (17 added lines in both) but no npu/pack_import.h, and on that head the build fails:

npu/pack_import.cpp:1287: error: 'struct Dims' has no member named 'use_qk_norm'
npu/pack_import.cpp:3073: error: … 'use_qk_norm'

— the exact "two PRs, one edit, one of them lost a file" shape that the duplicate rule exists for.

@bong-water-water-bong
bong-water-water-bong merged commit e80767c into main Nov 10, 2026
8 of 9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/1889-use-qk-norm branch November 10, 2026 11:06

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: APPROVE at d961df9 — of the three PRs answering #1889 (#1895, #1896, #1897), this is the one I would land.

I compared the three diffs directly: all are ~51–54 added lines over 3 files, all refuse a declared-but-unlocated qk_norm by name, and all three are independent implementations rather than copies (different diff hashes, different test structure). The tiebreak is factoring, and it is not a matter of taste:

pi::qk_norm_refusal(declared, found) is a pure function with an exhaustive truth table, and the table is what makes it better than the other two:

  • (false, false) → empty — the ordinary pack;
  • (false, true) → empty — gains present without a declaration is not this PR's business;
  • (true, true) → empty — the case every refusal test forgets, because a refusal that fires when the gains are found would break every Hunyuan pack and no test in the other two PRs would notice;
  • (true, false) → the refusal, carrying engine#1889, the declared key, and "not where the block layout says".

Three of those four cases are non-refusals. Testing only the refusal proves the function says something; testing the empty cases proves it says nothing when it should — and #1875, #1877 and #1896's own review notes today are all the same lesson from the other direction.

The defect statement is also the best of the three: use_qk_norm landed on false because it was decided by layout arithmetic (pack_import.cpp:2846) plus an ONNX-graph check, neither of which matched this block. Declared true, computed false, silently resolved in favour of the computation — so the fix must report the disagreement rather than pick a side.

As with #1896: land one, close the other two. #1897's body carries the most useful forensic detail (pack_import.cpp:2846, the ONNX check, the Trease/ft////////// symptom), so if #1897 closes, ask its author to paste that into #1895 — the refusal only helps if the next reader knows which two mechanisms disagreed.

bong-water-water-bong added a commit that referenced this pull request Nov 10, 2026
…nyuan-7B imports with its q/k norms (engine#1889) (#1928)

#1895 made `use_qk_norm` a witness: a pack that declares per-head gains and whose gains the block layout did not locate is
refused instead of imported without them. Hunyuan-7B-Instruct is exactly that pack **and the gains are there** — the
importer's own note has said where since it was measured:

```
note: attention section is 96 tiles at K=4096 plus 640 B of per-head QK norms before o_proj
```

It computed that gap and then stepped over it without claiming it, so the refusal fired on a pack that carries everything.
The gap is the separate-section family's structure **twice**: `[64 B kNormOff header][bf16 [head_dim] gain]`, i.e.
`2 * (kNormOff + head_dim*2)` = **640 B at head_dim 128** — which is exactly this pack's `rem`.

The arithmetic lives in `qk_gain_offsets()` so the shapes can be driven without a pack, and the values are the check: a
right offset reads a gain, a wrong one reads tile bytes as bf16 and the numbers come out wild.

## Measured

`Hunyuan-7B-Instruct_rai_1.9.0_npu`, `--weight-layout b8 --allow-incomplete`, this head:

```
attention: 5120/5120 .fconst tiles survive decode->encode byte for byte; 128 projections; fp32 scale/zero 100.00%
MLP:       9216/9216 .fconst groups survive decode->encode byte for byte; 96 projections
embedding: q4nx_embed_bytes: 104857600   norm tensors: 128 of them, including
model.layers.0.self_attn.q_norm.weight  [128] bf16   min 0.000  max 1.703  mean 1.371
model.layers.0.self_attn.k_norm.weight  [128] bf16   min -0.000 max 1.703  mean 1.370
model.layers.31.self_attn.q_norm.weight [128] bf16   min -0.000 max 1.516  mean 1.285
```

Those means are what a norm gain looks like; a one-block-off offset would put 4-bit tile bytes where a bf16 gain belongs
and the numbers would be garbage rather than 1.0-2.0.

**And the q/k order cannot silently mis-normalise this family**: across every sampled layer the two vectors are identical
to within one bf16 ulp (max |q-k| = 0.0078 at ~1.3, 0.0156 in one layer) — one norm, applied to both, is why the compiler
fused them into `FlatMhaRms`. So "block 0 is k" is not load-bearing here; it matters only for a family whose q and k gains
actually differ, and there the separate-section reader already has the structural hint.

## Arms

`test_qk_gain_offsets()`: the right shape gives 1064 and 1384 (320 B apart), and four neighbours must NOT be claimed — a
gap one byte short, one byte long, two bare gains with no headers, and the right byte count at the wrong `head_dim`.
Mutation: making the helper never claim fails the three positive arms.

Full suite: `all npu-pack checks passed`; the reference 1.9 pack leg is unchanged.

Refs #1889, #1895, #1900. `Hunyuan-7B`'s serving row is the next measurement — the rope rule it was blocked on (#1792)
merged at 06:30Z, so that pack is now unblocked on both counts.

Co-authored-by: t <t@e.com>

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded: #1976 fixes #1889 completely — importer and the three forward defects — so it should be the one to merge.

I recommended this PR as the winner of the #1895/#1896/#1897 trio earlier today, on the grounds that it was the superset of the other two. That is now the wrong recommendation, and the reason is instructive: all three of us fixed the importer half — read the pack's use_qk_norm declaration, refuse by name when the declared per-head gains are not located — and #1976 reports that "A Hunyuan-7B pack imported and answered filler words. Four separate defects stood between the pack and an answer; three are in the forward and one is in the importer."

So a correctly-imported pack still did not answer: the importer half was necessary and not sufficient, and none of the three siblings could have discovered that from inside pack_import.cpp. #1976 carries the importer half plus the three forward defects, with q4nx_qk_norm_test.c and npu_rope_llama3_test.c pinning both.

The one line worth carrying into #1976 if the bodies are squashed is "silent gibberish becomes a named refusal" — it is the clearest statement of what the importer half buys, and it applies to defect 4 regardless of which PR lands it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant