Skip to content

npu: minicpm_image_probe, one picture and one question through a MiniCPM-V 4.7 pack with the engine's own steps (engine#1788) - #2132

Merged
bong-water-water-bong merged 2 commits into
mainfrom
1bit/1788-image-probe
Nov 11, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
1bit/1788-image-probe

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #1788 (F6, MiniCPM-V 4.7). The issue stays open.

What this adds

minicpm_image_probe (under ONEBIT_NPU, next to hybrid_prompt_probe): one picture and one question through a MiniCPM-V 4.7 pack, with every step the engine's own.

RGB bytes -> plan_image -> preprocess -> tower_forward (host float32) -> image_tokens + prompt_positions
          -> the text model's step_batch_embeds() with the tower's rows where the placeholders are -> greedy decode

It chains what #2040, #2056, #2066 and #2089 added and what the forward already had (step_batch_embeds with three position planes, engine#1311). The image is a binary PPM or a generated colour, so no image decoder is involved. The prompt is the one the pack's chat template renders for a single user turn with one image and thinking off, written out in the probe rather than rendered. The text model opens the device unless NPU_INFER_HOST_GEMM is set.

It is a probe, not a route. forward-serve does not accept an image.

Dependency, stated plainly

On main today the published packs do not load in the forward: their lm_head is stored 2 (1B) and 16 (35B) rows past the vocabulary, and the reader refuses it. That is fixed in #2026 (READY, not merged yet). This PR is the probe only and builds on main; on main it stops here:

$ NPU_INFER_HOST_GEMM=1 ./minicpm_image_probe MiniCPM-V-4.7-1B-NPU2 ~/models/npu/d3-lite-elfs --none --prompt "..." 
minicpm_image_probe: text model: lm_head dequant failed

Every measured row below is from this commit with #2026's branch merged in (92c5a3e), which is how I ran it.

Measured on this box (MiniCPM-V-4.7-1B-NPU2, kernel set ~/models/npu/d3-lite-elfs)

Text model on the NPU, one npu-hold ticket (99 s):

image question answer
none What color is this image? Answer in one word. `black and white<
solid blue same `blue<
left half red, right half blue Which color is on the left side of this image, and which is on the right? Looking at the image, it's split vertically down the middle. The left side is red, and the right side is blue. So, (28 tokens)

Host arm (NPU_INFER_HOST_GEMM=1), solid red:

image 448 x 448: 1 piece(s), 64 rows; preprocess 0 ms, tower (host) 56440 ms
prompt ids vs hp-red/ids.i32: IDENTICAL (93 here, 93 there)
prompt 93 tokens (64 image rows), last position 34; init 1320 ms, prefill 23690 ms, 2 token(s) in 11751 ms
answer: red<|im_end|>

--expect-ids compares the probe's prompt token ids with a file; the file here is what the reference processor (transformers, the pack's own template and tokenizer) produced for the same prompt. They are identical, which checks the written-out prompt and the engine tokenizer on it.

Device timings per image, one run each: tower on the CPU 23 s (one thread), text prefill 6.0–6.4 s for 93–100 rows, about 0.9 s per token.

The left/right answer is the one that needs the patch order, the position table, both merges and the canvas positions to all be right.

Test

There is no ctest row: the probe needs a pack, a kernel set and the forward library, none of which CI has (same as hybrid_prompt_probe). The pieces it chains each have their own tests against the reference (npu_vision_pack, npu_vision_tower, npu_vision_plan, npu_vision_image, npu_vision_prompt). ci_targets_check --check: baseline unchanged.

Limits

  • 1B pack only. The 35B-A3B text model gives wrong text through the public forward (see the issue), so there is no point putting a picture through it yet.
  • Unsliced 448 × 448 images only were run. Sliced images go through the same code but I have not run one end to end.
  • Generated colours only; no photograph.

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

🤖 Generated with Claude Code

bong-water-water-bong and others added 2 commits November 11, 2026 02:21
…CPM-V 4.7 pack with every step the engine's own (engine#1788)

