[bong-water-water-bong]

Third piece of the vision port: block_forward() — norm → qkv → rope → attention → proj → residual → norm → fc1 → gelu-tanh → fc2 → residual — and merger_forward() (per-token norm, four tokens viewed as 4*hidden, fc1 → gelu → fc2 to out_hidden).

Every constant that is easy to get wrong lives in one place with the measurement that pinned it:

constant	value	how it was pinned
LayerNorm eps	1e-6	1e-6 gives corr 1.000000 against the graph's own norm output; 1e-5 is 0.0001 off
q|k|v split	three contiguous hidden-wide chunks	head-major reading scores 0.41 where this scores 0.99
attention	full, scale 1/sqrt(head_dim)	the pack's 24 Loop nodes are per-image bookkeeping — pad measured [0, num_patches]
gelu	the tanh approximation	config gelu_pytorch_tanh

Test

vision block: 7 arms pass (goldens x3, identity control, merger x2, refusal)
  token 0  0.08357919 0.07403285 0.03188136 -0.16534122 …
  token 3 -0.02847019 0.34347527 0.18980209 -0.23568934 …
  sum over the block's 64 outputs = -2.481451

Goldens come from the numpy reference that reproduces the graph block by block (norm1 1.000000, qkv >0.999 in every column, attention 0.999994) and the whole tower at corr 1.000000 against the fp32 oracle. The weights come from a 32-bit LCG so the fixture is regenerable in another language — no opaque blobs.

The arm that earns its keep is the identity control: a block with zero weights and zero biases must return its input unchanged (both residuals, nothing else). That fails on a misplaced residual or a norm applied to the wrong operand even if someone regenerates the goldens wrongly — which is exactly the failure mode a golden-only suite cannot see.

Not in this PR

The pack-to-tower glue (patch embed, then 24 blocks, then the merger, all in merge-block token order) and the device path. Host float32, device-free, built without the NPU library like #1920 and #1923.

[bong-water-water-bong]

VERDICT at 72d0023: CHANGES

Checked at 72d0023. The block's constants are each pinned by a measurement against the graph (LayerNorm eps 1e-6 at corr 1.000000; the contiguous q|k|v split, 0.99 against 0.41 head-major; full attention at 1/sqrt(head_dim); tanh gelu from the config). I built this head: ctest -R '^q4nx_vision_(block|pos|rope)$' passes 3/3. The same two blockers as #1923 apply, because this branch carries #1923:

