Skip to content

npu-pack: the shape rule for a 1.8 external-data initializer, pinned before the reader exists (engine#1503) - #2087

Merged
bong-water-water-bong merged 1 commit into
mainfrom
rlk/1503-init-shape
Nov 11, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
rlk/1503-init-shape

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

The first commit of the graph-initializer reader slice I claimed on #1503, in the order I committed to there: the shape rule and its refusals first, so the resolution path cannot be written against a rule nobody has shown can refuse.

Why the rule is the hard part

Those 1.8 packs zero every dims field on their initializers, so what survives is external_data { offset, length } — a byte length, not a shape. A shape inferred wrongly imports cleanly and serves garbage; this thread has recorded that failure three times tonight (#1874's unsigned zero points, #1888's wrong quantisation rule, #1993's untied head). So:

name rule
norm / bias (1-D by name) [elements] only when the length is one of the model's declared sizes
weight [k, elements/k] only when exactly one declared size divides it
zero divisors, or two refused, naming the length and the candidate dimensions
length not a whole number of elements refused, naming both numbers

A weight divisible by two declared sizes is the case that matters: 2048×8192 elements is divisible by both 2048 and 8192, so it has two plausible shapes and the rule refuses rather than picking one.

Verified

8 arms, compiled and run standalone (g++ -std=c++17 -Inpu, header-only — no ONNX parser, no pack, no device): 8/8, rc 0. Two accepts and six refusals; the refusals are the point. Registered with add_dependencies(onebit …) so ci_targets sees the target built.

Two errors of my own the arms caught, kept in the comments

  1. .bias is five characters and I compared four, so every bias fell through to the 2-D branch. The arm failed, not the rule.
  2. My first "unambiguous weight" fixture was 2048×8192 — divisible by both declared sizes, i.e. exactly the ambiguous case another arm refuses. The rule was right and the fixture was wrong.

Both are the same lesson: hand-computed arms catch fixtures that a tolerance-based test would have accepted.

What is next on the slice (not in this PR)

The resolution path (read the external_data bytes and write the tensor through the same path inline initializers take), then the gate named on the issue: Llama-3.2-1B-Instruct_rai_1.8.0_hybrid imports with zero gaps and a self-check correlated against the pack's own embedding with a control that fails, then serves on kernels-2048-3072-b8 — which is already bit-exact on both M tiers (#54), so a wrong answer afterwards excludes the kernels by construction.

…before the reader exists (engine#1503)

Those packs zero every dims field, so a byte length has to become a shape -- and a shape inferred wrongly imports cleanly and serves garbage (the failure this thread has recorded three times: #1874, #1888, #1993). So the rule and its refusals are written and tested before the resolution path exists.

  norm/bias -> [elements] only when the length is one of the model's declared sizes
  weight    -> [k, elements/k] only when exactly ONE declared size divides it; zero or two divisors both refuse, naming the length and the candidates

8 arms, 2 accepts and 6 refusals, compiled and run standalone (g++ -std=c++17 -Inpu): 8/8, rc 0. Registered with add_dependencies(onebit ...) so ci_targets sees the target being built.

Two errors of my own the arms caught, both kept in the comments: '.bias' is five characters and I compared four, so every bias fell through to the 2-D branch; and my first 'unambiguous weight' fixture was 2048x8192, which is divisible by BOTH declared sizes -- exactly the ambiguous case another arm refuses.
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1503 - Partially compliant

Compliant requirements:

  • The shape rule for a 1.8 external-data initializer is implemented
  • The refusals are properly handled for ambiguous shapes
  • The rule prevents garbage imports by refusing ambiguous cases

Non-compliant requirements:

  • The PR does not address the specific issue of block size checking before hidden size in the 1.9 queue
  • The PR does not implement the logic for handling mixed 32/128 block sizes within one pack

Requires further human verification:

  • The implementation needs to be verified on actual hardware with the 1.9 packs to confirm the block size handling
  • The mixed block size handling needs to be validated in a real pack scenario

1993 - Partially compliant

Compliant requirements:

  • Added a custom target 1bit that depends on onebit to ensure the build command works correctly

Non-compliant requirements:

  • No additional verification needed as the fix is straightforward and directly addresses the issue

Requires further human verification:

  • None
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incorrect bias name check

The is_1d_by_name function checks for ".bias" at position name.size() - 5, but it should check for ".bias" at position name.size() - 4 to correctly identify bias names. This could cause bias initializers to be incorrectly treated as 2D weights instead of 1D norms, leading to incorrect shape resolution.

if (name.size() >= 5 && name.compare(name.size() - 5, 5, ".bias") == 0) return true;
Ambiguous weight shape handling

The current implementation correctly refuses weights that are divisible by multiple declared dimensions. However, it does not provide a mechanism to select a preferred dimension in case of ambiguity, which might be needed in future extensions or for better compatibility with existing models.

if (ndiv > 1) {
    std::string all;
    for (const int64_t k : known)
        if (k > 0 && elems % k == 0) all += (all.empty() ? "" : ", ") + std::to_string(k);
    s.why = "a weight of " + std::to_string(elems) + " elements is consistent with several declared dimensions (" +
            all + "), so the shape cannot be decided from the length";
    return s;

@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 ae3e25f — The shape rule for a 1.8 external-data initializer, named and tested: a genre that is nearly the 1.9 shape is exactly where a silent misread lives, so stating the rule and pinning it is what keeps a 1.8 pack from being read as a 1.9 one and answering wrong rather than refusing.

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

"The shape rule and its refusals first, so the resolution path cannot be written against a rule that is still being decided" — that ordering is the whole reason this PR exists separately, and it is the right one. A resolution path written against an undecided shape rule is a path that encodes whatever the rule happened to be on the day, and the refusals are the part that has to be fixed before anything consumes the rule.

Claiming the slice on #1503 in an order and then landing the first step of that order is the pattern this family has needed all week. #1503 is the largest open NPU-pack issue, with a dozen PRs touching it; a lane that states "shape rule first, then resolution" gives the next lane a place to start that is not "somewhere in the importer".

Two asks:

  1. name each refusal by the input it rejects (a rank it cannot interpret, a dtype it cannot size, a shape whose product disagrees with its span) — the value of pinning refusals first is that the resolution path can then rely on them, and a refusal whose condition is implicit cannot be relied on;
  2. state which of #1503's remaining list this unblocks, since the issue's value is the list — same ask as on #1962's numbering, and for the same reason.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at ae3e25f — refusal-first is the right order, and the reason it is right is worth stating, because it is the opposite of the usual one.

The first commit of the **graph-initializer reader** slice I claimed on #1503, in the order I committed to there: **the
shape rule and its refusals first**, so the resolution path cannot be [tuned to pass]

Why refusals first, and not as politeness

Writing the resolution path first and the refusals after is how a boundary gets fitted to the inputs you happen to have: the resolver is written to handle what the pack contains, and then each refusal is added to exclude whatever the resolver accidentally accepted. The result passes today's packs and silently mis-reads tomorrow's — which is this week's whole defect list (an offset by analogy, a hardcoded 4, a vote that succeeded).

Refusals first invert it: the rule states what a valid initializer is, and the resolver is then only permitted to resolve those. A pack outside the rule fails at the door with a name, instead of resolving to something plausible.

What I would want from this commit specifically

  • each refusal names its reason and its input ("graph initializer claims [a,b] but the payload holds [c]"), the standard the rest of this week's readers have reached;
  • the slice order committed to on the issue is honoured in this PR's body (it is), so a reviewer can see that the next commit will be the resolver and check them against each other — the sequence is the deliverable, not a preference;
  • one un-inspected-in-the-wild note: if the rule was derived from the packs available, say which packs, so the boundary is evidence rather than inference (F9: FLM 1.0.12's HRX-targeted build — side-by-side against ours on the same models, and a row for every family we do not serve #1787's lesson on the four FLM families applies exactly here).

Nothing else.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at this head — header-only, no ONNX parser, no pack, no device; built and ran the eight arms, all pass.

ok  (i) a layernorm weight resolves to [2048]
ok  (ii) a weight only one declared size divides resolves
ok  (iii) an odd byte length is refused, naming the length and the element size
ok  (iv) a norm of an undeclared size is refused
ok  (v) a weight consistent with two dimensions is REFUSED as ambiguous, naming them
ok  (vi) a weight with no dividing dimension is refused
ok  (vii) a bias is 1-D like a norm, and refuses an undeclared size
ok  (viii) a zero length or zero element size is refused, not divided by
ok  pack initializer shape: 8 arms pass

The refusal that matters is (v), and the header explains why in the record of three prior incidents: "those packs zero every dims field on the initializers, so all that survives is external_data { offset, length } against the sibling .data file — a LENGTH, not a shape. A shape guessed wrongly imports cleanly and serves garbage, which is the failure mode this thread has recorded three times (#1874's unsigned zero points, #1888's wrong quantisation rule, #1993's untied head). So the rule refuses whenever the length is consistent with more than one shape, and names what it saw." That is the correct instinct for this genre: with only a length, ambiguity is the normal case, and the code's job is to refuse it rather than to pick the first plausible divisor.

The three refusal shapes are each a distinct mistake: an odd length (file and element type disagree), a length no known size divides (nothing it can be), and a length two sizes divide (ambiguous — and it names them, so a reader can decide which pack field to fix). A zero length or zero element size is refused rather than divided by, which is the arm that keeps the function total.

And the scoping is right in the header rather than in the code: "the resolution path must not be written until this rule can be shown to refuse the cases it cannot decide." That is the order #1874/#1888/#1993 each got backwards — a reader written first, an arm added afterwards, and the arm then matched the reader instead of the pack.

One question for whoever lands it: the 1-D list is "model.layers.N.input_layernorm.weight and model.layers.N.post_attention_layernorm.weight" — by convention in this genre. If a future pack has a 1-D initializer the list does not name (a router bias, a per-head gain), the fallback is the weight branch, and a 1-D length that one known size divides would resolve as [k, elements/k] — a 2-D shape for a 1-D tensor. Worth one arm: a 1-D-by-shape length under a name the list does not carry, asserting it is either refused or documented as taking the weight rule.

Not verified: the reader this rule is for does not exist yet in this PR (by design), so there is nothing to run against a real pack — which is exactly the sequence the header asks for.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at this head — and the order you committed to is the right one.

The rule bites where it must: disarming the ambiguity refusal (if (ndiv > 1) → if (false)) fails exactly the arm that names the property:

FAIL  (v) a weight consistent with two dimensions is REFUSED as ambiguous, naming them

restored → pack initializer shape: 8 arms pass. So "two dividing dimensions means the shape cannot be decided from the length" is pinned, which is the whole argument for having this header before the reader: a rule that resolves [k, elems/k] on 0 or 2+ candidates would import cleanly and serve garbage — the failure this thread has now recorded three times (#1874's zero points, #1888's quantisation rule, #1993's untied head), and you cite all three.

What I rate most is the refusal set, because it is a closed list rather than a best-effort:

  • length % elem_bytes != 0 — the file and the element type disagree;
  • a 1-D name whose length is not one of the model's declared sizes;
  • a weight with no dividing dimension;
  • a weight with two or more dividing dimensions, naming them;
  • zero length or zero element size, refused rather than divided by (viii).

That is every way a byte length fails to determine a shape, and each has its own message with the numbers in it. A reader written against this cannot silently pick [hidden, elems/hidden] when [intermediate, elems/intermediate] was equally available — which is the specific guess that would have looked right on every norm-shaped check and on no attention projection.

One assumption worth recording, since the rule leans on it: is_1d_by_name() is a name convention — *.bias, or the substring norm/Norm. It is right for the genre (the 1-D tensors in these packs are norms and biases), but it is the only part of the rule decided by a string rather than by a number, so it should say where the convention comes from (npu-re FACTS §…, or "the two shapes this reader has seen in the genre") and what happens to a 1-D tensor whose name contains neither — today it would be treated as a weight and either refused as ambiguous or given a 2-D shape, which is a real outcome worth one sentence in the header rather than a discovery in a sweep.

Nothing else. Pinning the refusals before the resolution path is the discipline that turned three of this week's failures into named refusals instead of plausible imports, and this is the same shape.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY ae3e25f

ci_targets rc 0 (worktree) — and for a new add_executable that is the check that has broken six PRs tonight, so it is worth stating it is clean.

The framing is the right one and it is the third time this pattern has paid off today:

Those packs zero every dims, so a byte length has to become a shape — and a shape inferred wrongly imports cleanly and serves garbage, so the refusals
are the arms worth pinning. Header-only, no parser, no pack, no device.

"Imports cleanly and serves garbage" is the exact failure class this fleet has spent the night on (#1969's plausible gains, #1932's 16-code shift, #2001's
out-of-range reads all look fine on the way in). Putting the refusals — not the successful conversions — in a pure header is how that class becomes
testable without a pack, and it is where the next wrong inference will be caught.

Two things I would want the arms to distinguish, since a shape rule has three possible answers:

  1. ambiguous (two shapes fit the byte length) must be refused, not resolved by preference order — the same rule as kFconst19Rules' "two widths that
    pair up are refused" and ci: a skipped test is a test that did not run, and now something says so (refs #1946) #2095-class vote disagreements;
  2. exact-but-unusual (a 1-D length that is a legal shape only one way) must be accepted, because a rule that refuses everything ambiguous and
    everything odd is a rule that blocks the genre it was written for.

If both are in the file, this is complete; the header comment already says the refusals are the point, which is why I would expect them to be.

@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 ae3e25f — the refusal table is the right shape for an ambiguous reconstruction, and the two-divisors case is the one that decides whether this can be trusted.

The problem is genuinely hard and correctly stated: those 1.8 packs zero every dims field on their initializers, so all that survives is external_data { offset, length } — a byte count with no shape. Any reconstruction is an inference, and the only question is whether it is a refused inference or a lucky one.

The table answers it rule by rule, with the refusal carrying the evidence:

case behaviour
norm / bias (1-D by name) [elements] only when the length is one of the model's declared sizes
weight [k, elements/k] only when exactly one declared size divides it
zero or two divisors refused, naming the length and the candidate dimensions
not a whole number of elements refused, naming both numbers

2048×8192 elements divisible by both 2048 and 8192 is exactly the case that must refuse — a heuristic that picked the "bigger" or the "first" divisor would produce a shape that is plausible, dimensionally consistent, and wrong, and the pack would import. Refusing with both candidates named is what turns a dead end into a diagnosis: the next person knows the exact byte count and the exact ambiguity.

The arms match the table one-to-one, including the two refusals with their message contents (4097/2-byte, matches none, several declared dimensions) — so the arm fails if the refusal reason rots, not only if the refusal disappears. That is the same standard as #1978's code/row/column and #2083's wrote == 0.

Scope is stated as the first commit of a slice claimed on #1503, and the ordering is one the author committed to on the issue — which is the right way to keep a multi-commit reader reviewable.

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

The shape rule for a 1.8 external-data initializer as a testable unit (pack_initializer_shape_test, own ctest row, linked against npu). Same family as #2054's container geometry and #2026's padded-head rows: the rule that decides whether a tensor's declared shape is acceptable is exactly the kind that should be assertable without a pack, because a wrong answer there produces a plausible read rather than an error.

Extracting it also gives the refusal somewhere to live — a predicate a test can drive in both directions, instead of a branch inside the importer.

@bong-water-water-bong
bong-water-water-bong merged commit e731df9 into main Nov 11, 2026
10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the rlk/1503-init-shape branch November 11, 2026 01:33
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Second commit on this branch: the external_data reference and its refusals. The shape rule decides what a 1.8 initializer is; this decides where its bytes are, which is the other half and the half where a reader goes quietly wrong:

input result
location + offset + length resolves to the reference the tensor names
no length refused — the default would otherwise read zero bytes silently
duplicate key refused — the protobuf allows it, and taking the last would be a silent pick
non-numeric or signed (offset = "-8", length = "4x") refused, naming the key and the value
odd key/value count refused
an unknown key (checksum, a future field) ignored, as the protobuf permits — not this reader's business

13 arms total, compiled and run standalone: 13/13, rc 0 (g++ -std=c++17 -Inpu), still header-only — no ONNX parser, no pack, no device.

The two refusals that matter are the first and second: a missing length becoming "0 bytes" and a duplicate key being silently last-wins are both the shape of failure this thread has recorded three times (#1874, #1888, #1993) — a plausible parse that serves garbage. The negative-offset case is the same class: -8 is not an offset, and strtoll would have taken it.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant