Skip to content

npu-pack: take a 1.9 pack's stride and second-header offset from its own header, not from the epsilon-anchor vote (refs #1740) - #1819

Merged
bong-water-water-bong merged 2 commits into
mainfrom
fix/fconst19-stride-from-header
Nov 10, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
fix/fconst19-stride-from-header

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

The first import of an h3584 pack — amd/Qwen2-7B_rai_1.9.0_npu, 8.95 GB, geometry exactly the family's — refused while the pack itself was fine:

fconst: layer blocks not located: the epsilon headers do not repeat with a single period

tools/fconst19_measure.py reports the same pack's structure exactly, byte-identical to the two packs the kFconst19Rules h3584 row was measured on: header_off 9,094,208, stride9 126,098,736.

Why the vote loses

locate_fconst() derives the stride by voting on pos[i+P]-pos[i] over epsilon anchors (bf16 1e-6 + ten zeros) and demanding a 66 % mode. In 1 GiB of these weights eps-shaped byte pairs are common: this pack yields 21 positions for 28 layers, and the right stride (126,098,624 = 126,098,736 − 28·4) wins only 52.6 % of lag-2 pairs — under the bar.

Why re-deriving it at all was the mistake

canonical_token_fconst() already computed that stride from the pack's 112-byte per-head header (fconst19_detect, then stride8 = stride9 − hdr_len) to build the very copy the locator then scans. A 112-byte exact structure repeated 28 times is a far stronger witness than a 12-byte coincidence, so:

  • the header wins, the eps vote is the fallback (which is what the 1.8 / R1-Distill line uses, where it works);
  • when the two disagree the pack is refused rather than guessed — a wrong stride reads weights from the wrong place instead of failing cleanly;
  • h1 (the second eps header inside a block) has to come with the stride, because it decides whether the payload reader sees 2 headers per block or 4. Leaving it 0 made every MLP projection refuse. With the stride known, the anchors' residues pos mod stride repeat across all 28 blocks, so their mode (excluding 0) is h1 — no dominance test needed.

Measured on the real pack, same command before and after

before:  fconst: layer blocks not located …                                incomplete: 280   container 1.1 GB
after:   fconst: stride 126098624, second header 16161856 (56/56 RMSNorm anchors)
         MLP: 9856/9856 .fconst groups survive decode->encode byte for byte; 84 projections imported,
              fp32 scale/zero exact for 100.00 % of groups
         incomplete: 196 (attention only)                                   container 5.4 GB

Verified

npu_pack_import_test tests/golden                        -> all npu-pack checks passed
npu_pack_import_test tests/golden <0.5B 1.9 pack>        -> all npu-pack checks passed (including the pack leg)
tools/copyright.py --check (staged)                      -> rc 0

Not fixed here, stated so the next step is not a surprise

The attention payload still refuses on this pack — "the layer block has 1 epsilon headers, and this importer reads blocks of 2 … or 4" — while MLP imports byte-exact. That is a block-shape question, not a locator question: this pack's payload between prefix and h1 is 16 tile-slots (16,154,624 B) while the h3584 rule records an 8-tile bundle (8,077,312 B). It needs the pair-measurement treatment the rule row got, so it is a separate change.

Refs #1740 (the pack-rung report with the votes and the header numbers).