RGB bytes -> plan_image -> preprocess -> tower_forward (host) -> image_tokens + prompt_positions -> the text model's
step_batch_embeds() with the tower's rows where the placeholders are, then a greedy decode. No image decoder (PPM or a
generated colour), no tokenizer or template outside the engine.

Measured on MiniCPM-V-4.7-1B-NPU2 with the d3-lite kernel set, text model on the NPU (one npu-hold ticket, 99 s):
  no image                 -> 'black and white'
  solid blue               -> 'blue'
  left red, right blue     -> '... split vertically down the middle. The left side is red, and the right side is blue.'
Host arm, solid red -> 'red'; the prompt's 93 token ids are identical to the reference processor's.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
… the published packs

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

Copy link
Copy Markdown
Collaborator Author

ready for review at efa7e88

@github-actions

Copy link
Copy Markdown

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
  • answers a known-answer prompt through the model's own chat template, 3 identical requests returning identical text
  • images: one known image, one known question, an answer that is only obtainable from the picture — not "it accepted the request"

Non-compliant requirements:

  • serves on --device npu
  • a measured decode tok/s and time to first token under npu-hold
  • the row names the kernel set and the engine commit that produced it
  • 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
  • a measured decode tok/s and time to first token under npu-hold
  • the row names the kernel set and the engine commit that produced it
  • 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: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing Error Handling for Image Size Limits

The code checks for image dimensions exceeding 16384 in both height and width, but does not validate that the total number of rows produced by plan_image does not exceed the hardcoded limit of 4096. This could lead to an overflow or unexpected behavior if an image results in more than 4096 rows, even if individual dimensions are within limits.

if (plan.total_rows() > 4096) throw std::runtime_error("this image would be " + std::to_string(plan.total_rows()) + " rows; the probe stops at 4096");
Potential Integer Overflow in Memory Allocation

The code uses size_t(h) * w * 3 to calculate the size of the RGB buffer, and later size_t(y) * width + x for indexing. While this is generally safe, if h or w are very large, it could lead to integer overflow when calculating buffer sizes or indices. This is especially concerning in a context where memory is managed carefully for NPU operations.

for (int y = 0; y < height; ++y)
    for (int x = 0; x < width; ++x) {
        const uint8_t* c = have_solid ? solid : (x < width / 2 ? left : right);
        std::memcpy(&rgb[(size_t(y) * width + x) * 3], c, 3);
    }
Inconsistent Token ID Handling

The code uses tok.token_id() to retrieve token IDs, but does not check for potential negative return values from this function. If a token is not found, token_id() returns -1, which could lead to incorrect behavior or crashes if not properly handled.

m.image_start = tok.token_id("<image>");
m.image_end = tok.token_id("</image>");
m.slice_start = tok.token_id("<slice>");
m.slice_end = tok.token_id("</slice>");
m.image_pad = tok.token_id("<|image_pad|>");
const std::vector<int> nl = tok.encode("\n");
const int id_open = tok.token_id("<image_id>"), id_close = tok.token_id("</image_id>");
if (m.image_start < 0 || m.image_end < 0 || m.slice_start < 0 || m.slice_end < 0 || m.image_pad < 0 || id_open < 0 || id_close < 0 ||
    nl.size() != 1)
    throw std::runtime_error("this pack's tokenizer lacks one of <image>, </image>, <slice>, </slice>, <|image_pad|>, "
                             "<image_id>, </image_id>, or does not encode a newline as one token");
m.newline = nl[0];

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at efa7e88

This is a probe that can be wrong, which is the property I check first on a probe — it takes a reference image id stream (--expect-ids) and a left/right colour pair
(--halves R,G,B:R,G,B), so two of its inputs exist specifically so that a correct-looking run can be shown to be incorrect.

The --halves mode is what makes the vision path falsifiable. Feed the tower a left/right split and the image rows must change with the split; if the preprocessor
collapsed the image (a resize bug, a channel-order bug, a canvas bug) both halves produce the same rows and the probe says so rather than printing a plausible answer. Same for
--expect-ids: the prompt ids are compared against a reference and printed as IDENTICAL or not, with both counts — "prompt ids vs %s: %s (%zu here, %zu there)" — so a
mismatch of one row is visible rather than being inside a cosine.