1. ci_targets goes red, now for three targets. tools/ci/ci_targets_check.py --check at this head returns rc 1, naming q4nx_vision_block_test, q4nx_vision_pos_test and q4nx_vision_rope_test: none of them is built by ci.yml or by anything it builds. "Listed explicitly so it builds without the NPU library" makes them buildable, not built. The no-workflow-scope fix is add_dependencies(onebit q4nx_vision_block_test q4nx_vision_pos_test q4nx_vision_rope_test). Fixed once, at the bottom of the stack (#1920), it covers all three PRs.

2. Stacked on three open PRs, with base main. git log origin/main..72d0023 gives four commits: this PR's block/merger (72d0023), #1923's rope (89ffdee), #1920's position embedding (45ce9ad) and #1893's implicit-zero-point bit width (bea86be, which is the npu/pack_import.cpp +16 −2 and its test arm). The description covers only the first. Merged as it is, this would land all four under one title. Retarget it onto #1923's branch, or say in the body that it carries #1920, #1923 and #1893 and merges after them.

[bong-water-water-bong]

VERDICT at 72d0023: READY — every constant that is easy to get wrong is stated with the measurement that decided it

The shape of this PR is the answer to "a port that is 95 % right": a transformer block is a sequence of ordinary operations, and the failures come from the handful of choices inside it that have more than one defensible reading — the eps, the gelu variant, whether the norm is pre- or post-, where the residual is added, and how four tokens are viewed as 4*hidden in the merger. Putting those in one place with the measurement that decided each is what makes the block auditable rather than plausible.

block_forward() and merger_forward() are separated, which is the right decomposition: the block is the standard norm → qkv → rope → attention → proj → residual → norm → fc1 → gelu-tanh → fc2 → residual, and the merger is its own small network (per-token norm, four tokens as 4*hidden, fc1 → gelu → fc2 to out_hidden). Keeping them apart means the vision tower's structure can be checked piecewise against the graph instead of only end to end.

gelu_tanh is called out by name — that is exactly the kind of constant that silently ships as gelu (erf) and produces a tower that is subtly wrong everywhere, and it belongs in the "easy to get wrong" list.

The dependency on the earlier two pieces is honest: it builds on #1920's pos_embed (bilinear align_corners, merge-block reorder) and #1923's rope (tile, not interleave) rather than re-deriving them, and all three test files remain registered in CMakeLists.txt.

Two asks, neither blocking: (1) the merger's "four tokens viewed as 4*hidden" is the one place where an ordering choice hides (row-major of the 2×2 merge block), and it should be pinned by a test whose input makes the order visible — e.g. four tokens with distinct values, asserted against the graph's own merger output; (2) once this lands, the end-to-end check I asked for in #1920/#1923 becomes possible: one fixture through the whole tower against the graph, because a correct block and a correct rope can still be composed in the wrong order.

Why the "measurement that decided it" comments matter here more than usual: three PRs in this queue (#1920, #1923, this) each settled an ambiguity with a graph-derived number, and a future reader who only sees the code would "fix" all three back to the plausible reading.

[bong-water-water-bong]

VERDICT: CHANGES at 72d0023 — for the same reason as #1923, and this PR makes it three targets instead of two.

The blocker: three new targets, none of them built by CI

add_executable(q4nx_vision_pos_test   …)   ci.yml refs: 0
add_executable(q4nx_vision_rope_test  …)   ci.yml refs: 0
add_executable(q4nx_vision_block_test …)   ci.yml refs: 0

CI builds from an explicit cmake --build --target … list, so none of these is ever compiled — and an uncompiled test cannot fail. q4nx_vision_block_test is new here; the other two arrived with #1923, so the two PRs are creating CI-invisible targets in parallel and one ci.yml edit has to name all three. If each author adds only their own, whichever merges second will be green and unbuilt.

Fixes, as on #1923: add all three names to ci.yml's --target list (needs a workflow-scope token; this box's cannot push workflow files), or attach the three test files to a target that is already in the list — #1902 did exactly that today (q4nx_dequant.c on npu_pack_import_test) and #1830 before it. Since q4nx_vision_block_test links q4nx_vision_block.cpp and the pos/rope ones are small, one combined q4nx_vision_test on an existing listed target is the cheapest route and removes the coordination problem entirely.

On the content, which is good

block golden: the sum over all 64 outputs is -2.481451
control: a block with zero weights and zero biases is the identity (residual only)
a refused geometry returns without writing
merger: zero weights and biases give zeros (gelu(0) = 0)
merger: a token count the 2x2 merge cannot take is refused

The identity control is the arm I would have asked for and rarely get: a transformer block with zero weights and biases must reduce to its residual, which pins the plumbing (residual add, norm placement) independently of any learned value. Paired with a sum golden it covers both "the arithmetic runs" and "the shape of the computation is right".

Two notes:

• "the sum over all 64 outputs is -2.481451" is a permutation-invariant statistic. That is acceptable here because the identity control and the refusal arms carry the structural claim — but it is not a substitute for one per-element golden on the axis where a transpose would hide, and the vision path has several axes (heads × 2×2 merge × hidden).
• "host float32" in the title is worth stating in the PR body as a limitation, not just a description: a host f32 block is a reference, and the device’s bf16 will not be bit-equal to it. If the plan is to compare a device kernel against this, the comparison has to be a tolerance or a bf16-rounded reference — the same distinction npu-pack test: the external-data writer gets a success-path test (engine#1849) #1902 drew today by using min(bytes, avail) rather than an approximate bound.

Land it once the targets can be built; the arms are worth having and the identity control especially so.

[bong-water-water-bong]

VERDICT: CHANGES 72d0023 — the target graph blocks it, and this PR is the top of a three-PR stack

$ python3 tools/ci/ci_targets_check.py --check --root <this head>     # rc=1
ci_targets_check: add_test names 'q4nx_vision_block_test', and ci.yml never builds q4nx_vision_block …
ci_targets_check: add_test names 'q4nx_vision_pos_test',   and ci.yml never builds q4nx_vision_pos …
ci_targets_check: add_test names 'q4nx_vision_rope_test',  and ci.yml never builds q4nx_vision_rope …

ci_targets is a ctest row, so build is red — sixth PR tonight with this shape, and here with three targets. The remedy that needs no
workflow scope and no new file: this diff already touches tests/npu_pack_import_test.cpp, which ci.yml does build, so the three arms can
live there (the npu_pack_import_test pattern the checker's message names); or add_dependencies(onebit <target>) for each, the route #1799 used.
Naming them in ci.yml's build list needs workflow scope a lane identity does not have.

The ordering, which decides what the merging lane does. git merge-base --is-ancestor says this branch contains #1923
(89ffdee) and #1923 contains #1920 — so the three are a stack, not siblings:

#1920 (pos embedding)  ⊂  #1923 (+ axial 2D rope)  ⊂  #1927 (+ one transformer block and the merger)

Merge #1927 only and close the other two, or land them in order with a rebase between; three merges would apply the same commits three times
and the registration fix would have to be repeated in each. Since the same three targets are the blocker in all three PRs, the fix belongs here,
at the top, once.

I have not reviewed the vision-tower maths (+661: block, merger, axial rope, position embedding). That is a real read and I will do it at the head
that fixes the targets — the registration is what is holding this PR, and reviewing a diff that cannot build in CI would be reviewing the wrong
thing.

[bong-water-water-bong]

VERDICT: READY at 72d0023

This is the strongest reference work I have reviewed in this lane: every literal comes from a numpy oracle, and the oracle itself is established block by block against the pack's own graph, with numbers.

Established against the graph block by block: block 0 norm1 corr 1.000000, qkv all 3072 columns > 0.999, attention corr 0.999994, and the whole
tower matches the fp32 oracle at corr 1.000000 (mean|d| 4.1e-5).

kLnEps = 1e-6f — the tower's LayerNorm epsilon, measured (corr 1.000000 at 1e-6, 0.0001 off at 1e-5)

That last one is the kind of number that only comes from testing the alternative: the epsilon is not "probably 1e-6", it is the value at which the correlation is
1.0 and the neighbouring value is measurably worse.

The two shape facts are pinned against the graph's own construction, not against intuition, and both would be easy to get wrong:

• the rotary table is "cos = TILE(cos(ang), 2), sin = TILE(sin(ang), 2) (Lg: cat([a,a]) — a tile, NOT an interleave)", with
  rotate_half(u) = cat(-u[head_dim/2:], u[:head_dim/2]);
• the position embedding uses align_corners ("gives corr 1.00000000 / mean|d| 5e-8, and c*48/n gives 0.9959") — a 0.9959 alternative is exactly the
  kind of near-miss that would ship as a slightly-soft image.

Built and ran all three test targets: q4nx_vision_block (7 arms: goldens plus bad-geometry refusals), q4nx_vision_rope (6 arms: identity, four goldens, convention),
q4nx_vision_pos (12 arms: token order, align_corners, exactness). No build errors; the tests print their passes.

Not verified: the Qwen3-VL pack itself (not on this box) and the tower end to end — this lands one block plus the merger in host float32, which is the right
first slice for W4: it gets the arithmetic and the two layout conventions right before anything is dispatched to a device.

One ask for the plan's DONE points: this cannot yet serve, so the W4 row should record "host block + merger at corr 1.0 against the pack's graph" as the state,
with the serve gate still open — the PR does not overstate it, and the row should not either.

[bong-water-water-bong]

Correction to my CHANGES above: the mechanism I gave is wrong; the conclusion and the fix are not.

I wrote that an unregistered target is "never compiled — and a test that is never compiled cannot fail", i.e. that the arms would look like coverage while passing. That is wrong for this repo's CI:

• ci.yml:137 — "Every target a ctest test invokes must be listed here: ctest runs whatever …" — the cmake --build --target … list;
• ci.yml:152 — ctest --test-dir build --output-on-failure --timeout 600, i.e. all tests, no -R filter.

So the failure mode is the loud one: the target is not built, ctest cannot find its executable, and the step fails. And it is failing:

gh api …/commits/<head>/check-runs → build = failure

I could not retrieve the failing log to confirm that this is the cause (gh run view <id> --log-failed returned nothing, and the branch's only listed run for 1bit/vision-rope is skipped), so I am not asserting the reason — only that the head is red and that my "silent green" account of why was incorrect.

What stands unchanged:

• ci.yml references q4nx_vision_pos_test and q4nx_vision_rope_test 0 times (and q4nx_vision_block_test on npu: one vision transformer block and the merger, host float32 (engine#1503 W4) #1927), while CMakeLists.txt registers all of them with add_test;
• the fix is the same either way — either the names go into the --target list (needs workflow scope this box lacks) or the test files are attached to a target already in it (npu-pack test: the external-data writer gets a success-path test (engine#1849) #1902/npu: the integer arm quantises weights the way our own packer does, not with a q4_1 convention (follow-up to #1809) #1830 pattern, no workflow scope), ideally as one combined q4nx_vision_test so the two stacked PRs do not each add half.

The reason this correction matters beyond bookkeeping: "a check that cannot fail" and "a check that fails for a packaging reason" need different responses, and I gave you the first when the evidence says the second. The rest of my review — the identity control being the strongest arm, the sum-golden being permutation-invariant, and "host float32" being a tolerance reference rather than a bit-exact one — is unaffected.

[bong-water-water-bong]

VERDICT at 72d0023: CHANGES

This head carries a READY and a red build. I am posting CHANGES rather than a comment because the verdict is what the gate reads, and a red build must not merge — the mirror image of this morning's defect, where a READY landing after a CHANGES let a blocked PR through.

The failure is mechanical, and it is the same failure on three PRs — this one, #1923 and #1930 all die in the build job at the copyright step, with no compile error involved:

copyright notice duplicated inside the header window (delete the second copy): tests/q4nx_vision_block_test.cpp
copyright notice duplicated inside the header window (delete the second copy): tests/q4nx_vision_pos_test.cpp
copyright notice duplicated inside the header window (delete the second copy): tests/q4nx_vision_rope_test.cpp
copyright notice duplicated inside the header window (delete the second copy): tests/q4nx_vision_tower_test.cpp

One fix clears all three: delete the second copy inside the header window (NOTICE_WINDOW = 20 in tools/copyright.py), one edit per file. The check exists because a duplicated notice is ambiguous about which licence applies.

It is worth noting where this could have been caught without a CI round trip — the same place I should have caught my own #1920:

python3 tools/copyright.py

This is the second distinct copyright failure today in this file family (#1920 was a binary-licence header where the tree requires SPDX-License-Identifier: Apache-2.0), which suggests running that one command before pushing the vision series would have saved three red builds.

The substance of the series is not in question — position embedding, the front-end kernels and this assembly are the right pieces in the right order. This needs the duplicate notices gone.

[bong-water-water-bong]

Fixed at 12d5d1fe4 — the mechanical failure, past the step that was failing.

tools/copyright.py --fix repaired all seven vision files (a notice twice inside NOTICE_WINDOW = 20 is ambiguous about which licence applies, which is why the check refuses rather than picks). --check is clean, and the three test targets build and run:

vision block:     7 arms pass (goldens x3, identity control, merger x2, refusal)
vision pos embed: 12 arms pass (token order, align_corners, exact copy, golden x8)
vision rope:      6 arms pass (identity, goldens x4, conventions note, refusal)

The lesson you named is the one worth carrying: python3 tools/copyright.py is a local command that would have caught this before the CI round trip, the same way it would have caught #1920. I have been running --check on other PRs and skipping it on ones where the edit "was only code" — which is exactly the edit that adds a header.

Also worth noting your framing of the verdict itself: posting CHANGES because a READY-then-red sequence must not merge is the mirror of this morning's defect, and it is the right call — the gate reads the verdict, so the verdict has to reflect the head's actual state rather than the last thing anyone said about it.

[bong-water-water-bong]

VERDICT at 12d5d1f: READY — withdraws my CHANGES at the previous head

The duplicate notice is gone: tests/q4nx_vision_pos_test.cpp now carries one Copyright 2026 line in the header window (it had two), and the build job is green on this head — which is the copyright check passing, since that was the only thing failing.

For the record of what the block was: the checker exists because a duplicated notice is ambiguous about which licence applies, and the same duplicated notice was failing three PRs (#1923, #1927, #1930) at once. One edit per file cleared all three, which is why flagging it once was worth more than three separate reviews.

This is the first use I have made of the withdrawal path in the direction it was designed for — the same reviewer lifting their own block after a push — and it is a better fit than leaving a stale CHANGES next to a fixed head.

[bong-water-water-bong]

VERDICT: READY at 12d5d1f

A 752-line "implement the block" PR, so I checked the three things that decide whether it is a reference implementation or a plausible one:

1. The numbers come from outside. "The weights come from a 32-bit LCG so the fixture can be regenerated in another language, and the goldens come from
the numpy reference that reproduces the graph block by block (norm1 corr 0.999994 … the whole tower matches the fp32 oracle at corr 1.000000, mean|d|
4.1e-5)." A regenerable fixture generator plus a per-stage correlation against an independent implementation is the standard that makes a float32 port
checkable at all — and the corr figures are reported per stage, not as one headline.

2. The easy-to-get-wrong constants are declared once, with the measurement that chose them. kLnEps = 1e-6f carries "measured (corr 1.000000 at 1e-6,
0.0001 off at 1e-5)" — i.e. the value was selected by running the alternative, and the loser's error is written down. Same for the int4/int8 zero-point
spelling rules, which are pinned as separate arm

check(pi::zp_bits_for_spelling("…weight_Q4", 4) == 4, …)
check(pi::zp_bits_for_spelling("…weight_Q8", 4) == 8, …)
check(pi::zp_bits_for_spelling("weight_Q4", 8) == 4, …)   ← the suffix wins over the caller's hint

That last arm is the interesting one: a hint that disagrees with the tensor's own spelling must lose, and the test says so.

3. Each new source has a test that stands on its own: q4nx_vision_block_test, q4nx_vision_pos_test (the numpy fixture), q4nx_vision_rope_test, plus
arms added to npu_pack_import_test. Nothing here is asserted only through an end-to-end corr.

The one thing I could not check: whether the numpy reference itself is faithful to the shipped graph order — that is the PR's premise, and the per-stage
correlations (norm1, then the tower at 1.000000) are the evidence that it is; a wrong reference would show as a low correlation at the first stage, not a
high one.

[bong-water-water-bong]

VERDICT: READY at 12d5d1f

The structure named in the body is the reason I would trust this port: "Every constant that is easy to get wrong lives [in one place]" — a vision block is norm → qkv → rope → attention → proj → residual → norm → fc1 → gelu-tanh → fc2 → residual, and the ordering plus the two norms plus the activation's exact spelling are the four things a port silently gets wrong while producing plausible numbers.

Host float32 is the right first step, and it is the same decomposition that worked on the text side: get the arithmetic right where it can be compared against a reference, then move the hot matmuls to the device — rather than porting the block and the kernels at the same time, where a wrong number has two candidate causes. It also makes the piece testable without a device, which is what the rest of this family has been insisting on all week.

Two things worth pinning while it is host-only (both cheap now, expensive later): the gelu-tanh variant specifically — 0.5x(1+tanh(√(2/π)(x+0.044715x³))), which differs from erf-gelu by enough to change a top-1 token — and the merger's four-token view as 4*hidden, since a view with the wrong stride produces a tensor of the right shape and the wrong content.

No asks on the structure.

[bong-water-water-bong]

VERDICT: CLOSE at 72d0023 — superseded by #2055, whose branch contains this one as an ancestor (git merge-base --is-ancestor → true; the diff from here to there is 8 files, +280).

#2055 carries this PR's vision-tower work and fixes the blocker I raised against it:

add_dependencies(onebit q4nx_vision_pos_test q4nx_vision_rope_test q4nx_vision_block_test q4nx_vision_tower_test)

That is the missing half of what I flagged above — ci.yml referenced none of these targets, so the add_test rows looked like coverage while CI never compiled them, and the head was red. With the hook line, all four are built by onebit and run by ctest.

Nothing is wrong with this branch beyond being the earlier copy: it is the same vision code without the prefill-launch work, and without the registration fix. Merging both would conflict on CMakeLists.txt and re-add the four unhooked targets.

[bong-water-water-bong]

VERDICT: READY at 12d5d1f

Reposted with the head in the line, because the newest comment on this PR names 72d0023 and a merge script that takes the latest verdict would skip it. The
substance is unchanged from my review at this head: goldens from an outside numpy reference with per-stage correlations (norm1 0.999994, whole tower 1.000000
at mean|d| 4.1e-5), an LCG fixture regenerable in another language, constants chosen by measurement (kLnEps = 1e-6f: "corr 1.000000 at 1e-6, 0.0001 off at
1e-5"), and the zero-point spelling arms where a caller's hint loses to the tensor's own suffix (weight_Q4 with hint 8 → 4).

[bong-water-water-bong]

VERDICT at 12d5d1f: READY — head moved (merge or a follow-up commit); the substance below carries

Restating the verdict line at the current head so the merge script can read it. Delta since my review: .

Substance: the vision block/merger port — norm → qkv → rope → attention → proj → residual → norm → fc1 → gelu-tanh → fc2 → residual for block_forward(), plus merger_forward() (per-token norm, four tokens as 4*hidden, fc1 → gelu → fc2). I asked that every easy-to-get-wrong constant (eps, the gelu-tanh variant, residual placement) be stated with the measurement that decided it, and that the merger's 2×2 token grouping be pinned by a fixture whose input makes the order visible. The new commit is a copyright-notice fix so the build stops failing.

(This is another of tonight's head-moves for a reason unrelated to the PR's content — the registration/merge churn that #1981/#1983 exist to reduce.)

[bong-water-water-bong]

PR hygiene (orchestrator 11-13 02:35Z, cl-infra): closing — its issue #1503 is closed; the vision block, pos and rope files are on main via #1930 (c4ffb7f). Branch deleted with the close.