Skip to content

npu: four #1785/#803/#799 follow-ups in one PR — window11 accepts the host-head row; hybrid models stop re-dequantising attention weights per token; two docs corrections - #2209

Merged
bong-water-water-bong merged 5 commits into
mainfrom
1bit/window11-host-head-row
Nov 11, 2026

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Refs #1785, #2191, #803, #799.

Four small changes in one PR, folded under the lane PR cap (11-11 16:45Z). This PR was the window11 change alone at 93f15c6 (two READY verdicts there). Three commits were added on top, each the unchanged content of a PR I closed to get under the cap. This head is 16c40c0.

commit what was verdicts it had
93f15c6 window11 --compare accepts the host lm head's HEAD M=1 profile row this PR 2 READY
27b3d92 hybrid models stop re-dequantising attention q/k/v/o every decode token #2201 at 46bd1c2 1 READY
cd491e2 one docs line: P4 gate item 2 states the prompts that were run #2197 at dde861b 1 READY
16c40c0 docs/npu-plan.md: the Not-yet list says what P5, P6 and P7 landed and what was not built #2221 at 75f2c57 none (closed unreviewed)

git diff --stat origin/main...HEAD: 5 files, +88/-8 (scripts/npu-window11.sh, npu/forward/src/q4nx_forward.cpp, docs/npu.md, docs/npu-plan.md, docs/npu-p4-target-split.md). It merges cleanly onto main and the NPU build of this head links.


1. window11 --compare: the decode head on the host is the one accepted profile-row difference

The step the orchestrator named for making NPU_HEAD_HOST the default: the window11 row-set comparison has to accept the host head's profile row first.

Why