It reports the intermediate steps with timings, which is what a lane needs when the end-to-end answer is wrong:

image %d x %d: %zu piece(s), %d rows; preprocess %.0f ms, tower (host) %.0f ms
prompt %d tokens (%zu image rows), last position %d; init %.0f ms, prefill %.0f ms, %zu tokens

That is a chain that can be bisected from the output alone — the same shape as the #1927 block-level numbers and the #1976 cache comparison, and the opposite of a probe
that prints "ok".

The gating is correct and is not the #2026 trap. add_executable(minicpm_image_probe …) sits under if(ONEBIT_NPU) (it links onebit_npu_rt), and that is right: it needs a
real pack and a device, so it is a tool, not a test pretending to run in CI. No add_test row is added, so nothing silently never runs — I checked for exactly that,
because a tool under a guard is fine and a test under a guard is a hole.

--none is a good third arm: the same prompt with no image must produce the text-only path, so "the vision path works" can be distinguished from "the model answers
regardless of the image", which is the failure that costs device tickets.

Not run: the probe itself — it needs a MiniCPM-V 4.7 pack and a device under npu-hold, and I have neither. My review is what the probe makes checkable: the two
falsifying modes, the intermediate numbers, the add_test absence, and the guard.

Note for the issue, not the PR: #1788 stays open per the header, and this is the tool that will produce its evidence — so the row gets its first comparable numbers when
someone runs --halves and --expect-ids on a ticket, not when this lands.

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

A probe that drives one picture and one question through the MiniCPM-V path end to end — which is the right granularity for a vision route's first instrument: it answers "does an image survive decoding, packing, the tower and the text side to produce an answer", and any answer at all is already more than the separate unit arms (#2066, #2089) can tell you.

Named minicpm_image_probe, registered as an executable with the npu include path, so it is a thing a lane runs by name when the demo path is in question — which is exactly the release-blocker condition that matters here.

@bong-water-water-bong
bong-water-water-bong merged commit 5ce405a into main Nov 11, 2026
10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/1788-image-probe branch November 11, 2026 07:04
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY efa7e88

What I could verify without a device, and the reader is the part I read line by line because it is where a bad input becomes a plausible wrong answer:

  • --size HxW is consistent, not an inversion trap. The parse is sscanf("%dx%d", &height, &width), i.e. height first — unusual, and exactly the kind of thing that silently swaps a picture. It is not a trap here: the usage string says [--size HxW] and the error says --size wants HxW, so the three agree. Please keep those three together if the flag is ever reworded.
  • read_ppm reads w then h, which is correct for P6 (and correctly different from the flag's order). It validates magic P6, maxval == 255, 0 < w,h <= 16384, and — the part that makes it trustworthy — return f.gcount() == size, so a truncated raster is a refusal rather than a half-image. Comments are skipped by a token loop that consumes the rest of a # line, and a non-numeric field lands at 0 and is rejected by the same bound check.
  • Failure surface is disciplined: every bad argument returns 2 with a message naming the offending value; no silent default is substituted for a malformed option.

What I did not verify, stated plainly: no end-to-end run — this needs an npu-hold ticket and a MiniCPM-V 4.7 pack, and the release's blocker rows want the NPU first (#2106, #2102), so I did not take a ticket to try it. So the chain the body describes (plan_image -> preprocess -> tower_forward -> step_batch_embeds) is reviewed, not measured, from my side.

Two non-blocking notes:

  1. Echo the inputs it actually used — the resolved WxH, the input form (ppm/solid/halves/none), and the image-token count, before the decode. A probe that reports its own inputs lets the next reader see that a --size was swapped or a PPM was the wrong picture; otherwise a wrong image and a wrong model look the same.
  2. 16384 x 16384 x 3 = 805 MB is the largest raster this will allocate; the cap is enforced before resize, so it is safe, but it is worth knowing the ceiling if someone ever points it at a whole slide.

The issue linkage is right: Refs #1788 with the issue left open, and the docstring says which of #2040/#2056/#2066/#2089 and #1311 it chains, plus the #2026 padded-head rule the published packs need — useful for anyone debugging a pack that "loads but looks wrong".

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