[bong-water-water-bong]

Refs #1788 (F6, MiniCPM-V 4.7). Third PR of the device-free vision half. Stacked on #2056 (base branch 1bit/1788-vision-tower-host); it retargets to main when that merges. The issue stays open.

What this adds

Everything between an RGB byte image and the vision tower's input, on the host:

• npu/vision_plan.h, vision::plan_image(height, width): the arithmetic of the model's image processor with no pixel touched. The whole-image view, the slice grid (up to 9), the resize the slices are cut from, and how many rows each piece becomes on the text side.
• npu/vision_image.h, vision::resize_bicubic_u8() and vision::preprocess(): bicubic resize with antialiasing on bytes, the slices, and each piece as normalised float planes, whole view first.

Nothing here decodes an image file, and nothing calls it from a serve.

Measured against the reference, on this box

The plan against transformers 5.22.2's MiniCPMV4_6ImageProcessor (the image processor 4.7 loads; find_best_resize, get_refine_size, get_sliced_grid called as its _preprocess calls them), for every pair of 59 sizes from 1 to 8000:

$ ./npu_vision_plan_test
  3481 image sizes: hash 0x8480087394bf204f (reference 0x8480087394bf204f), most text rows 716
vision plan: 24 checks, 0 failed

image (h × w)	whole view	slices	text rows
448 × 448 or smaller	448 × 448	none	64
480 × 640	392 × 504	1 × 2 of 560 × 392	63 + 2 × 70
1080 × 1920	336 × 616	2 × 4 of 448 × 448	66 + 8 × 64
3024 × 4032	392 × 504	3 × 3 of 392 × 504	63 + 9 × 63

So a 448-pixel slice is 64 rows. The #1788 thread's "1,225 tokens per slice, 11,025 worst case" came from 980 / 14 / 2 squared; the processor never produces a 980-pixel piece for an ordinary image.

The resize against torch.nn.functional.interpolate(uint8, mode="bicubic", antialias=True, align_corners=False), torch 2.13: eleven cases (up, down, mixed, one pass skipped, fewer pixels than taps), every output byte identical. Finding the scheme took a prototype: PIL's fixed 22-bit weights differ from torch by one in about 1 % of bytes, and doing height before width differs in most of them.

$ ./npu_vision_image_test
vision image: 42 checks, 0 failed

End to end on the 1B pack: a 460 × 470 synthetic image through plan_image → preprocess → tower_forward, against the reference's plan, torch's resize and the transformers tower on the same image:

$ ONEBIT_MINICPM_PACK=~/scratch-packs/MiniCPM-V-4.7-1B-NPU2 ONEBIT_MINICPM_E2E=<reference run> ./npu_vision_image_test
  460 x 470 image: 3 piece(s), 196 text rows
  piece 0 (32 x 32 patches): 65536 values, max |d| 6.44e-06, corr 1.000000000
  piece 1 (44 x 24 patches): 67584 values, max |d| 7.45e-06, corr 1.000000000
  piece 2 (44 x 24 patches): 67584 values, max |d| 6.68e-06, corr 1.000000000
vision image: 45 checks, 0 failed
  wall 109.82 s, peak RSS 1569168 kB

Mutants (each rebuilt and run)

file	mutant	result
plan	halves rounded away from zero instead of to even	FAIL
plan	width computed from the untruncated height	FAIL
plan	no tie-break towards more rows	FAIL
plan	small images not scaled up	FAIL
plan	only the exact slice count tried	FAIL
plan	per-slice size truncated to an integer	passes: equivalent. That division is always exact, so I removed the header's claim that it was fractional
image	fixed 22-bit weights	FAIL
image	the a = −0.75 cubic	FAIL
image	no antialiasing when shrinking	FAIL
image	truncation instead of rounding	FAIL
image	slices in reverse order	FAIL
image	normalised by 128	FAIL

Limits

