[bong-water-water-bong]

Refs #1788 (F6, MiniCPM-V 4.7). Small: three options on minicpm_image_probe (#2132). The issue stays open.

Why

A sliced image was described wrongly through the probe, the same on the host arm and on the device. Finding out why took three things the probe could not do, which I added locally and which led to #2149 (the forward was running this pack without q/k normalisation). They are worth keeping, because the next wrong answer on this path will need the same three cuts.

What

• --dump <dir> writes what the text model is about to be given, in hybrid_prompt_probe's file forms: ids.i32, positions.i32 (the t, h and w planes), pooled.f32 (the tower's rows). In the npu: q/k norm is read from the container, so a qwen3_5 text model under another name is normalised (engine#1788) #2149 hunt these matched a reference pipeline's exactly (ids and positions identical, rows within 1e-5), which put the fault past the inputs.
• --chunk N sends the prompt in calls of at most N rows, as the served route sends a prefill; 0 (default) is one call. There, 64, 128 and one call gave the same wrong answer, which ruled out the batched body and a row-count edge.
• --rows-argmax (text-only, with --none) prints the model's own next-token choice at every prompt row from one teacher-forced pass. Against the reference's per-row choices it showed 65 % agreement, flat across positions: an error in the layers, not at a position.

No change to what the probe does without these options.

Checked on this box (main at this head, host arm, MiniCPM-V-4.7-1B-NPU2)

$ NPU_INFER_HOST_GEMM=1 ./minicpm_image_probe <pack> ~/models/npu/d3-lite-elfs --none --prompt "The capital of France is a city called" \
      --decode 3 --rows-argmax --chunk 8 --dump <dir>
dumped 20 ids, 3 x 20 positions and 0 row values to <dir>
rows-argmax: 198 220 95772 3377 314 279 369 11751 3177 421 31586 198 15666 96280 248068 248068 198 248069 271 760
prompt 20 tokens (0 image rows), last position 19; ...
$ ls -l <dir>        ids.i32 80 B, positions.i32 240 B, pooled.f32 0 B
$ ./minicpm_image_probe <pack> <kernels> --solid 1,2,3 --prompt x --rows-argmax
minicpm_image_probe: --rows-argmax needs --none: the per-row pass takes token ids
$ ./minicpm_image_probe <pack> <kernels> --none --prompt x --chunk -3
--chunk wants a row count (0: one call)          (exit 2)

The pack loads on main now that #2026 has merged. The measurements that used these options are in #2149 and on the issue; I did not repeat them for this PR.

Not in this PR

The reference side (transformers with the model's original weights, and the script that compares per-row choices). It needs transformers and a 3.1 GB download, lives in my lane scratch, and is described in #2149.

No ctest row, as for the probe itself: it needs a pack, a kernel set and the forward library.

Docs: docs/npu.md, the probe's paragraph.

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at e63aa27

[bong-water-water-bong]

VERDICT at e63aa27: READY

Reviewed head e63aa27 (refs #1788). A diagnostic-only change to minicpm_image_probe (+67/−6) plus a docs paragraph. This is the first verdict.

Checked

• git merge-tree onto main: no conflicts. tools/copyright.py --check: rc 0. file_create_check: clean. CI build succeeded (waited for it before posting); CodeQL green.
• Without the new options the probe is unchanged. chunk = 0 takes the original single step_batch_embeds(embeds, 0, logits, in) call; --dump and --rows-argmax default off.
• --chunk N is the served route's shape. Each call takes rows [off, off+take) of the embeddings and of all three position planes (t|h|w, concatenated per chunk as the forward expects), and starts at cache slot off. --chunk < 0 is refused.
• --rows-argmax is refused without --none, as documented, because step_batch_all() takes ids, not image rows. It resets the forward before its teacher-forced pass, and the prefill that follows starts at slot 0.

Nits, not blocking

• The dumps bypass #2057's opener. --dump writes through std::ofstream, which file_create_check does not look for, so the files are created with the default mode and through a symlink. That is what #2057 just removed for every other dump. onebit_fopen_private_wb + fwrite would keep the dumps consistent with the rest of the tree.
• The chunk path builds its own HybridInputs. Each chunk gets a fresh HybridInputs ci with only position_ids set. That is equivalent today, because the probe never sets deepstack/image_lo/image_hi (minicpm_image_probe.cpp:266). But if a later pack routes deepstack through this probe, the chunked path will silently drop it while the single call keeps it. Copying in and replacing position_ids (and offsetting the image span) would make the two paths agree by construction.

[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 serves on --device npu 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" 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: 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 Requires further human verification: 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

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing Input Validation for --chunk The --chunk option does not validate that the chunk size is greater than zero, which could lead to unexpected behavior when chunk = 0 is passed. While the code treats chunk <= 0 as a special case, it's better to explicitly validate that chunk is a positive integer to prevent potential misuse or confusion. if (chunk < 0) { std::fprintf(stderr, "--chunk wants a row count (0: one call)\n"); return 2; } Potential Integer Overflow in --chunk The --chunk option uses std::atoi to convert the input string to an integer, which can lead to integer overflow if the input is larger than INT_MAX. This could result in undefined behavior or incorrect chunking logic. It's recommended to use a safer conversion method that checks for overflow. else if (a == "--chunk") chunk = std::atoi(value().c_str());