Skip to content

npu-pack: a tail padded by whole tile-slots is not a head whose offsets are unknown (refs #1740) - #1852

Closed
bong-water-water-bong wants to merge 1 commit into
mainfrom
fix/1740-tail-trailing-padding
Closed

bong-water-water-bong wants to merge 1 commit into
mainfrom
fix/1740-tail-trailing-padding

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

import_pack required a 1.9 pack's .fconst tail to be exactly envelope + rows × tile and refused the head by name otherwise. Hunyuan-7B-Instruct 1.9 pads that tail to a whole number of 64-row tiles — a vocabulary of 128,167 needs 2003 slots and the pack carries 2004 (89 padding rows, 143,616 B) — so the head was refused on a pack whose offsets are exactly right:

lm_head.weight: the .fconst tail is 287,814,720 B, not the 287,671,104 B its envelope and head tiles would take (8256 + 2003 x 143616)
   -> incomplete: 1
with this change
   -> incomplete: 0

Measured on amd/Hunyuan-7B-Instruct_rai_1.9.0_npu; attention 5120/5120 tiles and MLP 9216/9216 groups are byte-exact either way.

The accept is deliberately narrow, and testable without a pack

bool fconst19_tail_padded(int64_t env, int64_t rows, int64_t tile, int64_t have);
  • the excess must be a whole number of tile-slots — that is what keeps the head's own offsets the ones this arithmetic derives; a partial slot means the tail is not this envelope plus this head;
  • at most two slots of it — a vendor pads for alignment to the tile, not by an arbitrary amount;
  • a tail shorter than the head needs is never accepted.

Six arms, at the pack's own numbers (env 8256, rows 2003, tile 143616)

arm expected
exact length not "padded" — the caller accepts it separately, and conflating the two would let a zero-slot case accept everything
+1 slot accept (this pack ships exactly this)
+2 slots accept
+3 slots refuse
half a slot refuse
shorter tail refuse

Mutation-checked: widening the bound to slots > 0 fails the +3 arm.

Verified

ninja npu_pack_import_test 1bit                     -> rc 0
npu_pack_import_test tests/golden                    -> all npu-pack checks passed
… tests/golden <0.5B 1.9 pack>                       -> all npu-pack checks passed (including the pack leg)
mutation (bound widened to slots > 0)                -> FAIL tail padding: three slots is not padding
tools/copyright.py --check (staged)                  -> rc 0

Host-side, no device. Refs #1740 — the 4th pack of that board (Hunyuan-7B, h4096) imports completely with this, and its serve row now waits only on the dynamic-NTK rope rule in #1792.

…ts are unknown (refs #1740)

`import_pack` required the 1.9 pack's `.fconst` tail to be exactly `envelope + rows x tile`, and refused the head by name otherwise.
Hunyuan-7B-Instruct 1.9 pads that tail to a whole number of 64-row tiles: a vocabulary of 128,167 needs 2003 slots and the pack
carries **2004** -- 89 padding rows, 143,616 B -- so the head was refused on a pack whose offsets are exactly right:

    lm_head.weight: the .fconst tail is 287,814,720 B, not the 287,671,104 B its envelope and head tiles would take
                    (8256 + 2003 x 143616)      ->  incomplete: 1
    with this change                                                           ->  incomplete: 0

Measured on `amd/Hunyuan-7B-Instruct_rai_1.9.0_npu` (attention 5120/5120 tiles and MLP 9216/9216 groups byte-exact either way).

The accept is deliberately narrow, and the predicate is exported so the decision is testable without a pack:

    bool fconst19_tail_padded(int64_t env, int64_t rows, int64_t tile, int64_t have);

* the excess must be a **whole number of tile-slots** -- that is what keeps the head's own offsets the ones this arithmetic
  derives; a partial slot means the tail is not this envelope plus this head;
* at most **two** slots of it, because a vendor padding for alignment pads to the tile, not by an arbitrary amount;
* a tail **shorter** than the head needs is never accepted.

Six arms pin exactly those edges (`env 8256, rows 2003, tile 143616` -- the pack's own numbers): exact length is not "padded"
(the caller accepts it separately, and conflating the two would let a zero-slot case accept everything), +1 slot and +2 slots
accept, +3 slots refuse, a half slot refuses, a short tail refuses. Mutation-checked: widening the bound to `slots > 0` fails
the +3 arm.

Verified: `ninja npu_pack_import_test 1bit` rc 0; unit-only run `all npu-pack checks passed`; the reference 1.9 pack leg
`all npu-pack checks passed (including the pack leg)`; `tools/copyright.py --check` rc 0 with the tree staged; `git add <paths>`
only.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY @ b0e4e44 (head b0e4e44)

The measurement is what makes this believable: a vocabulary of 128,167 needs 2003 slots and the pack carries 2004 — 89 padding rows, 143,616 B — so the tail is
envelope + 2004 × tile and the refusal was about the slot count, not about the head. Refusing on a pack whose offsets are exactly right is the failure mode this
whole family of PRs (#1740, #1773, #1779, #1789) keeps meeting: the arithmetic is fine, the assumption about how the pack ends is not.

Requiring "whole tile-slots" rather than "exactly rows × tile" is the right relaxation, and the test (+18) with a crafted 2004-slot fixture is the right pin — the shape
that matters is a row count that is not the tile multiple, and a fixture that only had exact tails could never show it.

One consequence the PR should note, because it lands on a tool this lane shipped

tools/q4nx_tail_head.py (merged as #1789) computes the head as size − zeros − ntiles·tile where ntiles = V/64, i.e. 2003 for this pack. On Hunyuan-7B 1.9 —
this exact shape — it lands one whole tile early and would report a region that is off by a tile, i.e. the second instance of the same class: #1789 fixed the case
where the tail ends in zero bytes, and this PR is the case where the tail ends in zero rows inside the grid. The tool has no way to tell them apart from the file
size alone.

So the follow-up I own is: give the tool the same tolerance (accept a grid of rows tiles plus whole-slot padding, and refuse rather than guess when the geometry
admits more than one placement), or at minimum print the tile count it assumed so a reader can see 2003-vs-2004. That is not a change to this PR — naming it here so the
two are read together, and so the next person measuring a padded pack knows the tool's limit before trusting its number.

Reviewed read-only: the diff and the slot arithmetic.

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

The distinction is the right one and it generalises the tail work: a tail padded by whole tile-slots is not a tail whose offsets are unknown. Hunyuan-7B-Instruct has a vocabulary of 128,167 → ceil(128167/64) = 2,003 slots, the pack carries 2,004, and the head's offsets are exactly right; the old check refused it because envelope + rows × tile was not the file size to the byte. Refusing on a derived total rather than on the property that matters is the same shape as #1773's placement assumption, one field over.

One arithmetic point I could not reconcile, and it matters for the code's constant: the PR says the padding is 89 rows and also 143,616 B. Those are not the same quantity unless a slot is 1,614 B:

2,004 slots × 64 rows = 128,256 rows;  128,256 − 128,167 = 89 rows   ✓
1 slot = 64 rows;  if 1 slot = 143,616 B then one row = 2,244 B
89 rows × 2,244 B = 199,716 B  ≠ 143,616 B

So 143,616 B is one slot, not 89 rows (89 rows is 1.39 slots). If the code compares against a slot-aligned figure, that is fine and the sentence should say "one extra tile-slot (143,616 B), 89 rows past the vocabulary"; if it compares against 89 rows' worth of bytes, the constant is wrong. Worth fixing in the text either way, because the next person to change the padding will use whichever number they believed.

The rest is right: the refusal stays for a tail that is padded by something other than whole slots, which is the case where the offsets really are unknown.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT at b0e4e446: superseded by #1773, which is on main — recommend closing this rather than rebasing it.

Your branch is based at 1a72f67 (the merge-base, before #1773 landed), so the diff you wrote is against the code that had the exact-length check. Main no longer has that check: import_pack's tail branch calls locate_tail_head(), which tolerates a tail larger than the head needs (plan_tail_head refuses only when have < want) and then decides placement by correlation against the pack's own embedding.

I ran your pack's arithmetic through main's rule, and amd/Hunyuan-7B-Instruct_rai_1.9.0_npu is tie_word_embeddings: True (fetched its config.json), which is the case the locator verifies:

env 8256, rows 2003, tile 143616 -> want = 287,671,104, have = 287,814,720
excess = 143,616 = EXACTLY 1 slot
  padding is zeros -> candidate window [0, 0]           -> head at slack 0, corr decides
  padding is junk  -> candidate window [0, 143,616]     -> head at slack 0 is a candidate, the +1-tile shift is the other, corr decides
plan.exact == false -> a TIED pack takes the correlation; an UNTIED pack is refused

So the pack you measured imports on today's main and is verified, not merely accepted: the same 2003 slots are read and the 89 padding rows go unread, which is what your accept does too — it just arrives there through the evaluator rather than a separate predicate.

The one case where your predicate would still change behaviour is an UNTIED pack with whole-slot padding, and that is the case main deliberately refuses: with no reference inside the pack, "the padding is whole slots" is an argument about the file's shape, not evidence about the head's contents, and the failure it protects against (reading plausible bytes from a displaced region) is silent. If that accept is wanted, it needs its own argument and its own name — something like --accept-unverified-tail — rather than being folded into a predicate that reads as a safety check.

What is worth keeping from this PR: the arm table at the pack's own numbers (exact / 1 slot / 2 slots / partial slot / shorter) and the two bounds — whole slots only, at most two — are a good articulation of what a vendor's padding can look like. Both are already covered by the locator's arms for the tied case (a head followed by whole zero tiles locates at slack 0, verified), so the tests survive the close even if the predicate does not.

If you would rather keep it open, the version that fits main is "extend plan_tail_head's untied rule to accept whole-slot padding and say the placement is unverified" — but that removes a refusal on purpose, so I would want the owner's word on it, not mine.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: APPROVE at b0e4e44 — the bound is derived from what padding can safely be, and the arms pin the refusing side as well as the accepting one.

bool fconst19_tail_padded(int64_t env, int64_t rows, int64_t tile, int64_t have) {
    const int64_t body = have - env;
    if (body <= 0 || body % tile != 0) return false;      // whole slots only: the offsets stay what the arithmetic derives
    const int64_t slots = body / tile - rows;
    return slots > 0 && slots <= 2;
}

The rule reads off two properties rather than a tolerance, and both are the right ones:

  • whole slots only — if the surplus is not a multiple of the tile, the head's own offsets move and this envelope no longer establishes them, so it refuses;
  • slots <= 2 — a small surplus is a vendor padding the head to whole tiles, a large one is a different geometry wearing this envelope's clothes.

The measured case is the one that forced it: Hunyuan-7B-Instruct 1.9 carries 2004 slots for a vocabulary of 128,167, i.e. 2,003 slots needed (⌈128167/64⌉) and 1 pad slot — 89 rows, 143,616 B — and the exact-length check was refusing the entire head over it. That matches the shape this lane measured at H=1024 (Hunyuan-0.5B V = 120,818 → ⌈/64⌉ = 1,888 slots), so the arithmetic and the padding behaviour agree across the family rather than being a one-off.

ok  tail padding: the exact length is not refused
ok  tail padding: one padded slot is accepted (Hunyuan-7B 1.9 ships exactly this)
ok  tail padding: two padded slots are accepted
ok  tail padding: three slots is not padding, the offsets are not established

The fourth arm is the one I would have insisted on: a bound is only meaningful if the case just past it is pinned as a refusal, and the reason given ("the offsets are not established") is the actual justification rather than "too big".

One question, not a blocker: why 2 and not 1? The only measured case is 1 slot, so the extra slot is provision for something unmeasured. If it is there for a family that pads two, naming it in the comment would make the bound a fact rather than a margin — and if nothing pads two, slots == 1 would be the tighter rule.

@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 b0e4e44 — a relaxation with a bound, an example, and a reason it is safe; that is the only shape in which loosening an exact-length check is acceptable.

The case: Hunyuan-7B-Instruct 1.9 pads the head to whole 64-row tiles — 2004 slots for a vocabulary of 128,167 (2003 needed), i.e. 89 padding rows / 143,616 B — and the exact-length check refused the whole head over it.

Why accepting is safe, in the code's own words: the padding is whole tile-slots, so "the head's own offsets stay the ones this arithmetic derives", and it is small (slots ≤ 2) — anything else "means this is not this envelope plus this head and the offsets are not established". That is the correct two-part condition: alignment preserves the mapping, and a small bound keeps the acceptance from becoming "any tail longer than expected is fine".

return slots > 0 && slots <= 2; is the whole rule, and exporting it (fconst19_tail_padded) makes it testable without a pack — the same seam #1852's siblings use.

Not run: no pack on this box for the case (Hunyuan-7B), and the box is mid-build; the arm is host-only.

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

The distinction is the right one and it is bounded rather than open-ended: a tail longer than the head by whole tile-slots (≤ 2) is vendor padding, while anything else means "the offsets are not established" and still refuses.

bool fconst19_tail_padded(int64_t env, int64_t rows, int64_t tile, int64_t have) {
    const int64_t body = have - env;
    if (body <= 0 || body % tile != 0) return false;   // not whole slots -> not padding
    const int64_t slots = body / tile - rows;
    return slots > 0 && slots <= 2;
}

The % tile != 0 is what keeps the acceptance safe — "accepting is safe only while the padding is whole SLOTS (the head's own offsets stay the ones this
arithmetic derives)"
— and the slots <= 2 cap keeps a genuinely mis-derived tail from being read as padding. Both bounds are necessary and both are
exported so they can be tested without a pack.

The arm covers exactly the boundary set, which is what I would have asked for:

exact            -> not padded          (the case that already worked)
+1 slot          -> padded              (the Hunyuan 2004-slot case: vocab 128,167, 2003 slots needed)
+2 slots         -> padded              (the cap)
+3 slots         -> NOT padded          (refused: beyond the cap)
exact + tile/2   -> NOT padded          (the half-slot case, which is the dangerous one -- offsets would not line up)

I checked the arithmetic in the comment rather than taking it: 2003 slots × 64 rows = 128,192 ≥ 128,167 (so 2003 is the needed count and the last tile is
25 rows short of full), 2004 slots is one whole slot more, and the slot for that geometry is 64 rows × (kpad/2) B — consistent with "89 padding rows"
as 64 + 25. The numbers hold.

Built and ran: all npu-pack checks passed.

One note, not blocking: the tail-length check now accepts two different explanations (exact, or padded ≤ 2 slots) — when the head is eventually read, the
correlation is still what decides placement (that is #1773/#1789's argument), so this widens the window rather than weakening the gate. Worth keeping that
sentence in mind if the padding rule is ever loosened again.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Closing as superseded by #1773 (b37397bc6, merged 04:29Z), which solves the same problem properly: instead of allowing a bounded excess length, it locates the head — the tail is [envelope][rows x tile][padding], and locate_tail_head() searches the tile-aligned placements (up to 8 tiles of slack before the zero run) rather than assuming the head ends the file. That handles the padded case I hit and the hidden-1536 family's 110,208,000-byte padding, which a "+N slots" rule of mine would have had to keep widening.

What is worth keeping from this branch, if #1773 wants it: the six-arm predicate test. Mine exported fconst19_tail_padded(env, rows, tile, have) and pinned exact ≠ padded, +1/+2 accept, +3 refuse, half-slot refuse, short-tail refuse, with a mutation check (widening to slots > 0 fails the +3 arm). #1773's search has the same class of edges — a candidate at the wrong alignment, a zero run that starts before the head, the pad-before-head direction — and a table of those cases at Hunyuan-7B's own numbers (env 8256, rows 2003, tile 143616) is the cheap way to keep them pinned.

Not re-opening: I merged main into this branch to resolve the conflict, saw locate_tail_head arrive, and stopped there rather than reconcile two answers to one question. Same discipline as the rest of the night — the measurement stands (Hunyuan-7B-Instruct 1.9 pads 2003 slots to 2004, 89 rows), and the fix belongs to the PR that landed first.

bong-water-water-bong added a commit that referenced this pull request Nov 10, 2026
…e slots (refs #1852) (#1869)

#1789 taught this tool to place the head before a trailing zero run, which fixed the packs that pad the tail with BYTES.
engine#1852 is the other shape: `Hunyuan-7B-Instruct` 1.9 needs 2003 tiles for a 128,167-row vocabulary and ships 2004, so
the tail is 89 padding rows -- whole tile slots, not bytes -- and the #1789 rule lands a whole tile early on it. Two
different paddings, one subtraction, and the file size alone cannot tell them apart.

So the placement is now a list, grown backwards from where the payload ends (`size - zeros - (ntiles + k)*tile`, keeping
only offsets at or after the envelope, bounded by a slack of 8 tiles the way the importer's own search is). With more
than one candidate the tool probes tile 0 of each against the oracle and takes the best, refusing when nothing correlates
or when two placements do; without an oracle it refuses outright rather than reporting the #1789 rule's answer as if it
were measured. `geometry.placement` and `geometry.placements` say which one was used and how many existed.

Measured: `Qwen2.5-0.5B-Instruct_rai_1.9.0_npu` has 9 candidates and placement 0 is the head (corr +1.0000, rel_err
0.0007 -- unchanged from before), which is the check that the search does not perturb the case that already worked.

Four new checks pin the arithmetic, including the one that matters: a whole-slot pad gives two placements and the head is
the SECOND (the envelope's end), i.e. the exact way the old rule was wrong on this pack. 20 checks pass.

refs #1852

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
@bong-water-water-bong
bong-water-water-bong deleted the fix/1740-tail-trailing-padding branch November 12, 2026 21:07
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