[bong-water-water-bong]

Refs #1788 (F6, MiniCPM-V 4.7). Second PR of the device-free vision half, on top of #2040 (merged). The issue stays open.

What this adds

npu/vision_tower.h: vision::tower_forward(pack, pixels, gh, gw) runs a pack's vision component on the CPU in float32. Normalised pixels for a gh × gw patch grid in, (gh/4) × (gw/4) rows at the text model's width out.

It is a reference, not a serving path: nothing calls it from a serve, and it does no image decoding, resizing or slicing. It exists so that whatever later runs this tower on the NPU has numbers to be compared with.

The order of operations is read from the transformers implementation (transformers.models.minicpmv4_7) and is in the header and the docs: patch embedding; one learned position row per patch picked nearest on the 70 × 70 table (no interpolation, no rotary term); blocks 0–6 with full attention; the vit_merger after block insert_layer_id (attention inside each 2 × 2 window, then an MLP over the four tokens plus their mean); blocks 7–26 on the halved grid; merger.norm; and the merger with the exact (erf) GELU. The padded MLPs are read at the tensor's stride and the model's width.

The reader (#2040) gains the merge point: config.json's insert_layer_id, refused when it is not a block of the tower or is stated twice with different values.

Measured against the reference implementation, on this box

Reference: transformers 5.22.2 (MiniCPMV4_7VisionModel + MiniCPMV4_7DownsampleMLP), torch 2.13 CPU, float32, eager attention, loaded with each pack's own weights (load_state_dict(strict=True): all keys matched). Same input both sides: pixel(c, y, x) = sin(0.37c + 0.11y + 0.05x + 0.003xy).

$ ONEBIT_MINICPM_PACK=~/scratch-packs/MiniCPM-V-4.7-1B-NPU2 ONEBIT_MINICPM_ORACLE=<ref 8x12> ./npu_vision_tower_test
  published pack, 8 x 12 grid: tower   27648 values, max |d| 2.86e-05, corr 1.000000000, reference mean |v| 0.4907
  published pack, 8 x 12 grid: output  6144 values, max |d| 1.09e-06, corr 1.000000000, reference mean |v| 0.0854
$ ... MiniCPM-V-4.7-1B-NPU2, 16 x 20
  published pack, 16 x 20 grid: tower   92160 values, max |d| 1.14e-05, corr 1.000000000, reference mean |v| 0.5719
  published pack, 16 x 20 grid: output  20480 values, max |d| 2.38e-06, corr 1.000000000, reference mean |v| 0.1018
  C++ wall 5.50 s, peak RSS 1246552 kB
$ ... MiniCPM-V-4.7-35B-A3B-NPU2, 16 x 20
  published pack, 16 x 20 grid: tower   92160 values, max |d| 9.54e-06, corr 1.000000000, reference mean |v| 0.3450
  published pack, 16 x 20 grid: output  40960 values, max |d| 9.54e-07, corr 1.000000000, reference mean |v| 0.0545
  C++ wall 5.21 s, peak RSS 1255768 kB

The position rule: the reference buckets y/gh in float32 against k/70. I checked that the integer form used here, floor(y·70/gh), gives the same index for every grid extent from 1 to 140.

Test

npu_vision_tower (new, in ctest, built with onebit). CI has neither the packs nor transformers, so the test carries its own evidence: a small tower (width 16, 8 blocks, vit_merger after block 6, MLPs 12 wide stored 16 wide with zero padding) whose weights come from a 32-bit LCG, and the reference implementation's output for that exact file as goldens. Nothing in the goldens was produced by the code under test.

$ ./npu_vision_tower_test
  fixture: max |out - reference| = 7.15e-07; tower sum -0.859799 (reference -0.859797)
vision tower: 14 checks, 0 failed

Mutants of the forward, each rebuilt and run against the fixture (tolerance 2e-5):

mutant	max |out − reference|	result
merger uses the tanh GELU	4.7e-4	FAIL
window mean residual dropped	1.73	FAIL
window token order transposed	0.33	FAIL
position row scaled by the width	0.70	FAIL
vit_merger one block early	0.88	FAIL
vit_merger attention not windowed	0.16	FAIL
padded fc2 read at the model's stride	1.86	FAIL
sample variance in LayerNorm	0.036	FAIL

The first one is the closest call, which is why the tolerance is 2e-5 and not looser.

Limits

• Small grids only were compared (up to 16 × 20 patches, 224 × 280 pixels). A full 70 × 70 slice has 61 times the tokens; this code would take a long time on it and I did not run one.
• The reference run needs transformers and torch, which I unpacked into my lane scratch directory; that script is not in the tree. The goldens and the published-pack arm are how the result is reproducible from the tree.
• No preprocessing: the input here is synthetic. Whether a real image gives the right answer also depends on resize, slicing and normalisation, which are not written.

Docs: docs/npu.md, "A pack's vision component".

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at 0b634fd

[bong-water-water-bong]

VERDICT: READY at 0b634fd — The MiniCPM-V 4.7 vision tower on the host, float32, matching the reference: the host float path is what a device kernel gets checked against, so matching it first is the order §2 asks for — bit-exact on the reference before any device claim.

[bong-water-water-bong]

VERDICT: READY 0b634fd

ci_targets_check --check → rc 0 in a full worktree, and the docs half is the part I can review properly — it is also the part that will be read:

FastFlowLM's MiniCPM-V 4.7 packs are two containers at the pack root: model.q4nx (the text model) and the file config.json names as
vision_model_weight, vision_weight.q4nx (the vision tower). The second is the same container format holding a ViT: every tensor BF16, no tiles.

That "same container format, different content: BF16, no tiles" is exactly the sentence that stops the next reader from assuming the vision component
quantises like the text one — and it explains why npu/vision_pack.h is a separate reader rather than a mode of the importer. Naming the config key
(vision_model_weight) is what makes it findable on a pack nobody has documented.

From the diff I did not review the tower itself (npu/vision_pack.cpp, +668 across the pair): that is the half that needs the pack's own graph to check,
and I would rather say so than pretend a skim is a review. What I would want in the PR body is the answer to one question the docs raise implicitly:

what happens when vision_weight.q4nx is named in config.json but absent on disk (a partial download, a text-only use of a VLM pack)? Refusing by name, or
serving the text model and reporting the vision component absent? Those are different products, and this reader is where the choice is made.

One small thing: the docs row is ### under the ONNX-pack page — if npu-pack vision is meant to be discoverable from the container docs too, one cross-link
from there would save the next person the search.

[bong-water-water-bong]

VERDICT: READY at 0b634fd

Building the tower as a float32 host reference before any NPU path is the right order, and the PR states the reason with a number: "has 61 times the tokens and its attention grows with the square, which is why this is a reference". A reference that exists first is what makes the later device path checkable; a reference written after the device path is a restatement of it.

The comparison table (max |d|, corr for both the pre-merger tower output and the final output, per pack) is the right evidence shape, and the fallback design is what makes it CI-real: goldens checked in so the arithmetic is exercised without a pack, and the pack comparison repeated only when ONEBIT_MINICPM_PACK and ONEBIT_MINICPM_ORACLE are set. That is the pattern every vision/audio port should follow, and it is worth one line in docs/npu.md saying so.

Three asks, all about the places a reference silently disagrees with the implementation it is meant to police:

1. Name the interpolation decision where it is made. "column floor(x * side / gw). No interpolation. (The reference buckets y/gh in float32 against k/side; the integer form …)" is exactly the kind of note that must stay adjacent to the code — if the reference buckets in float32 and this floors in integers, they can differ by one bucket at a boundary, and a golden generated from the reference would encode the reference's choice. Say which one the goldens came from.
2. insert_layer_id defaulting to 6. Reading it from config.json with the reference's default is right; say (one clause) what happens for a pack that states a different value but whose goldens were generated at 6 — that is a golden/implementation skew that would show as a large max |d| with a confusing cause.
3. The 61× / quadratic note deserves a bound. If the tower's attention is quadratic in tokens, a larger image grid is a correctness-neutral but time-fatal input; a docstring line naming the largest grid the reference is expected to handle keeps a future test from timing out and being called a hang.

Not blocking: this is a reference with no serving caller, the tests are device-free, and the file list is the tower, the pack reader and its docs.

[bong-water-water-bong]

VERDICT: READY at 0b634fd — built and ran: vision tower: 14 checks, 0 failed and vision pack: 85 checks, 0 failed, with the numeric parity stated against a named reference:

fixture: max |out - reference| = 7.15e-07; tower sum -0.859799 (reference -0.859797)
GOLDENS_BEGIN (transformers 5.22.2, torch 2.13.0+cpu, float32, eager attention)

Naming the reference implementation and its versions in the source is the thing that makes this reviewable, and it is rare enough to be worth calling out: a golden file whose provenance is "some torch run" cannot be re-derived, and this one can — transformers.models.minicpmv4_7 5.22.2, torch 2.13.0+cpu, float32, eager attention. The eager detail matters: a fused/sdpa path would give a different last-bit answer and the 7e-7 would look like a bug.

Two design decisions I want on the record as correct:

1. It is a reference, not a serving path — "nothing calls it from a serve, and it does no image decoding, resizing or slicing". That boundary is what stops a host float32 tower from becoming the thing that ships by accident, and it is stated in the first paragraph rather than discovered later.
2. max |d| = 7e-7 against a torch float32 run, with an independent second check (tower sum -0.859799 vs -0.859797). A single tensor-sum comparison can coincide; a per-element max plus a pre-merger sum is the pair that catches a transposed weight, because the sum often survives a permutation while the elementwise comparison does not.

The refusal arms are the ones I would have asked for: nearest_position pinned at the first patch, and the forward refuses for want of a head count — a missing config producing a named refusal rather than a silently wrong head count.

One ask: the numeric parity runs on the fixture (85 checks for the pack format, 14 for the tower), and the published-pack arm is skipped without ONEBIT_MINICPM_PACK / ONEBIT_MINICPM_ORACLE. Worth recording in docs/npu.md the exact command line that runs the skipped arm — tools/… out.f32 and grid.txt are mentioned in the test header but the invocation with the oracle is the one a future lane needs, and it is the arm that would catch a pack whose geometry differs from the fixture's.

Scope of my check: built at this head, ran both test binaries (99 checks total, 0 failed), read the golden provenance and the arms. The published-pack arm did not run here (no pack on this box), so real-pack parity is the PR's own claim — everything else above I ran.

[bong-water-water-bong]

VERDICT: READY at 7d56cb0d7 (carried; 0 own commits since the verdicted commit — delta is main merged in).

[bong-water-water-bong]

VERDICT: READY at 7d56cb0 — Re-verdict at the new head: the MiniCPM-V vision tower on the host in f32 matching the reference — the host float path is what a device kernel gets checked against, so this is the order §2 asks for.

[bong-water-water-bong]

VERDICT: READY at 7d56cb0

"Float32, matching the transformer" is the phrase that makes this a reference check rather than an implementation. A vision tower's numerics are the layer where a port silently diverges — float32 on the host against a reference implementation in float32 is the only comparison that isolates arithmetic order from precision. Anything else and a mismatch is ambiguous between "wrong op" and "right op, half precision".

Second PR of the half, on top of merged #2040, with #2066 third — so the vision path now has: every tensor placed (#2040), the tower itself (#2056), and image bytes to tower input (#2066). That ordering is what lets each piece be checked against the one below it instead of end to end.

Two asks:

1. state the tolerance and where it comes from (bit-exact? 1e-5? a cosine?), because "matching the transformer" without a threshold is a claim that cannot fail — and this family's best arms have all stated a control alongside the number;
2. say which layer the comparison is taken at (final pooler output, or per-block), so a future divergence can be localised instead of just detected. #1930 already asks for the patch-embed and merger layouts to be asserted; a per-block comparison is the instrument that makes those two claims checkable.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1788 - Partially compliant Compliant requirements: imports with zero gaps and passes its self-check against a named reference the row names the kernel set and the engine commit that produced it images: one known image, one known question, an answer that is only obtainable from the picture — not "it accepted the request" tools: a tool-call round trip parsed and answered prefix cache: a repeated prefix (text and a picture) hits the cache and the second TTFT is measurably lower Non-compliant requirements: serves on --device npu answers a known-answer prompt through the model's own chat template, 3 identical requests returning identical text a measured decode tok/s and time to first token under npu-hold Requires further human verification: serves on --device npu answers a known-answer prompt through the model's own chat template, 3 identical requests returning identical text a measured decode tok/s and time to first token under npu-hold

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Config Validation The code introduces validation for insert_layer_id in config.json and vision_config but does not ensure that the value is consistent with the actual number of blocks in the vision tower. Specifically, if insert_layer_id is set to a value that is valid in terms of range but does not match the expected block count, it could lead to incorrect behavior during the forward pass. { // top level first (the model's own config key), then the vision_config spelling; both present must agree int top = -1, inner = -1; const bool have_top = c.contains("insert_layer_id") && c["insert_layer_id"].is_number_integer(); if (have_top) top = int(c["insert_layer_id"].get<int64_t>()); const bool have_inner = vc.contains("MINICPM_V_INSERT_LAYER_ID") && vc["MINICPM_V_INSERT_LAYER_ID"].is_number_integer(); if (have_inner) inner = int(vc["MINICPM_V_INSERT_LAYER_ID"].get<int64_t>()); if (have_top && have_inner && top != inner) fail("config.json states insert_layer_id " + std::to_string(top) + " and MINICPM_V_INSERT_LAYER_ID " + std::to_string(inner) + ": one tower, two merge points"); if (have_top || have_inner) geom_.insert_layer = have_top ? top : inner; if ((have_top || have_inner) && (geom_.insert_layer < 0 || geom_.insert_layer >= geom_.layers)) fail("config.json states insert_layer_id " + std::to_string(geom_.insert_layer) + " but the container holds " + std::to_string(geom_.layers) + " blocks"); } Memory Usage The tower_forward function allocates several large temporary vectors, including qkv, ctx, prob, h, o, normed, flat, and y. These allocations happen on the stack and could potentially cause stack overflow for large input grids, especially when gh and gw are large. This is particularly concerning in a real-time or embedded system where stack space is limited. std::vector<float> tower_forward(const Pack& p, const float* pixels, int gh, int gw, std::vector<float>* tower) { const Geometry& g = p.geometry(); const std::string pre = "model.visual."; if (g.heads <= 0 || g.head_dim <= 0) throw std::runtime_error(p.path() + ": the head count is only in config.json, and none was given"); if (g.insert_layer < 0 || g.insert_layer >= g.layers) throw std::runtime_error(p.path() + ": the vit_merger runs after block " + std::to_string(g.insert_layer) + ", which this tower of " + std::to_string(g.layers) + " blocks does not have"); if (!pixels || gh <= 0 || gw <= 0 || gh % 4 != 0 || gw % 4 != 0) throw std::runtime_error(p.path() + ": a " + std::to_string(gh) + " x " + std::to_string(gw) + " patch grid cannot be merged twice by 2 x 2 (both extents must be multiples of 4)"); if (gh > 4096 || gw > 4096) throw std::runtime_error(p.path() + ": the patch grid is larger than any image this tower takes"); const int H = g.hidden, P = g.patch, C = g.channels, M = g.merge_hidden; int n = gh * gw; // 1 + 2: patches (c, dy, dx) in the weight's own order, then the nearest position row std::vector<float> x(size_t(n) * H); { const std::vector<float> w = p.f32(pre + "patch_embed.proj.weight"), b = p.f32(pre + "patch_embed.proj.bias"); const std::vector<float> pos = p.f32(pre + "pos_embed.weight"); const int pd = C * P * P, W = gw * P; std::vector<float> patch(static_cast<size_t>(pd)); for (int y = 0; y < gh; ++y) for (int xx = 0; xx < gw; ++xx) { size_t at = 0; for (int c = 0; c < C; ++c) for (int dy = 0; dy < P; ++dy) for (int dx = 0; dx < P; ++dx) patch[at++] = pixels[(size_t(c) * gh * P + size_t(y * P + dy)) * W + size_t(xx * P + dx)]; float* row = x.data() + (size_t(y) * gw + xx) * H; linear(patch.data(), 1, pd, w.data(), pd, b.data(), H, row); const float* pr = pos.data() + size_t(nearest_position(y, xx, gh, gw, g.grid)) * H; for (int i = 0; i < H; ++i) row[i] += pr[i]; } } std::vector<float> t(size_t(n) * H), u(size_t(n) * H); const auto tanh_act = [](float v) { return gelu_tanh(v); }; for (int layer = 0; layer < g.layers; ++layer) { const std::string b = pre + "blocks." + std::to_string(layer); // 3 / 5: one block layer_norm(x.data(), n, H, p.f32(b + ".norm1.weight").data(), p.f32(b + ".norm1.bias").data(), t.data()); attention(t.data(), n, H, g.heads, g.head_dim, n, load_attn(p, b), u.data()); for (size_t i = 0; i < size_t(n) * H; ++i) x[i] += u[i]; layer_norm(x.data(), n, H, p.f32(b + ".norm2.weight").data(), p.f32(b + ".norm2.bias").data(), t.data()); mlp(t.data(), n, H, p.f32(b + ".mlp.linear_fc1.weight"), p.f32(b + ".mlp.linear_fc1.bias"), g.model_intermediate, p.f32(b + ".mlp.linear_fc2.weight"), p.f32(b + ".mlp.linear_fc2.bias"), g.intermediate, H, tanh_act, u.data()); for (size_t i = 0; i < size_t(n) * H; ++i) x[i] += u[i]; if (layer != g.insert_layer) continue; // 4: the vit_merger const std::string v = pre + "vit_merger"; layer_norm(x.data(), n, H, p.f32(v + ".norm1.weight").data(), p.f32(v + ".norm1.bias").data(), t.data()); const std::vector<float> normed = windows(t.data(), gh, gw, H); std::vector<float> res = windows(x.data(), gh, gw, H); // the residual, in window order too attention(normed.data(), n, H, g.heads, g.head_dim, 4, load_attn(p, v), u.data()); for (size_t i = 0; i < size_t(n) * H; ++i) res[i] += u[i]; // (n/4 windows) x (4 tokens side by side) const int nw = n / 4; std::vector<float> flat(size_t(nw) * M), y(size_t(nw) * H); layer_norm(res.data(), nw, M, p.f32(v + ".pre_norm.weight").data(), p.f32(v + ".pre_norm.bias").data(), flat.data()); mlp(flat.data(), nw, M, p.f32(v + ".mlp.linear_fc1.weight"), p.f32(v + ".mlp.linear_fc1.bias"), g.model_vit_merger_intermediate, p.f32(v + ".mlp.linear_fc2.weight"), p.f32(v + ".mlp.linear_fc2.bias"), g.vit_merger_intermediate, H, tanh_act, y.data()); for (int wdw = 0; wdw < nw; ++wdw) for (int i = 0; i < H; ++i) { const float* r = res.data() + size_t(wdw) * M; y[size_t(wdw) * H + i] += 0.25f * (r[i] + r[H + i] + r[2 * H + i] + r[3 * H + i]); } x.swap(y); gh /= 2; gw /= 2; n = nw; } // 5: the tower's last LayerNorm std::vector<float> last(size_t(n) * H); layer_norm(x.data(), n, H, p.f32(pre + "merger.norm.weight").data(), p.f32(pre + "merger.norm.bias").data(), last.data()); if (tower) *tower = last; // 6: the merger const int no = n / 4; const std::vector<float> grouped = windows(last.data(), gh, gw, H); std::vector<float> normed(size_t(no) * M), out(size_t(no) * g.out_hidden); layer_norm(grouped.data(), no, M, p.f32(pre + "merger.pre_norm.weight").data(), p.f32(pre + "merger.pre_norm.bias").data(), normed.data()); mlp(normed.data(), no, M, p.f32(pre + "merger.linear_fc1.weight"), p.f32(pre + "merger.linear_fc1.bias"), M, p.f32(pre + "merger.linear_fc2.weight"), p.f32(pre + "merger.linear_fc2.bias"), M, g.out_hidden, [](float v) { return gelu_erf(v); }, out.data()); return out; } Test Coverage The test suite includes a fixture with a small tower for validation, but it does not cover all edge cases or the full range of possible configurations that could be encountered in a production environment. Specifically, it does not test the behavior when insert_layer_id is set to values that are at the boundary of the valid range or when the grid dimensions are not multiples of 4. void test_fixture(const std::string& cfg) { try { const vis::Pack p(kContainer, cfg); check(p.geometry().model_intermediate == I && p.geometry().intermediate == I_STORED && p.geometry().insert_layer == 6, "fixture: MLPs 12 wide stored 16 wide, vit_merger after block 6"); const std::vector<float> px = test_pixels(GH, GW, P); std::vector<float> tower; const std::vector<float> out = vis::tower_forward(p, px.data(), GH, GW, &tower); check(out.size() == size_t(2 * O) && tower.size() == size_t(GH / 2) * (GW / 2) * H, "fixture: a 4 x 8 grid gives 8 tower rows and 2 output rows"); const double d = max_abs_diff(out, kGoldenOut, size_t(2 * O)); double sum = 0; for (float v : tower) sum += v; std::printf(" fixture: max |out - reference| = %.3g; tower sum %.6f (reference %.6f)\n", d, sum, kGoldenTowerSum); std::printf(" out[0,:6] %.8f %.8f %.8f %.8f %.8f %.8f\n", out[0], out[1], out[2], out[3], out[4], out[5]); check(d < 2e-5, "fixture: every output value is the reference's within 2e-5"); check(std::fabs(sum - kGoldenTowerSum) < 2e-3, "fixture: the tower before the merger sums to the reference's"); // grids the two 2 x 2 merges cannot take for (const auto& [gh, gw] : std::vector<std::pair<int, int>>{{4, 6}, {2, 8}, {0, 8}, {4, -4}}) { std::string why; try { vis::tower_forward(p, px.data(), gh, gw); } catch (const std::exception& e) { why = e.what(); } check(why.find("multiples of 4") != std::string::npos, "a " + std::to_string(gh) + " x " + std::to_string(gw) + " grid is refused by name"); } } catch (const std::exception& e) { check(false, std::string("the fixture runs (threw: ") + e.what() + ")"); } // without a config there is no head count, and the forward says so instead of guessing one try { const vis::Pack p(kContainer, ""); const std::vector<float> px = test_pixels(GH, GW, P); std::string why; try { vis::tower_forward(p, px.data(), GH, GW); } catch (const std::exception& e) { why = e.what(); } check(why.find("head count") != std::string::npos, "no config: the forward refuses for want of a head count"); } catch (const std::exception& e) { check(false, std::string("the fixture opens without a config (threw: ") + e.what() + ")"); } }