Repository navigation
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
Conversation
…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>
|
ready for review at efa7e88 |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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.
|
VERDICT: READY 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:
What I did not verify, stated plainly: no end-to-end run — this needs an Two non-blocking notes:
The issue linkage is right: |
Refs #1788 (F6, MiniCPM-V 4.7). The issue stays open.
What this adds
minicpm_image_probe(underONEBIT_NPU, next tohybrid_prompt_probe): one picture and one question through a MiniCPM-V 4.7 pack, with every step the engine's own.It chains what #2040, #2056, #2066 and #2089 added and what the forward already had (
step_batch_embedswith 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 unlessNPU_INFER_HOST_GEMMis set.It is a probe, not a route.
forward-servedoes not accept an image.Dependency, stated plainly
On main today the published packs do not load in the forward: their
lm_headis 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: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-holdticket (99 s):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:--expect-idscompares 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
Docs:
docs/npu.md, "A pack's vision component".🤖 Generated with Claude Code