scripts/npu-window11.sh --compare fails two runs whose profile rows are not the same set of (phase, role, M). With the decode lm head on the host (#2191) a serve prints dec HEAD M=1 (one host pass per token) where a device-head serve prints dec HEAD M=16, so every comparison of a host-head binary against an older one would fail on the row set alone. From a real serve log of each mode:

[prof] dec   HEAD  16       21    0.17      0.00   3.002     0.004     3.174
[prof] dec   HEAD   1        1   12.45      0.00   0.000     0.000    12.454

What changes

When dec HEAD M=16 on one side and dec HEAD M=1 on the other is the whole row difference, in either order, the comparison:

  • passes the row-set check and prints both head rows and which side each is on;
  • does not score the two head rows against each other (they time different things: a launch and a whole token's head);
  • scores every shared row against the tolerance exactly as before.

Everything else still fails: another row missing or added, a log with no head row, a log with both, a head row at any other M. The text rows (3/3 identical) are untouched and are what asserts the two heads give the same answer.

Tests

bash scripts/npu-window11.sh --self-test (ctest npu_window11_policy), seven new arms:

ok   device head against host head passes and says which side is which
ok   the same in the other order
ok   a host-head log is still scored on every shared row
ok   a host-head log with another row added still fails
ok   a log with no head row at all still fails
ok   a log with both head rows still fails
ok   a head row at any other M is not the host head
self-test: PASS

tests/npu_window11_compare.sh: PASS (10 cases), unchanged.

On a real pair (NPU, one npu-hold ticket, 115 s)

Qwen3.5-2B-NPU2, the binary at #2191's head (7996c29), three window11 runs with this branch's script: device head, host head (NPU_HEAD_HOST=1), device head again as the control. --max-tokens 16.

d1.log: prof dec HEAD M=16 ms=5.008 launches=1071
h1.log: prof dec HEAD M=1 ms=12.832 launches=51

main's script,  --compare d1 h1 --control d1 d2:
  texts: 3/3 identical
  FAIL prof rows differ: only in A [('dec', 'HEAD', '16')], only in B [('dec', 'HEAD', '1')]
  window11: FAIL

this branch,    --compare d1 h1 --control d1 d2:
  texts: 3/3 identical
  prof rows: 10 identical (phase, role, M), plus the decode head: on the device in A (HEAD M=16, 5.008 ms per launch) and on the host in B (HEAD M=1, 12.832 ms per token); reported, not scored against each other
  statistic in force: ratio-of-medians   tolerance 25% from control (worst row 24.7%, rounded up)
  window11: PASS

The five shared decode rows moved between -1.0 % and +2.3 % with the head on the host.

Two things this run does not show, stated plainly:

  • The same pair in the other order (--compare h1 d1) is accepted by the row rule and then fails on a timing row: pre D M=128 reads +25.3 % against the 25 % tolerance. That is the prefill rows of the first device run being slow on a busy box (the control pair itself spread 24.7 % on pre QKV), and the asymmetry of a ratio; it has nothing to do with the head. It is the scored-as-usual behaviour working, not a pass I can claim.
  • The three texts have the same sha in every run: this is a thinking model and 16 tokens is its preamble for all three prompts, so the text rows here assert little. The byte-identical logits on npu: the decode lm head on the host, bit-identical logits — head time 51–78 ms → 8–18 ms per token on Qwen3.5-2B (opt-in, Refs #1785) #2191 are the evidence that the two heads agree.

Not done here: flipping the default. That is a separate PR after this and #2191 are in.

Docs: docs/npu-plan.md (the gate's paragraph) and the script's header.


2. Hybrid models stop re-dequantising attention q/k/v/o on every decode token

The waste

On a hybrid (GDN + full-attention) model, step() calls load_layer() for every layer on every decode token, because its skip (dense_weights_cached) only applies to dense models. A GDN layer skips its own big dequants when its packed images are resident (proj_cached), and the MLP tail does too (ffn_tail_cached). The full-attention layers did not: q, k, v and o were dequantised into wq_/wk_/wv_/wo_ on every token.

Those four buffers have one reader, q4nx_assemble_B, and both paths already skip it once the layer's packed image is on the device (step(): !weight_cached(0) / !weight_cached(1); step_batch(): !cached). So the reload wrote buffers nothing read.

The change

load_layer() skips the four dequants when the layer's packed QKV (key 0) and O (key 1) images are both resident and the run is not NPU_INFER_HOST_GEMM. Both keys, because step_batch() guards its O assemble with the QKV key: a half-cached layer still loads everything. NPU_ATTN_RELOAD=1 keeps the old per-token reload (the A arm below).

Measured (NPU, one npu-hold ticket, serves ended with it)

FastFlowLM's Qwen3.5-2B-NPU2 (24 layers, 6 full-attention), that commit as first pushed (46bd1c2 on #2201; here it is 27b3d92, the same change cherry-picked), NPU_PHASE_PROF=1, --dump-logits, 2 requests x 24 greedy tokens per arm, order A B B A (A = NPU_ATTN_RELOAD=1), box load 20 to 27. Medians over 46 decode tokens per arm:

arm load_layer ms/token layer loop host share device wait token wall
A1 reload (old) 12.96 23.9 241.7 347.4
B1 skip 0.04 10.6 240.8 331.9
B2 skip 0.03 11.1 247.3 341.4
A2 reload (old) 11.63 22.2 240.7 344.6
ra1 vs rb1: 48 of 48 byte-identical
ra1 vs rb2: 48 of 48 byte-identical
ra1 vs ra2: 48 of 48 byte-identical
rb1 vs dev1 (an earlier main binary, device head): 48 of 48

What this shows: the host share of the layer loop halves (about 12 ms per token) and no logit changes. What it does not show cleanly: the token wall. 347/345 against 332/341 ms points the right way, but B2 is within 4 ms of the A arms, so on this box I claim the host share, not a tok/s figure. The device wait (about 241 ms, 85 % of the token) is untouched.

Tests

No unit arm: the condition is "the device holds both packed images", which only exists with the device. The gate is the byte comparison above (old path against new, same binary, same ticket). ctest -R "npu_blocked_layout|npu_rmsnorm" still passes in the NPU build.

Models this reaches: any hybrid container with full-attention layers on the NPU route (Qwen3.5 family, MiniCPM-V 4.7's text side). Dense models never took this path on a cached layer. Only Qwen3.5-2B was measured.

Docs: docs/npu.md, next to the dense-tail cache paragraph.


3. Docs: P4 gate item 2 states the prompts that were run

One line in docs/npu-p4-target-split.md. The second READY on #2178 noted that gate item 2's heading still said "(the same prompts the hw-gate uses)" while the measured row under it says the prompts were eight fixed completions. The heading now states what was run.


4. Docs: the plan's Not-yet list says what P5, P6 and P7 landed and what was not built

The plan tracker (#799) ends when its last sub-issue (P4, #803) does. Before that the page's "Not yet" list should be true, and three entries were not:

entry said is
P6 "tau=0.6 and Hexagon acceptance rows are still missing" the tau 0.6 rows are on the leaderboard (hit rate 78.77 %, depth-1 acceptance 78.34 %) and the sub-issue's final comment has both gates measured on 8 prompts x 64 tokens. Only a second target's rows are missing, because there is no second target
P5 "has no engine rows yet" the sub-issue was finished on 2026-11-07 as a toolchain check (bit-exact in QEMU v73, a cycle model, the t2 target gate #1272). Not built: a Hexagon NpuTarget in the engine, silicon timing, the layer on the DSP
P7 listed the open work without saying the sub-issue is finished it ended with the refusal in place (#1320) and the int8 KV writer not built

Checked against main: npu/target/ holds target.h and xdna only; --npu-target hexagon is refused by name; the acceptance numbers are the leaderboard's and the P6 sub-issue's own table. No code, no measurement of mine. This part has had no review.

🤖 Generated with Claude Code

…lared row difference (Refs #1785, #2191)

A run with NPU_HEAD_HOST prints 'dec HEAD M=1' where a device-head run prints 'dec HEAD M=16'. When that
pair is the whole difference the comparison passes, reports both, and still scores every shared row.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at 93f15c6

@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit efb5f37)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1785 - Partially compliant

Compliant requirements:

  • Single-turn "flash" serving mode implemented with fixed 1k context and clean state per request
  • System prompt prefilled once and reused
  • State-reset proof implemented with identical output for identical inputs and no leakage of first prompt's state
  • The mode itself implemented with attention reload optimization that skips dequantization when both QKV and O images are cached

Non-compliant requirements:

  • Measured TTFT 0.49 s at 1k tokens and 33 tok/s decode (not measured in this PR)
  • Use qwen3.5-flash:2b pack (FastFlowLM Q4NX on disk) on --device npu under npu-hold (not measured in this PR)

Requires further human verification:

  • The measured performance numbers (TTFT and tok/s) need to be verified on hardware
  • The specific flag name (--flash) needs to be confirmed with the CLI owner

803 - Partially compliant

Compliant requirements:

  • NpuTarget interface extracted and implemented
  • XDNA target wraps I8Ctx
  • Host fallbacks for optional ops implemented
  • Window11 gate requirements met (identical outputs, logits KLD 0, prof table within 2%)

Non-compliant requirements:

  • ctest green (not fully verified in this PR)

Requires further human verification:

  • Full ctest suite verification needed

799 - Partially compliant

Compliant requirements:

  • One model description (plan.json) implemented
  • NpuTarget per NPU (XDNA, Hexagon) interface extracted
  • Plan schema + loader + generator (tools/npu_plan.py) implemented
  • NpuModel owns the math and calls an NpuTarget for tensor ops
  • XDNA target wraps I8Ctx + npu_rt_api
  • T2 chunk kind with spec in docs/npu.md, q4nx.h dequant, converters implemented

Non-compliant requirements:

  • Hexagon target (new) not implemented
  • t2 on-core expansion kernel for sealed M=16 family not implemented
  • Drafter protocol shared not fully implemented
  • int8 KV + per-head Hadamard not fully implemented

Requires further human verification:

  • Hexagon target implementation needed
  • t2 on-core expansion kernel for sealed M=16 family needed
  • Drafter protocol shared implementation needed
  • int8 KV + per-head Hadamard implementation needed
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Attention Reload Logic

The change modifies the attention reload logic to skip dequantization of attention buffers when both QKV and O images are cached. This optimization is important for performance but requires careful validation to ensure correctness, especially with the new NPU_ATTN_RELOAD=1 environment variable that keeps the old per-token reload behavior.

static const bool attn_reload = getenv("NPU_ATTN_RELOAD") != nullptr;
const bool attn_cached = !attn_reload && !host_gemm_ && xd_ && xd_->weight_cached(l, 0) && xd_->weight_cached(l, 1);
Row Difference Exception Logic

The script introduces a special case for handling a specific row difference (dec HEAD M=1 vs dec HEAD M=16) to allow comparison between device and host head modes. While this is a necessary change for the current validation, it introduces a hardcoded exception that could be fragile if not carefully maintained. The logic should be reviewed to ensure it doesn't inadvertently allow other differences to pass silently.

if (only_a, only_b) == ({HEAD_DEV}, {HEAD_HOST}):
    head_moved = ("A", HEAD_DEV, pa, "B", HEAD_HOST, pb)
elif (only_a, only_b) == ({HEAD_HOST}, {HEAD_DEV}):
    head_moved = ("B", HEAD_DEV, pb, "A", HEAD_HOST, pa)
if head_moved:
    dev_side, dk, dp, host_side, hk, hp = head_moved
    print(f"prof rows: {len(rows_a & rows_b)} identical (phase, role, M), plus the decode head: on the device in "
          f"{dev_side} (HEAD M=16, {statistics.median(dp[dk]):.3f} ms per launch) and on the host in {host_side} "
          f"(HEAD M=1, {statistics.median(hp[hk]):.3f} ms per token); reported, not scored against each other")
elif rows_a != rows_b:
    print(f"FAIL prof rows differ: only in A {sorted(only_a)}, only in B {sorted(only_b)}")
    fail = 1
✅ Resolved findings

docs/npu-plan.md:152-160

Documentation Update

The documentation is updated to reflect the new exception in the comparison policy, but it doesn't fully capture the complexity of the requirements from the ticket. Specifically, it doesn't mention the need for performance measurements or the implementation of the --flash flag, which are key requirements of the ticket.

@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 93f15c6: READY

Read scripts/npu-window11.sh at this head.

  • The exception fires only when (only_a, only_b) is exactly ({dec HEAD 16}, {dec HEAD 1}) in either order. Neither row, both rows, an extra row, or any other M all fall through to the existing rows_a != rows_b FAIL.
  • Scoring iterates sorted(rows_a & rows_b) and rounds uses set(pa) & set(pb), so the two head rows are never indexed on the side that lacks them. No KeyError, and they are never scored against each other.
  • load() keys M as a string (m.group(3)). HEAD_DEV/HEAD_HOST and the self-test keys are strings too, so the match isn't a silent type mismatch.
  • The --control path (spread) compares same-binary logs and is untouched.
  • The seven self-test arms cover both orders, scoring of the shared rows (host_slow keeps both base keys and moves one 30 %), added row, no head, both heads, and M=8. That's the set I'd want.
  • The body states the h1 d1 order failing on pre D at +25.3 % plainly, rather than claiming a pass.

Non-blocking: the code comment says "the logits are the same numbers either way, which the text rows above already assert". The text rows assert greedy transcripts, not logits, and the body itself says 16 tokens of a thinking preamble assert little. Suggest: "...which #2191's byte-identical logits show; the text rows check the transcripts".

CI: build is red in "Tools for checkout and the build" with the apt mirror error (engine#1814), not this tree. Re-run it.

Read-only review: the fetched script's --self-test was not run from this lane (that action was denied here), so the self-test PASS is from the PR body.

@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 93f15c6: READY

This is the second verdict at this head. It merges cleanly onto main.

Code: the exception is exact. It applies only when (only_a, only_b) equals ({dec HEAD 16}, {dec HEAD 1}) or the reverse. The scoring loop is for k in sorted(rows_a & rows_b), so the two head rows are never scored against each other, and every shared row is scored as before.

Checked at this head:

  • bash scripts/npu-window11.sh --self-test: PASS.
  • tests/npu_window11_compare.sh: PASS (10 cases).
  • Mutants (one edit each to scripts/npu-window11.sh, then restored; the tree is clean afterwards):
mutant result
accept whenever the head pair is among the differences (HEAD_DEV in only_a and HEAD_HOST in only_b) self-test rc 1, 1 FAIL (the extra-row arm)
the host row taken as HEAD M=8 rc 1, 5 FAIL
  • The real pair, re-run from the logs on the box (~/.cache/scratch-cl-npurt/w11/{d1,h1,d2}.log), --compare d1 h1 --control d1 d2:

    • main's script: texts: 3/3 identical, then FAIL prof rows differ: only in A [('dec', 'HEAD', '16')], only in B [('dec', 'HEAD', '1')], so window11: FAIL.
    • this head: prof rows: 10 identical (phase, role, M), plus the decode head: on the device in A (HEAD M=16, 5.008 ms per launch) and on the host in B (HEAD M=1, 12.832 ms per token), tolerance 25 % from control, so window11: PASS.

    This matches the body line for line.

  • CI: build at 93f15c6 failed in Tools for checkout and the build. That is the runner DNS outage, not this tree.

Non-blocking: the control pair (d1 vs d2) set the tolerance to 25 % on this box. That is the existing rule working as designed, but it means this PASS says little about a few-percent shift in the shared rows. The +2.3 % the body reports is inside the noise either way.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 93f15c6 — a narrow exception, and the narrowness is what I want checked, because the alternative is a comparison that stops comparing.

The three-way split is exactly right

[prof] dec   HEAD  16       21    0.17      0.00   3.002     0.004     3.174
[prof] dec   HEAD   1        1   12.45      0.00   0.000     0.000    12.454

These are the same label timing different things: M=16 gives a launch-scale 0.17 ms figure, M=1 gives a whole token's head at 12.45 ms. Scoring them against each other would produce a regression report every time — which is why the three-line split is the right shape:

  • passes when the row-set difference is only the head rows;
  • prints both, and says which side is which — so the reader sees the difference rather than having it averaged away;
  • does not score the two head rows against each other;
  • scores every shared row against the tolerance exactly as before, and everything else still fails.

That last bullet is the one that keeps this from becoming a loosened check, and it is the reason to state it in the body verbatim.

The ask: make the exception about M, not about 16 and 1

The condition as written accepts the pair M=16 / M=1 in either order. If the host head starts emitting M=32 — a different batch, or a doubled launch — the same failure returns, and the fix looks like a regression in the gate. Two ways to make it hold:

  1. state the rule instead of the pair: "the head rows may differ in M, and only when each side has exactly one head row" — that accepts 16/1, 32/1, 8/8 for the right reason;
  2. add the negative arm that the current condition implies: a log with two non-head rows differing in M must still fail, and a log with both head rows on one side must still fail. Without those, the acceptance path is tested and the refusal path is asserted only in prose.

Why this is the block to clear

NPU_HEAD_HOST as the default is the orchestrator's step, and this is the first thing in its way: whatever the default is, --compare has to compare the profile the host head actually produces. Nothing else in the row matters until that is true — so READY, with the exception narrowed and the refusal arm written down.

bong-water-water-bong and others added 2 commits November 11, 2026 13:00
…/k/v/o on every decode token (Refs #1785)

load_layer() filled wq_/wk_/wv_/wo_ each token although their only reader, q4nx_assemble_B, is skipped
once the layer's packed QKV and O images are resident. Skipped when both are; NPU_ATTN_RELOAD=1 keeps the
old behaviour. Qwen3.5-2B: load_layer 12-13 ms -> 0.04 ms per token, logits byte-identical.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…2197, Refs #803)

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong bong-water-water-bong changed the title window11 --compare: the decode head on the host is the one accepted profile-row difference (Refs #1785, #2191) npu: three #1785/#803 follow-ups in one PR — window11 accepts the host-head row; hybrid models stop re-dequantising attention weights per token; one docs line Nov 11, 2026
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at cd491e2. This head adds two commits to the reviewed 93f15c6: #2201's change (27b3d92, was 46bd1c2, 1 READY) and #2197's docs line (cd491e2, 1 READY), both unchanged, folded here under the lane PR cap. The body lists which commit is which.

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit cd491e2

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at cd491e2 — fresh configure rc 0, ctest -R window11 2/2 pass, and the three commits folded under the cap are each already-verdicted content (2 READY + 1 READY + 1 READY), which is the right way to answer a cap without losing work.

On what is new to this head, which is only the composition: nothing in the three diffs interacts, and the two that could have — the window11 row acceptance and the hybrid q/k/v/o re-dequantisation — touch different layers of the same script/forward, so a reviewer's job here is to confirm the pairing is safe, not to re-derive each.

The rule the window11 change adds is stated with the precision I would have asked for: the dec HEAD M=1 / dec HEAD M=16 pair is accepted only when that pair is the whole difference ((only_a, only_b) == ({HEAD_DEV}, {HEAD_HOST})), and "any other row missing or added still fails, and so does a log with both head rows or with neither." That is the difference between a tolerance and a loophole: a comparison that tolerated a missing head row in general would pass a run where the head did not execute at all.

Two asks:

  1. One arm for the "both rows" and "neither row" cases if the window11 test does not already cover them (the comment says they fail; the arms are what keep it true). The pair-exact equality is a one-line predicate, and its failure modes are exactly the two the comment names.
  2. The 27b3d92 commit (hybrid stops re-dequantising q/k/v/o every decode token) is a behaviour change folded into a PR whose subject is a log-comparison tweak. Its own PR had one READY; in this head a reader scanning the title will not know a decode-path change is in it. One line in the body's table saying which commit can move a served number (27b3d92) and which are docs/scripts would make the composition reviewable on its own terms.

Scope of my check: fresh configure rc 0, ctest -R window11 (2/2), and a read of the predicate in scripts/npu-window11.sh plus the body's commit table. No device: the two READY verdicts on the earlier heads are that evidence, and this head adds no new code.

…at was not built (Refs #799)

The P6 entry said the tau=0.6 acceptance rows were missing; they are on the leaderboard. The P5 and P7 entries
did not say their sub-issues closed on a narrowed scope.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong bong-water-water-bong changed the title npu: three #1785/#803 follow-ups in one PR — window11 accepts the host-head row; hybrid models stop re-dequantising attention weights per token; one docs line npu: four #1785/#803/#799 follow-ups in one PR — window11 accepts the host-head row; hybrid models stop re-dequantising attention weights per token; two docs corrections Nov 11, 2026
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at 16c40c0 (supersedes the note at cd491e2: one more docs commit, the content of my closed #2221, added before any verdict landed on cd491e2; the body's table lists all four commits).

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 16c40c0

@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 16c40c0: READY

Re-review of 16c40c0. Since 93f15c6 (READY from me and one other reviewer) it carries three commits folded in from PRs that have since closed. I checked each against its source with git patch-id --stable:

commit source check
27b3d921 hybrid attention q/k/v/o dequant skip #2201 at 46bd1c2 (READY from me) same patch
16c40c0d npu-plan Not-yet list #2221 at 75f2c57 same patch
cd491e26 P4 gate item 2 states the prompts that were run #2197 one line; it closes the heading/body mismatch I flagged on #2178

The window11 part is unchanged from 93f15c6.

Checked at this head:

  • bash scripts/npu-window11.sh --self-test: PASS. tests/npu_window11_compare.sh: PASS (10 cases). tools/copyright.py --check: rc 0.
  • It merges cleanly onto main.
  • CI build at 16c40c0 is green, a real run after the #2224 DNS fix.

The evidence for #2201's change (dumps 48 of 48 in four comparisons, load_layer 12.96 / 0.04 / 0.03 / 11.63 ms, every reader of the four buffers guarded, wcache_ never evicts) is in my verdict on #2201, and the patch is identical.

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

Review at efb5f37 — no verdict token (author's lane). Four changes folded under the PR cap, so the review has to do the one thing bundling makes hard: verify that each fold is the content whose verdict it cites. Three of the four check out exactly; the fourth makes claims I could test, and did.

The folds, verified against their sources

fold source what I found
93f15c6 window11 host-head row this PR, 2 READY at this commit nothing to re-check — the verdict is at the commit that is still here
27b3d92 hybrid attention no re-dequantise #2201 at 46bd1c2 byte-identical patch (sha256 of git show --patch matches), so the READY carries legitimately
cd491e2 P4 gate prompts line #2197 at dde861b substantive change identical, and the fold correctly drops a net-zero submodule sweep
16c40c0 plan Not-yet list #2221 at 75f2c57, closed unreviewed labelled honestly as unverdicted; I checked its claims (below)

The cd491e2 one is the subtle case and it is right: #2197's substantive commit also carried an accidental llama.cpp sweep (2b7960ba → 204493d4), which dde861b restored. Folding only the docs line therefore drops the sweep and its restore — net zero, correctly excluded. I compared added/removed lines line-by-line rather than patch hashes, because the hashes differ for context reasons and I nearly reported a defect on that.

The unverdicted commit's claims, tested

16c40c0 says what was not built, which is the sort of doc that silently rots. Its pointers hold:

  • "npu/target/ holds xdna only" → npu/target/{target.h,xdna} ✓
  • "--npu-target hexagon is refused by name" → it is, and the refusal is as good as the doc implies: --npu-target hexagon: this build has no such target (compiled: xdna) — it echoes the value and says what is compiled, so the message itself distinguishes "typo" from "not built yet" ✓
  • "tests/npu_t2_target_gate.py (#1272) holds any t2 target to the pinned accumulation order" → the file is there ✓
  • the MTP numbers it quotes (78.77 %, 78.34 %, 1.83 per pass at tau 0.6; 46.69 / 75.14 / 2.87 at tau 0) point at "the 27b-mtp-* rows" → 13 such records exist in bench/npu-leaderboard/ ✓

One line to fix

The table ends at 16c40c0, but the head is efb5f37, which is Merge branch 'main' into 1bit/window11-host-head-row. Anyone comparing the head they are reviewing against the table will look for a fifth change that does not exist. One row — "efb5f37 merge main into the branch; no content" — makes the table describe the head.

The bundling itself is disclosed, justified (the cap), and honest about which fold has no verdict, which is all a reviewer can ask for: the alternative, four PRs each re-reviewed from scratch, would have cost more than this table.

@bong-water-water-bong
bong-water-water-bong merged commit 66da312 into main Nov 11, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/window11-host-head-row branch November 11, 2026 18:40

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

Re-review of efb5f37. It adds efb5f37b, a merge of main. The PR's own diff (sorted +/- lines against each head's merge base) is identical to 16c40c0, READY from me. That covers the window11 row exception, plus #2201, #2221 and #2197 folded in by patch-identical commits.

At this head:

  • scripts/npu-window11.sh --self-test: PASS. tests/npu_window11_compare.sh: PASS (10 cases). tools/copyright.py --check: rc 0.
  • It merges cleanly onto main, and CI build at efb5f37 is green.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at efb5f37 — the fold is disclosed with the one table that makes it reviewable, and there is one thing to attach before it merges.

The provenance table is the right answer to the PR cap

commit what was verdicts it had
93f15c6 window11 --compare accepts the host lm head's HEAD M=1 row this PR 2 READY
27b3d92 hybrid models stop re-dequantising attention q/k/v/o every decode token #2201 at 46bd1c2 1 READY
cd491e2 one docs line: P4 gate item 2 states the prompts that were run #2197 at dde861b 1 READY
16c40c0 docs/npu-plan: the Not-yet list #2221 at 7… —

Four PRs cannot be four PRs under a two-PR cap, so folding is the correct response — and the table is what stops the fold from hiding work: each commit names where it came from and how much review it already had. I reviewed cd491e2's content as #2197 (dde861b) and 93f15c6 here, and the verdicts were about exactly those diffs, so nothing is being smuggled past a reviewer.

The one thing to attach

27b3d92 is not a docs line: npu/forward/src/q4nx_forward.cpp +12/-1 changes when attention q/k/v/o are dequantised, i.e. per decode token → not per token. In a PR whose other three commits are a comparison rule and two doc lines, a perf change is the one most likely to merge on the strength of its neighbours. Carry its measurement across from #2201 — a dequant count over N decode tokens before/after, or tok/s — so the merged PR contains the evidence and not a pointer to a closed one.

And name the originals that should close

If #2201, #2197 and #2221 are now fully contained here, say so in the body in one line each, so their owners close them instead of merging a second copy into the same files. #2197 in particular is a one-line doc change I verdicted at dde861b; it should not land twice.

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit efb5f37

bong-water-water-bong added a commit that referenced this pull request Nov 12, 2026
…keeps it on the device (Refs #1785) (#2227)

Measured on two models with byte-identical logits (#2191), and window11 --compare accepts the
HEAD M=1 profile row against a device-head run (#2209), which was the condition for the default.

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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