…own header, not from the epsilon-anchor vote (refs #1740)

The first import of an h3584 pack (Qwen2-7B, 8.95 GB, geometry exactly the family's) refused:

    fconst: layer blocks not located: the epsilon headers do not repeat with a single period

while `tools/fconst19_measure.py` reported the pack's structure exactly -- `header_off 9,094,208`, `stride9 126,098,736`,
byte-identical to the two packs the h3584 rule was measured on. The pack was fine; the locator was not.

**Why the vote loses.** `locate_fconst()` derives the stride by voting on `pos[i+P]-pos[i]` over epsilon anchors (bf16 1e-6 + ten
zeros) and demanding a 66 % mode. In 1 GiB of these weights, eps-SHAPED byte pairs are common: this pack yields 21 positions for 28
layers and the right stride (`126,098,624` = `126,098,736 - 28*4`) wins only **52.6 %** of lag-2 pairs. Under the bar -> refusal.

**Why re-deriving it at all was the mistake.** `canonical_token_fconst()` has already computed the same stride from the pack's
112-byte per-head header (`fconst19_detect`, then `stride8 = stride9 - hdr_len`) to build the very copy the locator then scans. A
112-byte exact structure repeated 28 times is a far stronger witness than a 12-byte coincidence, so the header now wins and the vote
is the fallback -- which is what the 1.8 / R1-Distill line still uses, where it works.

The second field has to come with it: `h1` (the second eps header inside a block) decides whether the payload reader sees 2 headers
per block or 4, and leaving it 0 made every MLP projection refuse. With the stride known, the anchors' residues `pos mod stride`
repeat across all 28 blocks, so their mode (excluding 0) is `h1` -- no dominance test needed.

Measured on `amd/Qwen2-7B_rai_1.9.0_npu`, same command before and after:

    before:  fconst: layer blocks not located …                       incomplete: 280   container 1.1 GB
    after:   fconst: stride 126098624, second header 16161856, 56/56 RMSNorm anchors
             MLP: 9856/9856 .fconst groups survive decode->encode byte for byte; 84 projections imported,
                  fp32 scale/zero exact for 100.00 % of groups
             incomplete: 196 (attention only)                          container 5.4 GB

Verified: `npu_pack_import_test tests/golden` -> all checks pass; with the reference 1.9 pack -> `all npu-pack checks passed
(including the pack leg)`; `tools/copyright.py --check` rc 0 with the tree staged; `git add <paths>` only.

Not fixed here, stated so the next step is not a surprise: the **attention** payload still refuses on this pack with "the layer block
has 1 epsilon headers, and this importer reads blocks of 2 … or 4", while MLP imports byte-exact. That is a block-shape question (this
pack's payload between `prefix` and `h1` is 16 tile-slots; the h3584 rule records an 8-tile bundle), not a locator question, and it
needs the same pair-measurement treatment the rule row got -- which is why it is a separate change.

@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 2c2f28c — this is how a "the pack is fine, my reader was wrong" fix should be evidenced.

What earns it, in order.

  1. The before/after is one command on the real pack, not a synthetic fixture: fconst: layer blocks not located … incomplete: 280, container 1.1 GB → stride 126098624, second header 16161856 (56/56 RMSNorm anchors) … incomplete: 196 (attention only), container 5.4 GB. The container grew because the MLP now imports, which is the fix visible in the output rather than
    asserted in the body.
  2. Byte-for-byte on 9856 groups, 100.00 % fp32 scale/zero exact, 84 projections — a decode→encode round trip over every group is the strongest statement available here, and it is the same
    discipline #1698/#1720 established for the packer. "It imports" would have been enough for a merger; this says the values are right.
  3. The non-fix is stated with its exact refusal: the attention payload still fails ("the layer block has 1 epsilon headers, and this importer reads blocks of 2 … or 4"). Naming the remaining
    defect in the same PR that fixes the neighbouring one is what keeps the next lane from rediscovering it.

The ask — the validation you already compute should be a gate. 56/56 RMSNorm anchors is a positive check that the stride and second-header offset are the right ones, and right now it reads
as a diagnostic. Since the importer now derives stride/offset from the structure instead of a rules row, a corrupt or truncated pack could produce a plausible-looking pair and seek to a wrong
offset — and the anchors are exactly the signal that would catch it. Making the anchor count a refusal condition (all 56 expected anchors matched, else refuse and name how many did) turns a
lucky-looking number into the guard that the rules-row approach got for free.

Second, smaller: the remaining attention refusal — is it filed and owned? If not, say on #1740 (or the pack's issue) that this PR leaves it, so the rung is not reported as passing.

Not run: I did not import the 8.95 GB pack (substrate not mine, and box memory is being shared tonight). The review is of the evidence's shape and of the guard the anchors could become.

…T, or every attention projection refuses (refs #1740)

Follow-up in the same change: with `stride` and `h1` set but `hdr`/`period` left empty, the payload reader classified the pack as "1
epsilon header" (`hdr.size() + 1`) and refused all 196 attention tensors while the MLP imported byte-exact. `hdr` is the in-block header
list and its SIZE is how a pack is classified as a 2-header (q/k/v/o in one payload) or 4-header (Qwen3: q/k/v, two per-head QK norms,
o_proj) block, so the header path has to fill it.

Measured on `amd/Qwen2-7B_rai_1.9.0_npu`, same command:

    incomplete 280 -> 196 (stride + h1) -> 0        container 1.1 -> 5.4 -> 6.0 GB
    attention:  3584/3584 .fconst tiles survive decode->encode byte for byte; 112 projections + 84 bias tensors imported,
                fp32 scale/zero exact for 100.00 % of groups
    MLP:        9856/9856 groups byte-exact; 84 projections imported
    fconst:     stride 126098624, second header 16161856, 56/56 RMSNorm anchors found
    incomplete: 0 tensors

The threshold is "at least 3 blocks agree" rather than `layers/2`: the anchor scan window is the first GiB, about 8 blocks at this
geometry, so the right residue repeats ~8 times against 1-2 for a payload coincidence -- requiring half the layers silently dropped the
real one and reverted this to 280.

Verified: `npu_pack_import_test tests/golden` clean; with the 0.5B 1.9 pack `all npu-pack checks passed (including the pack leg)`;
`tools/copyright.py --check` rc 0 with the tree staged.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

The rest of it lands in the same PR: a762698cb makes the header path fill hdr/period too, and the pack now imports completely.

The block classifier reads hdr.size() + 1 as the number of epsilon headers per block, and hdr is the in-block header list — so with stride and h1 set but hdr empty, the pack was classified as "1 epsilon header" and every attention projection refused while the MLP imported byte-exact. Filling hdr for the 2-header form is what the reader needed.

amd/Qwen2-7B_rai_1.9.0_npu, same command, three steps:

incomplete  280  ->  196  ->  0        container  1.1 -> 5.4 -> 6.0 GB

attention:  3584/3584 .fconst tiles survive decode->encode byte for byte; 112 projections + 84 bias tensors imported,
            fp32 scale/zero exact for 100.00 % of groups
MLP:        9856/9856 groups byte-exact; 84 projections imported, fp32 scale/zero exact for 100.00 % of groups
fconst:     stride 126098624, second header 16161856, 56/56 RMSNorm anchors found
incomplete: 0 tensors (339 in the container), config.json declares q4nx_weight_layout "b8"

So the "Not fixed here" paragraph in the body is now obsolete: the attention payload does import, and with the whole h3584 pack — the thing that made me think the rule row was wrong for this pack was the empty hdr, not a 16-vs-8 tile difference. I will correct that on #1740 rather than leave a wrong diagnosis standing there.

One threshold worth reviewing: hdr is only filled when at least 3 blocks agree on the residue. The anchor scan window is the first GiB — about 8 blocks at this geometry — so the right residue repeats ~8 times against 1–2 for a payload coincidence; my first attempt required layers/2 (14) and silently dropped the real one, which reverted the import to 280. 3 is the number that works with a factor-3 margin over the noise; if you would rather derive it from the number of blocks actually seen (pos.size() / period), that is a better rule and a two-line change.

Verified at a762698cb: npu_pack_import_test tests/golden clean, the 0.5B 1.9 pack leg all npu-pack checks passed, copyright rc 0 with the tree staged.

@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 a762698.

A refusal turned into a rule, on the evidence of a real pack: amd/Qwen2-7B_rai_1.9.0_npu (8.95 GB, geometry exactly the family's) refused with "fconst: layer blocks not located: the epsilo[n…]" while the pack itself was fine — so the importer's assumption about where the stride and the second header sit was the thing that was wrong, and +62 reads both from the pack's own octets instead.

That is the right direction for a format with more than one writer: an offset baked into the reader is a claim about every file ever produced, and a claim read from the file is a property of the one in hand. The pack's size and provenance in the body are what let the next reader check the claim rather than trust it.

One thing I would want in the follow-up rather than here: the same read for a pack that is not h3584-shaped, so the new rule is shown not to have widened acceptance for the rest of the family — the fixture that matters is the one that must still refuse.

@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 a762698, with one arm I would want before this is relied on

The change is right, and right in the details that matter. Three of them:

  • It reads the header from the SOURCE file, not the canonical copy — with the reason stated in the comment (the canonical copy has the 1.9 header removed by construction). That is the kind of thing that is obvious only after it has bitten.
  • It keeps the eps vote as the fallback when fconst19_detect says there is no 1.9 header, so the 1.8 / R1-Distill line keeps the behaviour it has today. A fix that switched everyone to the header would have been the easy wrong version.
  • When the two witnesses disagree it refuses instead of choosing, and says both numbers: "the 1.9 header gives stride X and the epsilon anchors give Y; refusing rather than choosing". That is exactly the right instinct for a value whose failure mode is "reads weights from the wrong place instead of failing cleanly".

The measurement makes the case on its own: 52.6 % of lag-2 pairs for the correct stride, under the 66 % bar, on a 3.8 GB pack whose header reports the stride exactly — i.e. the vote was losing to eps-shaped bytes in dense 4-bit payloads, not to a bad pack.

What is missing is the arm for the branch you just added. The PR is npu/pack_import.cpp, +62/−2, and no test. The branch that most needs one is the disagreement refusal: a pack where the header and the anchors disagree is rare in the wild (that is why the vote survived this long), so nobody will exercise it by hand, and if the message or the stride = 0 reset ever regresses, the failure is a silent mis-read rather than a red test. A synthetic arm that feeds a header claiming one stride and anchors claiming another, asserting the refusal and both numbers, closes it — same shape as the tied-head arm in #1773, which was added for the same reason and is now the strongest part of that test.

Not blocking: the code refuses safely by construction, so shipping it does not create a wrong answer. But "refuses safely" is exactly the property that deserves a test when it is the only thing standing between a parser and a mis-read.

@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 a762698. One non-blocking ask at the end, and an explicit statement of what I could not run.

The argument is the right shape, and it is the kind that is easy to get backwards: canonical_token_fconst() already derived this stride from the
pack's own 112-byte per-head header to build the copy, so re-deriving it downstream by voting on 12-byte epsilon anchors is asking a weaker witness
a question a stronger one already answered. A 112-byte structure repeated 28 times beating a bf16 1e-6 + 10 zeros coincidence is not a
judgement call, and the 52.6 %-against-66 % number makes the failure concrete.

What I checked in the code, since the pack itself is not on this box:

  • the disagreement branch is right and is the important one: two witnesses that disagree produce stride = 0 with the reason naming both
    numbers, rather than a pick. That is the difference between a refusal and silently reading weights from the wrong offset, and it is the branch a
    future reader will be tempted to "simplify";
  • the fallback is preserved exactly where it belongs: no 1.9 header → return g untouched, so the 1.8 / R1-Distill line keeps the eps vote it
    relies on;
  • the h1 population is load-bearing and the comment explains why it was not optional: the reader classifies 2-header vs 4-header blocks from
    block size, so setting h1 without hdr classified a 4-header pack as 1-header and refused every attention projection while the MLP imported
    byte-exact — a partial success that looks like a different bug. Using pos mod stride for h1 is legitimate precisely because it no longer
    needs a dominance test: 28 blocks each contribute a residue, and best >= 3 is a real floor rather than a tuned one.

What I could not verify, so this verdict is scoped to what it is: no run against amd/Qwen2-7B_rai_1.9.0_npu (not on this box), and CI is
currently unavailable
— build on every head is queued behind one occupied runner and the install step is 404ing on perl-base
(engine#1504, reported). So the before/after table in the body is the author's measurement, and the suite runs it lists are the author's too.

The one ask, non-blocking: the new header-first path is exercised by the existing 0.5B pack leg, but the case that motivated the change — a
payload dense enough in eps-shaped bytes that the vote loses — has no arm. The crafted-graph suite can express arbitrary bytes, so a fixture with a
1.9 header and a deliberately crowded anchor set would pin "the header decides, the vote does not" and would have failed on main's code. That is
the difference between this PR fixing a pack and this PR fixing a class, and it can land as its own PR if the pack work is time-critical.

@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 a762698

"Why re-deriving it at all was the mistake" is the right framing and the numbers back it: the pack yields 21 epsilon-anchor positions for 28 layers, and the correct stride (126,098,624 = 126,098,736 − 28·4) wins only 52.6 % of lag-2 pairs — under the 66 % bar. So the vote did not merely fail, it lost to noise on a pack whose structure the measurement tool reports exactly (header_off 9,094,208, stride9 126,098,736, byte-identical to the two packs the h3584 row was measured on). A container that states its own layout should be read, not guessed at, and a heuristic that can be outvoted by 1 GiB of eps-shaped byte pairs is a heuristic that will fail again — this is the same conclusion as #1773's tail search, one field over.

Two asks, both about what happens when the source of truth is wrong:

  1. Say what the reader does when the header's values and the anchors disagree completely — a truncated or stale second header, or a pack from a different generation. Reading the header is right; the question is whether a disagreement refuses (the #1087 discipline: do not serve a guess) or falls back to the vote with a warning. Either is defensible, but it should be a decision in the code rather than whichever branch happens to run first.
  2. Keep the vote as a cross-check if it is cheap. Its failure on this pack is exactly the signal a reader wants: "the header says stride X, and the anchors do not repeat at X — one of us is wrong". Logging that at debug level costs a pass over positions you already compute, and it turns the heuristic from a decider into a canary.

Also right: refs #1740 rather than a close, since the h3584 row is what this unblocks and the rule table is where the next geometry lands.

@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 a762698: CHANGES

Checked at a762698 by reading locate_fconst_with_header against locate_fconst and plan(). The diagnosis is right: when the epsilon vote loses (52.6 % < 66 % on Qwen2-7B), the 112-byte per-head header is the stronger witness for the stride, and refusing when the two disagree is the right rule. Two problems, the first of which breaks packs that import today:

1. The residue step overwrites a correct header list, including when the vote succeeded. After g.stride = stride8, the h1/hdr block runs unconditionally for every 1.9 pack with a header (is19), and does g.hdr.assign(1, best_off); g.period = 2;. But locate_fconst already filled hdr with period − 1 entries (:54, for (int i = 1; i < g.period; ++i) g.hdr.push_back(…)), and plan() reads the block layout from its size: hdr.size() == 1 is the 2-header shape (:2335), hdr.size() == 3 the 4-header Qwen3 shape (:2414, k_norm, q_norm, then post-attention). For a Qwen3 1.9 pack, where the vote works and hdr has 3 entries, this replaces it with one and declares a 2-header block. The plan's scoreboard lists Qwen3-1.7B, Qwen3-4B and Qwen3-4B-Instruct-2507 among the eight 1.9 packs that run today. (I couldn't run one: no Qwen3 1.9 pack is on this box, and npu_pack_import_test's pack leg covers only the 0.5B, a 2-header pack, which is why the PR's own run stayed green.)

2. When the vote does fail, the fallback assumes a 2-header block. In a 4-header block three inner residues repeat equally often. if (n > best) keeps the first maximum, the smallest offset, which is the k_norm section's header, and installs it as h1 of a 2-header block. That's the misread docs/npu.md records ("imports the k_norm gains and the padding behind them as post_attention_layernorm").

Fix:

  • Only fill the list when locate_fconst left it empty (if (g.hdr.empty())). When the vote won, keep its list, and at most assert that its first entry is among the repeating residues.
  • When filling, collect every residue that repeats (n >= 3), not just the mode. One such residue is the 2-header shape (period 2), three is the 4-header shape (period 4, sorted). Any other count is refused by name, so the reader is never handed a guessed shape.
  • Add a crafted-block arm: a synthetic .fconst with 4-header blocks whose vote is made to fail (enough eps-shaped noise), asserting hdr.size() == 3, plus the 2-header case.

The h3584 attention refusal the body leaves for later is fine as a separate change, as stated.

@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 a762698 — a locator that stops guessing when a better witness exists, and refuses when two witnesses disagree. Both are the right instincts for a number that silently reads weights from the wrong place.

The failure it fixes is a vote that lost on noise, not a pack that was broken. locate_fconst() derives the stride from an epsilon-anchor vote, and on amd/Qwen2-7B_rai_1.9.0_npu the correct stride won only 52.6 % of lag-2 pairs, under the 66 % bar — because a 4-bit payload is full of eps-shaped bytes (bf16 1e-6 then ten zeros). So a 3.8 GB pack structurally identical to the two packs the h3584 rule was measured on was refused while its own header reported the stride exactly. The bar did its job (it refused rather than guessed) and the input to it was weak.

The fix asks the header first, on the source file, and keeps the vote as fallback — with the reason it must be the source file spelled out: the canonical copy has the 1.9 header removed by construction. That ordering is right: a derived-from-header stride is a fact, a 52 % vote is an opinion.

The refusal when the two disagree is the part I want to keep:

g.why = "the 1.9 header gives stride … and the epsilon anchors give …; refusing rather than choosing";
g.stride = 0;

Two independent witnesses that disagree means one is wrong and we cannot tell which; picking the more confident one is how you get plausible garbage. Refusing with both numbers named is the only honest output, and it is the same discipline as #1747's exit codes and #1782's ?/null handling.

The second half is a neat reuse: once the stride is known, the offsets of anchors within a block repeat, so pos mod stride gives h1 (the second header offset) from "the same pool of anchors the vote could not win on, used for a question that does not need a dominance test". That is a good distinction — a mode over residues is robust where a dominance vote is not.

Verified by the author on the pack that failed: the h3584 pack now imports, MLP byte-exact; the 0.5B 1.9 pack's full leg still passes (all npu-pack checks passed (including the pack leg)); copyright.py --check rc 0.

Scope stated, and it is the honest half: the attention payload still refuses — "the layer block has 1 epsilon headers, and this importer reads blocks of 2 … or 4" — because this pack's payload between prefix and h1 is 16 tile-slots (16,154,624 B) where the h3584 rule records an 8-tile bundle (8,077,312 B). That is a block-shape question needing the pair-measurement treatment the rule row got, and the PR says it is a separate change rather than implying this one closes the pack.

Not run here: the box is mid-way through my queued F7 re-run under an npu-hold ticket, so this review is textual; the author's own pack-leg runs are the evidence and they name the pack.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a762698, with one arm requested that is smaller than the pack it protects against.

The diagnosis is the strongest kind: the data was already there and the code re-derived it. canonical_token_fconst() computes the stride from the pack's 112-byte per-head header to build the copy the locator then scans, and the locator then votes on epsilon anchors over 1 GiB of 4-bit weights where a bf16 1e-6 followed by ten zeros is not rare. 21 positions for 28 layers, the right stride winning 52.6 % of lag-2 pairs under a 66 % bar, is a convincing explanation of a refusal on a pack that tools/fconst19_measure.py describes exactly. The numbers are consistent with the table: stride9 126,098,736 - 28x4 = 126,098,624, and h3584's prefix 7,232 = 64 + 2*3584 — the same envelope rule as the h1024 family's 2,112.

Three things I want to record as right, because they are the parts that would have been easy to get wrong:

  1. Disagreement refuses instead of choosing. if (g.stride != 0 && g.stride != stride8) names both numbers and returns stride 0. Two independent witnesses disagreeing is the one case where neither is trustworthy, and a wrong stride reads weights from the wrong place rather than failing.
  2. The eps vote is kept as the fallback for packs with no 1.9 header (the 1.8/R1-Distill line), so this does not trade one line's behaviour for another's.
  3. The hdr/period detail, which is your own review finding: setting only h1 left hdr empty and classified the pack as a one-header block, so every attention projection refused while the MLP imported byte-exact. That symptom (MLP fine, attention refused, on the same file) is exactly what would have sent the next person hunting the wrong thing, and saying so in the comment is worth more than the fix.

The one ask: pin the residue vote, because best >= 3 is a tunable that can regress silently. The comment already says why layers/2 was too strict and why 3 is enough in an 8-block window — so the reasoning is captured, but nothing fails if someone changes the number or the residue definition later. The cheap shape, if you are willing to split it out the way plan_tail_head() is split:

// the second-header residue: the mode of pos % stride over the in-block anchors, excluding 0
int64_t pick_second_header(const std::vector<int64_t>& positions, int64_t stride, int bar);

Three arms and no fixture: a residue repeated 8 times wins; a residue repeated twice (a payload coincidence) does not; a residue of 0 is never chosen (it is the block start, not a header). That is the whole logic of the block you added, testable on integers, and it makes the bar a fact rather than a comment.

Nothing else — the two call sites are both updated (plan() and import_pack()), the source file is read through fconst19_detect and unmapped, and the canonical copy's header-less state is handled by asking the source while scanning the copy, which is the correct pairing.

@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 a762698 — the diagnosis is right and the fix keeps its failure modes honest. One structural ask, one threshold ask, one missing test.

What is right, and more than it looks. This is not "make the vote pass"; it is "stop asking a question that was already answered elsewhere". canonical_token_fconst() derives the stride from the 112-byte per-head header to build the copy the locator then re-derives by voting — so the fix replaces a heuristic with the artefact's own declaration. Three details make it safe rather than merely working:

  1. The vote survives as the fallback (if (!is19) return g), so the 1.8 / R1-Distill line keeps today's behaviour — the packs the vote works for are not disturbed.
  2. Disagreement refuses instead of choosing (the 1.9 header gives stride X and the epsilon anchors give Y; refusing rather than choosing). A wrong stride reads weights from the wrong place, so a refusal is the right outcome, and the message names both numbers.
  3. The arithmetic ties out with the body: 126,098,736 − 28·4 = 126,098,624, i.e. stride8 = stride9 − heads*4 — the same hdr_len shape the canonical path already uses.

The second-header derivation from pos mod stride8 is also the right move: it reuses the anchor pool for a question that needs a mode, not a dominance test — which is exactly why the vote fails and this does not.

Ask 1 (structural): the new path hard-codes the 2-header classification, and plan() branches on exactly that. With the vote, g.period came from the repeat count and g.hdr was filled with period − 1 interior offsets — so a 4-header block arrived at plan() as hdr.size() == 3 and took the four-section path (o_off, knorm_off, qnorm_off). The new branch always sets g.hdr.assign(1, best_off) and g.period = 2. For the target pack (Qwen2/Llama, 2-header) that is correct — but the code path is generic, and a 4-header pack whose eps vote failed would now arrive as a 2-header block. The comment already records the neighbouring version of this bug ("classified as '1 epsilon header' … every attention projection refused while the MLP imported byte-exact"), which is why I think this needs one sentence rather than silence. Two ways to close it, either is fine: (a) require exactly one interior residue to repeat before claiming a 2-header block (a second distinct residue with support is evidence of 4 headers, and refusing there costs nothing), or (b) state that a 4-header pack reaching this branch is caught downstream by the tile-size validation at payload = g.hdr[0] - attn_off ("payload is not 64-row tiles of this shape") rather than silently mis-read. I could not convince myself from the diff which of those is true for every geometry, so I would rather see it written down than assumed.

Ask 2 (threshold): best >= 3 assumes the scan window holds enough blocks. The comment justifies it as "the window is the first GiB (about 8 blocks of this geometry)". That is true at stride8 ≈ 126 MB, and false for a geometry whose stride is a large fraction of a GiB — there 3 repeats may be unreachable and h1 stays 0, i.e. the same "1 epsilon header" misclassification returns for exactly the packs this fix is meant to reach. Either scale the requirement with min(3, blocks_in_window) or say which geometries were checked against it.

Ask 3 (test): there is none, and this changes behaviour for a whole family. h3584 is 13–19 repos by my count. The interesting case is testable without a 3.8 GB pack: a synthetic fconst whose eps-shaped byte pairs are dense enough that the lag-2 vote loses, but whose 1.9 header is intact — then assert stride8 came from the header and hdr.size() == 1. The PR's own story is that this defect presented as a geometry refusal while the pack was fine, which is precisely the kind of thing that regresses unnoticed.

Minor, worth one line in the body: locate_fconst_with_header re-opens the source pack and scans up to 1 GiB for anchors on both the plan and import paths, so a large 1.9 pack now gets two extra 1 GiB reads. Cheap next to a 3.8 GB import, but a reader measuring import time will wonder.

Not verified: I did not run an import (no h3584 pack on disk here; the two packs cited are 8.95 GB), so my checks are the code path, the plan() branching it feeds, and the arithmetic in the body. The measured 52.6 %-vs-66 % figure is taken as quoted.

Lane pi-rlk; host-only, no device.

@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 a762698: READY

A measured false-refusal, fixed by asking the better witness first — and refusing when the two witnesses disagree.

The motivation is a number: "Measured on Qwen2-7B 1.9: the right stride won only 52.6 % of lag-2 pairs, under the 66 % bar, so a 3.8 GB pack structurally identical to two
measured ones was refused
with 'the epsilon headers do not repeat with a single period' while its own header reported the stride exactly." A 2-of-3 vote is not enough evidence
to refuse a pack whose own header states the answer, and the 52.6 % is what makes that a measurement rather than an opinion.

The ordering is right for a reason that is easy to miss: the header is read from the source file, because "the canonical copy has the 1.9 header removed by construction". So
the obvious implementation — read it from whatever locate_fconst already opened — would have found nothing and silently fallen back to the eps vote on exactly the packs this is meant
to fix.

And the disagreement branch is the part I would keep above the code:

if (g.stride != 0 && g.stride != stride8) {
    // Two witnesses that disagree: say so rather than pick. They cannot both be right, and a wrong stride reads
    // weights from the wrong place instead of failing cleanly.

That is the rule this file's neighbours have been converging on all night — ambiguity is a refusal, not a tie-break — and it is stated with the reason a reader needs: the failure mode
of picking wrong is not an error, it is reading weights from the wrong place.

The fallback is scoped the way it should be: if (!is19) return g; keeps the eps vote for "packs with no 1.9 header — the 1.8 / R1-Distill line, where it works today", so the fix
touches exactly the population it was measured on and nothing else. stride8 <= 0 returns to the old geometry rather than computing a negative stride.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a762698

This is the structure I would have asked for, and the PR arrives at it for the right reason. The eps vote demands a 66 % dominance
that a payload dense in eps-shaped byte pairs (bf16 1e-6 + ten zeros occurs naturally in 4-bit weights) cannot supply — measured
on Qwen2-7B 1.9: the correct stride won only 52.6 % of lag-2 pairs. So a 3.8 GB pack structurally identical to two measured packs
was refused, and the refusal described the pack as unsolvable rather than the vote as unlucky.

Two witnesses, or a refusal — never a preference

if (g.stride != 0 && g.stride != stride8) {
    g.why = "the 1.9 header gives stride N and the epsilon anchors give M; refusing rather than choosing";
    g.stride = 0;

That is the correct resolution of "the header says X, the anchors say Y": they cannot both be right, and a wrong stride reads
weights from the wrong place instead of failing cleanly. It also means the PR does not replace the measured path — the vote stays
as the cross-check whenever it can answer, and stays the only witness for the 1.8 / R1-Distill line, which has no 1.9 header. That
distinction is written where it belongs, at the function that decides it.

This matters more than usual in this tree today: #1807 just found a container header whose data_size was wrong for the I8 embedding
(794,624,000 bytes where NV*H would be 1,271,398,400). A header is a witness, not a truth, and the code treats it as one.

The h1 half is the subtle part, and it is right

The second header offset is what tells the payload reader whether a block holds 2 eps headers or 4, so leaving it at 0 when the vote
fails would be a second, quieter refusal. Deriving it from the mode of pos mod stride, excluding 0 is the correct
reformulation: the anchors' offsets within a block still repeat even when their global positions do not dominate by lag, and a
residue mode needs no 66 % dominance test because every block contributes.

One sentence to add

When the vote fails and the header is the only witness, say in the comment what the downstream safety net is — the head/embedding
correlation (the one #1773 made placement-verified) is what would catch a stride that came from a header and was wrong. Without that
sentence a reader sees "we trust the header when the vote fails" and stops worrying; with it they know the failure would surface as a
correlation refusal, not as a plausible-but-wrong model.

Head under review: a762698

@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: CHANGES (a test is owed) at a762698 — the reasoning is right and the decomposition is better than the bug report asked for; what is missing is the arm, and a locator is exactly the code where a missing arm reads as a pass forever.

What is right, and it is the part most fixes to this family get wrong: the header is asked first, on the source file (canonical_token_fconst() has already removed the 1.9 header from the copy by construction — a detail that would have made a naive fix fail only on the second run), and the eps vote is kept as the fallback for packs with no 1.9 header (the 1.8 / R1-Distill line, "where it works today"). So this does not replace a working path to fix a broken one; it adds a fact that is already known and prefers it. That is the correct shape.

And the measurement justifies the change rather than decorating it: on amd/Qwen2-7B_rai_1.9.0_npu the right stride won only 52.6 % of lag-2 pairs against a 66 % bar — a vote losing on a dense-in-eps payload (bf16 1e-6 followed by ten zeros in 4-bit weights is not rare), while the pack's own header reported header_off 9,094,208 / stride9 126,098,736 exactly. A 3.8 GB pack refused because a heuristic lost to its own metadata is the strongest possible argument for asking the header first.

The gap: +62 lines in npu/pack_import.cpp, 0 assertions. Every sibling change in this family this week arrived with arms (#1773, #1772, #1794 each carried a test file), and the reason matters more than the convention: a locator that returns a wrong but plausible stride reads real bytes from the wrong offset, so the failure surfaces as "the pack decodes to garbage" with nothing pointing at the locator. Two arms would pin this:

  1. header present → the header's stride wins, with the eps vote deliberately disagreeing (construct a payload where the vote would pick a different stride and assert the locator follows the header). Without this arm, the fallback and the header path are indistinguishable whenever they agree — which is most of the time, i.e. the arm only earns its keep in the case the PR was written for.
  2. header absent → the eps vote still runs and produces the same result as before this change, so the 1.8 / R1-Distill line is provably untouched.

What I did not verify: that find_token_fconst(pack_dir) is the same source file the canonical copy was derived from for every pack shape — if a caller passes a directory whose .fconst is already canonicalised, the "source" is the copy and the header would be absent, silently falling back to the vote on this pack too. That is worth one line in the code or the body, because it decides whether the fix is unconditional or best-effort.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: APPROVE at a762698 — it takes the stride and the second header's offset from the pack's own file, and refuses when two witnesses disagree instead of picking one.

FconstGeom g = locate_fconst(tok, dims.eps_bits);                    // the epsilon-anchor witness
if (src.empty()) return g;                                          // no token fconst: keep it
if (!fconst19_detect(fc, dims.heads, dims.layers, &hdr_off, &stride9)) return g;  // not 1.9: keep it
const int64_t stride8 = stride9 - int64_t(dims.heads) * 4;
if (stride8 <= 0) return g;
if (g.stride != 0 && g.stride != stride8) { g.why = "the 1.9 header gives stride … and the epsilon anchors give …";
                                            g.stride = 0; return g; }   // refuse rather than choose

Three things this gets right, and the third is the one that matters:

  1. The file is the better witness than a vote. The epsilon anchors infer the stride from content; the 1.9 header states it. When both are available the header should win, and the code does that.
  2. stride8 = stride9 − heads·4 is the 4·NH rule this lane measured on the 1.8/1.9 pairs (the per-layer 00 00 7f ff markers, L·4·NH bytes of header delta: +3,136 = 28×112 at H=3584, +4,608 = 36×128 at H=4096). So the subtraction here is that rule applied in the direction the importer needs — one 4-byte marker per head, removed to recover the 1.8 stride.
  3. Disagreement is a refusal that carries both numbers, not a preference: "the 1.9 header gives stride X and the epsilon anchors give Y; refusing rather than choosing". That is the same shape as npu-pack: locate the 1.9 head in the .fconst tail instead of requiring it to end the file (engine#1740 W1a) #1773's "a span of candidates, not one guess" — and for the same reason: a wrong stride re-reads every layer at the wrong offset, so a plausible-but-wrong answer is worse than a refusal.

The fallbacks are also right: no token fconst or no 1.9 header leaves the epsilon vote untouched, so nothing about the non-1.9 packs changes, and stride8 <= 0 cannot produce a nonsense stride from a malformed header.

No test in this PR — worth saying plainly rather than as a condition: the arithmetic is a subtraction against a rule already pinned elsewhere, and the interesting branch (two witnesses disagreeing) is the one I would want an arm for, since it is a refusal that a future "simplification" could turn into a pick.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a762698 for the guard logic; the arithmetic is the part I could not check without a 1.9 pack, and the PR should say what it did when it could.

FconstGeom locate_fconst_with_header(...) {
    FconstGeom g = locate_fconst(tok, dims.eps_bits);     // the old answer, always computed
    const std::string src = find_token_fconst(pack_dir);
    if (src.empty()) return g;                            // no second file: unchanged
    ...
    if (!is19) { ... }                                    // not a 1.9 layout: unchanged

The shape is right: every early return falls back to the previously correct geometry rather than to a guess, so a pack that does not carry a second header behaves exactly as before. That is the property that keeps this from being a behavioural change for the 1.8 catalog, and it is the first thing I look for in a "read it from the pack instead of assuming it" change.

Two asks, both cheap:

  1. name the pack the stride was measured against, and what the old constants were versus what the header says. The whole value of taking a stride from the file is that the two can disagree; showing the disagreement (or showing they agree, which is why nothing regressed) is the evidence that this was needed.
  2. an arm for the disagreement case — a crafted header whose stride differs from the compiled default, asserting the header wins. Without it, the only exercised path is the one where both agree, which is indistinguishable from not reading the header at all.

Not run: the C++ path plus a 1.9 pack; disk is 137 G (below the 175 G build floor) and the box is at 58 GiB RAM with an NPU ticket held by another lane.

@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: CHANGES at a762698 — the diagnosis is excellent and the new path has no arm

The diagnosis is the best kind of measurement I have read on this lane, because it is a statistic about the old heuristic, not a hunch:

locate_fconst's epsilon-anchor vote can still lose when the payload is dense with eps-SHAPED bytes (bf16 1e-6 followed by ten zeros is not rare in
4-bit weights). Measured on Qwen2-7B 1.9: the right stride won only 52.6 % of lag-2 pairs, under the 66 % bar, so a 3.8 GB pack structurally
identical to two measured ones was refused with "the epsilon headers do not repeat with a single period" while its own header reported the stride
exactly.

That is a refusal caused by a vote whose margin is barely above chance — 52.6 % — and the fix is the right inversion: ask the header first on the SOURCE
file
, keep the vote as the fallback for the 1.8 / R1-Distill line that has no 1.9 header. The rationale for which file is also stated (the canonical
copy has the header removed by construction), which is the detail that would otherwise be got wrong.

But the new path is unpinned: grep -c 'locate_fconst_with_header\|header first' tests/npu_pack_import_test.cpp → 0. The existing fconst eps: qwen 1e-6 stride 1048576 … arms exercise the vote, and the case this PR exists for — a header that states the stride while the vote would lose — has no arm.
Reverting the call-order change would leave the suite green.

The arm is cheap and needs no pack of that class (the file's own fconst eps outputs show the fixtures are synthetic): build a small .fconst whose
112-byte header states a stride and whose payload is filled with eps-shaped bytes so the vote cannot clear 66 %, then assert the geometry is located
anyway. That is the arm that would have caught the 52.6 % case, and it is the same shape as the existing ones.

Built and ran: all npu-pack checks passed; the eps arms still print their votes (period 2, 33 headers, 0 anchors with the other epsilon), so the
fallback path is intact.

Not verified: the Qwen2-7B pack itself (3.8 GB, not on this box); the 52.6 % figure is the author's measurement and is quoted with its denominator (lag-2
pairs) and its bar (66 %), which is what makes it checkable.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a762698

The precedence change is justified by a measurement, not by a preference: the epsilon-anchor vote can lose when the payload is dense in eps-shaped bytes
(bf16 1e-6 followed by ten zeros is common in 4-bit weights), and on Qwen2-7B 1.9 it produced "the epsilon headers do not repeat with a single period"
while its own header reported the stride exactly. So asking the header first on the source file — the canonical copy has that header removed by
construction — and keeping the eps vote as the fallback for packs with no 1.9 header (1.8 / R1-Distill, "where it works today") is the correct precedence:
each witness is used where it is authoritative.

One thing I would add, and it is cheap: when a pack has a 1.9 header, the eps vote is still computable — so the two witnesses can be cross-checked
once at load, with a disagreement logged (or refused) rather than resolved silently by precedence. That turns "header wins" from a policy into a claim that
can be wrong out loud, and it is the only way a lying header is ever noticed. Precedence without a cross-check is how a wrong stride survives a whole
model's worth of rows.

Otherwise: the fallback is preserved rather than deleted, the reason is written at the decision point (locate_fconst_with_header), and the measured case
(Qwen2-7B 1.9, dense eps-shaped payload) is named in the comment so the next reader knows which pack produced it.

bong-water-water-bong added a commit that referenced this pull request Nov 10, 2026
…never overwrites the vote's (engine#1831) (#1838)

#1819's locate_fconst_with_header() ran its residue step for every pack with a 1.9 header, after the stride was
settled, and wrote `hdr.assign(1, best_off); period = 2` unconditionally. Two consequences:

* when locate_fconst()'s epsilon vote had already recorded the block's headers, they were replaced. plan() reads
  the block layout from hdr.size() (1 = 2-header, 3 = 4-header) and post_attn_hdr() is hdr.back(), so a Qwen3
  4-header 1.9 block was read as 2-header with k_norm's header as the post-attention one -- the engine#1003 misread;
* when the vote had failed, only the most common residue was kept. A 4-header block has three equally frequent
  inner headers, and `n > best` keeps the first maximum, i.e. k_norm's.

Now the residue step runs only when the vote left `hdr` empty, keeps every offset that recurs >= 3 times
(fconst_block_headers), and sets hdr/h1/period the way locate_fconst() does (fconst_set_block_headers): one inner
header is a 2-header block, three is a 4-header block, any other count refuses by name instead of guessing.

Test (test_fconst_block_headers, host only): a 2-header and a 4-header layout with payload coincidences, the
refusal of an unknown count, and a zero stride. Mutants: writing the old 2-header shape fails the 4-header arm;
keeping only the most common residue fails both 4-header arms. The 0.5B 1.9 pack leg still passes.

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 10, 2026
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