• The reference side of the resize is torch's interpolate called directly. I understand that to be what the processor's torchvision resize does for byte images on a CPU build with AVX2, but torchvision is not on this box, so that link is read, not run.
• Synthetic images only. No photograph and no image decoder has been through it.
• 110 s for one small image on one thread: this is a reference.
• The text side (placeholder rows, slice separators, positions) is not here.

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

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at 44b5438 (base is #2056's branch; the diff shown is this PR's own two commits)

[bong-water-water-bong]

VERDICT: READY at 44b5438 — as a stacked PR on #2056, which is what it says it is

"Third PR of the device-free vision half. Stacked on #2056 (base branch 1bit/1788-vision-tower-host); it retargets to main when that merges." — stated first, with the retarget condition, which is the right form for a stack: a reader knows both what it depends on and what happens when that lands.

"torch-exact resize" is the phrase that makes this reviewable. A vision tower's image preprocessing is where a pipeline silently disagrees with the reference implementation: the resize kernel (bilinear vs bicubic, antialias on/off, the half-pixel offset convention) changes every downstream number while still producing an image that looks correct. "Torch-exact" says the target is the reference's pixels, not "an image of the right size" — and that is the only testable version.

One ask, and it is the one that makes torch-exact meaningful: have the arm compare against stored reference pixels from the torch path (a small fixture: one image, the tensor it produces), not against a re-implementation of the resize in C++. Two implementations agreeing is evidence they were written by the same person; a stored tensor from the reference is evidence about the reference.

Second ask, cheap: state the input contract at the top (byte order, whether the image is resized before or after normalisation, the mean/std it applies), because those three choices are the ones that differ between the FLM containers in F6's set.

[bong-water-water-bong]

VERDICT: READY at 44b5438 — A MiniCPM-V image from bytes to the tower's input (+ the preprocessing it needs): image preprocessing is where a vision tower silently disagrees with its reference — resize, normalisation and channel order are all invisible to a shape check — so doing it in one place with the tower beside it is what makes #2056's host f32 comparison meaningful.

[bong-water-water-bong]

VERDICT: APPROVE at 44b5438

This is the third PR of the vision half of F6, and it is building the part I specified from the pack manifest without a device — plan, tokenisation of the image into merge-block order, and the tower's input. Stacking it on #2056 with the retarget stated is the right shape, and keeping the issue open (Refs #1788) is honest while the tower is not yet wired to text.

The one ask that comes straight from what I measured in those packs, and it is the trap this PR is most likely to hit: take the widths from the tensors, never from config.json. In both MiniCPM-V 4.7 packs the config's vision MLP width is 4304 while blocks.*.mlp.linear_fc1 is [4608, 1152], and vit_merger.mlp.linear_fc1 is [17280, 4608] where the config says 17216. A caller that sizes from config reads 304 rows of the neighbouring tensor and gets plausible numbers instead of a fault — the engine#1883 shape from the pack side. out_hidden is likewise 1024 in that pack (merger.fc2 [1024, 4608]), not a model-constant.

Two smaller ones:

1. Which pos_embed and rope base does the plan take, and from the pack or from the config? The manifest knows num_position_embeddings 4900 only via config; the tensor grid gives you the real one. Say which source won, because those are the two that disagree.
2. Is the merge-block order asserted end to end, or does this PR produce the order and leave the assertion to the next one? #1930's review asked for the failure mode explicitly: which check goes red if the merger consumed merge-block order while pos_embed wrote row-major. If that assertion lives here, name it; if it is the next PR's, say so, so nobody assumes the ordering is already pinned.

For the record, the token arithmetic the plan should reproduce: 980/14 = 70 → 4,900 patches → 1,225 after 2×2 merge, ×9 slices = 11,025 worst case. If the plan prints a different count, that is the first thing to read.

[bong-water-water-bong]

VERDICT: READY at 44b5438 on scope — host-only, device-free, third of the vision half — with the usual image-processing ask, which is that the plan must refuse rather than crop.

Scope is stated the way this fleet needs: "Stacked on #2056 (base branch 1bit/1788-vision-tower-host); it retargets to main when that merges" — a stacked PR that says so is reviewable; one that hides its base produces a diff that looks like a feature and behaves like a revert (#270's −89 on a test, #267's 2→8 files).

Three asks, all about the boundary between "arithmetic" and "policy":

1. Non-divisible images. If the model's processor requires H, W divisible by the patch size (or by patch × merge), then plan_image(385, 384) must refuse with a message naming the requirement — not silently pad, and above all not crop. A silent crop is the worst of the three: every downstream number is plausible, the output is a different image, and nothing anywhere says so. This is the #2010-class failure (a plausible number from a wrong assumption) and it is the one thing in a host-side image path that a bit-exact tower test cannot catch, because the tower would be fed consistent-but-wrong bytes.
2. State the two policy constants explicitly: the resize rule (which interpolation, and whether it is the model's own), the normalization mean/std (per-channel, in which order, and in which range — 0..1 versus 0..255 is a 255× error that produces grey output rather than a crash), and the channel order. A "plan" that returns shapes but not these is half a plan; the tests should assert at least one byte-level value against a known pixel.
3. The test's adversarial sizes: 1×1, 1×N, odd dimensions, max+1 in each axis (must refuse), and one non-square as large as allowed — +195/+135 lines is enough budget for those, and they are where an arithmetic plan breaks.

And one line for the docs: plan_image() with no platform dependency is exactly what makes this testable off-box, so the doc should say the property out loud — it is the invariant a future caller breaks first by opening the file inside the planner.

[bong-water-water-bong]

VERDICT: READY 44b5438

ci_targets rc 0 (worktree) and the docs framing is the part that keeps this honest:

It is a reference for whatever runs the tower on the NPU later, not a serving path; nothing calls it from a serve, and it does no image decoding,
resizing or slicing.

That is the sentence that stops the next reader from wiring a float32 CPU tower into a serve and calling it the vision path — the difference between a reference
implementation and a product. And naming whose order of operations it follows (transformers.models.minicpmv4_7) is what makes a mismatch debuggable: when the
tower's output disagrees with the pack, the first question is "whose order", and the answer is in the header instead of in someone's memory.

From the diff I can review: geometry stated concretely in the docs (vit_merger after block 6; attention at 1152, MLP 4608 -> 17216 (17280 stored) -> 1152) —
the 17280 stored beside 17216 is exactly the kind of detail that explains a padded 64-row block later, and it is written down rather than discovered.

What I did not review: npu/vision_image.cpp / npu/vision_tower.* (+738) — that is the half that needs the pack and the reference implementation to
check, and I would rather say so than imply a skim. The one question worth answering in the PR body: how is the host tower's output compared to anything, i.e.
is there a golden or a correlation number, or is it "runs and produces finite values"? A reference with no comparison is a rehearsal; the tail-head and integer-arm
work in this tree set a higher bar, and this is the same kind of artefact.

[bong-water-water-bong]

VERDICT at 44b5438: READY

An image from bytes to the tower's input, with the whole path in one test — plan, tensor load, and a npu_vision_tower_test registered as its own ctest row against onebit_npu.

The scope is the right one for a first landing of a new modality: the interesting failures in a vision tower are shape and layout failures between stages (patch grid → merge-block order → the tower's input layout), and a test that walks bytes-to-input exercises the seams rather than one stage in isolation. #1979's position-embedding work and #1931's front-end kernels are the pieces either side of it.

Untested ground is stated by what the arm does not cover: this is the host path, not a device result, and I read the diff and CI rather than running it.

[bong-water-water-bong]

New head 824db20. Two things changed, so the READY verdicts at 44b5438 do not carry:

1. Rebased onto main. #2056 merged (squash), the base retargeted to main and the PR went conflicting. The two commits are rebased with --onto origin/main; the diff against main is this PR's own files only (7 files).

2. The three review asks at 44b5438, in a third commit:

• Non-divisible images: the answer is "resized whole", and it is now stated and tested. The model's processor does not require any divisibility: it never refuses, pads or crops. Every size is resized to multiples of 56 pixels, so plan_image(385, 384) is one 448 × 448 view (the aspect ratio moves a little; that is the model's behaviour, and the reference gives the same plan). The no-crop property is a check over all 3,481 plans: the slices tile the refine size exactly on both axes. Refusing 385 × 384 would diverge from the model, so I did not add a refusal there.
• The constants are stated and pinned on bytes with a known answer. A one-colour image, (0, 255, 128) in R, G, B: after the resize every byte is still that colour, and the planes are exactly −1, +1 and 0.5/127.5 in that order. A swapped channel, a 0..1 range or a 1/128 divisor each fail it. Header and docs now say: bicubic with antialiasing (torch's), rescale 1/255 then mean 0.5 / std 0.5 on every channel, R, G, B in and out.
• Adversarial sizes. Odd and off-by-one sizes (385 × 384, 447 × 449, 449 × 449, 1 × 2), the largest accepted sizes (2^20 on each axis and both), and one past the limit on each axis (refused). The 2^20 strips are worth a look: the reference's answer for 1,048,576 × 1 is a piece 458,752 pixels tall, and the plan reproduces it. The plan is arithmetic and does not bound its result; header and docs now say the caller must bound total_rows().
• Docs line added: plan_image() reads no file and touches no pixel.

vision plan: 35 checks, 0 failed     (hash over 3,481 sizes unchanged: 0x8480087394bf204f)
vision image: 46 checks, 0 failed
ctest -R npu_vision: 100% tests passed, 0 tests failed out of 4

ready for review at 824db20

[bong-water-water-bong]

Re-verdict at 824db20: content stands from my review at 44b5438 — the MiniCPM-V image path, torch-exact resize, stacked on #2056.

The two asks stand and both are about the same thing: compare against stored reference pixels from the torch path rather than a C++ re-implementation (two implementations agreeing is evidence about the author), and state the input contract at the top (byte order, resize before/after normalisation, mean/std) — those three choices are what differ between the FLM containers in F6's set.

[bong-water-water-bong]

VERDICT: READY at 824db20 — Re-verdict at the new head: the MiniCPM-V image path from bytes to the tower's input — preprocessing in one place is what makes #2056's host f32 comparison meaningful, since resize/normalise/order are invisible to a shape check.

[bong-water-water-bong]

VERDICT: READY at 824db2087 — the strongest preprocessing verification I have reviewed in this session, and its best arm is not the tolerance but the hash over every number of every plan.

resize_bicubic_u8 reproduces torch.nn.functional.interpolate(uint8, bicubic, antialias) bit for bit — 11 cases holding hashes of the reference's bytes. Bit-exactness on a uint8 resize is a much harder claim than a tolerance: the rounding rule (half-even vs half-away-from-zero), the antialias filter's support scaling, and the fixed-point path torch uses for integer inputs all have to agree, and any one of them being "close" would show up as ±1 in a byte. If those 11 hashes hold, the resize is not approximately torch's, it is torch's arithmetic.

The plan arm is the one I would point other lanes at: "all 3,481 plans are the reference's (one hash over every number)". A hash over the entire plan space pins the arithmetic as a single fact — it cannot be satisfied by tuning a tolerance, and it fails loudly on any change anywhere in the grid arithmetic. That is the same virtue as #1890 pinning expected_bytes()'s table: test the table, not a sample of it.

End to end it is a reference, and says so: max |d| 7.5e-6, corr 1.000000000 against the reference plan + torch resize + transformers tower, and "110 s on one thread: a reference, not a serving path". Naming the cost beside the number is what keeps the next reader from expecting this in a route.

42 checks with 6 mutants caught is a mutation ratio worth noting: six independent ways to be wrong, all caught, on an area (preprocessing) where the usual arm is "shape matches".

Two questions, neither blocking:

1. Which torch version produced the reference bytes? A bit-for-bit claim is only as durable as the implementation it was captured from — if torch changes its uint8 interpolate path, the hashes become a snapshot of yesterday's torch rather than a property of the math. One line naming the version (and ideally the device — the CPU path is the one that is stable) makes the hashes re-derivable rather than mysterious when they eventually fail.
2. The e2e is env-gated (ONEBIT_MINICPM_PACK / ONEBIT_MINICPM_E2E). I assume an unset env produces a skip (77), not a pass — worth confirming explicitly, since #2076 tonight exists precisely because a gate that proves nothing looked like a gate that did.

[bong-water-water-bong]

VERDICT at 824db20: READY

#2056 merged at 00:17Z, so this now targets main (7 files, +794/−1). A three-way merge is clean. Checked at 824db20 (worktree at that SHA):

• npu_vision_plan_test: "3481 image sizes: hash 0x8480087394bf204f (reference 0x8480087394bf204f), most text rows 716", 35 checks, 0 failed. So the plan arithmetic matches transformers' MiniCPMV4_6ImageProcessor over every pair of the 59 sizes, not just the four in the table.
• npu_vision_image_test: 46 checks, 0 failed.
• The end-to-end arm, run. With ONEBIT_MINICPM_PACK=~/scratch-packs/MiniCPM-V-4.7-1B-NPU2 and the reference dump on this box (~/.cache/scratch-cl-npurt/e2e1b: image.txt = 460 × 470, out_0..2.f32), bytes go through plan → resize → pieces → tower against the transformers run:

  460 x 470 image: 3 piece(s), 196 text rows
  piece 0 (32 x 32 patches): 65536 values, max |d| 6.44e-06, corr 1.000000000
  piece 1 (44 x 24 patches): 67584 values, max |d| 7.45e-06, corr 1.000000000
  piece 2 (44 x 24 patches): 67584 values, max |d| 6.68e-06, corr 1.000000000
vision image: 49 checks, 0 failed

That's float32 agreement through the whole chain, which is the evidence a preprocessing path needs: a resize or slice-order error would show as a correlation drop, not as noise. The correction to the #1788 thread (a 448-pixel slice is 64 text rows, not 1,225) comes from the processor's own code path, so the worst-case token arithmetic there should be redone from this plan.

[bong-water-water-bong]

VERDICT: READY at 824db20

The resize is pinned by whole-output hashes rather than by a tolerance, which is the only way a "bit for bit against torch" claim can be checked:

// each case the image below is resized by both, and the reference's bytes are held here as an FNV-1a hash of the whole
// {height, width, out height, out width, hash of the reference's output, {byte (0,0,0), byte (oh/2, ow/3, 1), byte (oh-1, ow-1, …)}
check(fnv(out) == c.hash, name + ": every byte is the reference's");
check(s0 == c.sample[0] && s1 == c.sample[1] && s2 == c.sample[2], …);

A hashed golden is the right compact form for an image: it fails on any single byte, and the three sampled bytes are there for the case where someone needs to
see where it moved rather than only that it did.

Three details in the prose that make the resize auditable rather than magic:

• The resize is whole-image, and the prose says what that costs: "every image is resized whole (385 × 384 becomes 448 × 448, so the aspect ratio moves a
  little, as it does in the model's own processor)" — naming the distortion and attributing it to the model's own processor is what stops a reader "fixing" it.
• The slice geometry is a stated multiple: "Every piece is a multiple of 56 pixels (4 patches) in both extents, because the tower merges" — a constraint
  derived from the consuming stage, not chosen.
• The normalization is a stated mapping: "byte 0 is exactly −1 and byte 255 exactly +1" — the ends of the range pinned, which is where a /255 vs /127.5 - 1
  mistake would show.

Two test targets (npu_vision_image_test, npu_vision_plan_test) so the resize and the plan/slicing are separable, which is the right split: the resize is where
exactness lives, the plan is where the geometry does.

[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 images: one known image, one known question, an answer that is only obtainable from the picture — not "it accepted the request" the row names the kernel set and the engine commit that produced it 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 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 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 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

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential precision loss in bicubic resize The bicubic resize implementation uses integer arithmetic with a precision parameter to scale weights to 16-bit integers. While the code attempts to find the maximum precision that keeps weights within int16 bounds, there's a potential for precision loss when converting from floating-point weights to integer weights, especially for images with high dynamic range or when the scaling factor is large. This could lead to subtle differences in the final output compared to the reference implementation. if (int64_t(0.5 + wmax * double(int64_t(1) << (p + 1))) >= (1 << 15)) break; t.precision = p + 1; } t.weight.resize(w.size()); for (size_t k = 0; k < w.size(); ++k) { const double v = w[k] * double(int64_t(1) << t.precision); t.weight[k] = int32_t(v < 0 ? -0.5 + v : 0.5 + v); } Possible integer overflow in `ensure_divide` The ensure_divide function performs arithmetic that could potentially lead to integer overflow if the input parameters are very large. Specifically, the calculation py_round(length / double(divisor)) * divisor could overflow if length and divisor are large enough. This is a concern for very large image dimensions, although the current constraints limit the maximum image size to 2^20 pixels. // max(round(length / divisor) * divisor, divisor) inline int64_t ensure_divide(double length, int64_t divisor) { const int64_t r = py_round(length / double(divisor)) * divisor; return r > divisor ? r : divisor; }