[bong-water-water-bong]

Refs #1788 (F6, MiniCPM-V 4.7). First scoped PR of the device-free vision half; the issue stays open and my comment there lists what remains.

What was missing

FastFlowLM's MiniCPM-V 4.7 packs are two containers: model.q4nx and vision_weight.q4nx (the vision tower). Nothing in the engine opened the second one; the facts about it in #1788 were read by hand with a Python parser. On main:

$ git grep -il "vision_weight" -- npu app      (nothing)

What this adds

npu/vision_pack.h: a reader for the vision component, and 1bit npu-pack vision <pack dir> to run it.

• Every tensor must be BF16, inside the payload, exactly as long as its shape, and overlap no other.
• The tower's geometry is derived from the tensors' own shapes, and every tensor must have the shape the others imply. A tensor the reader has no place for is refused, as is a missing one (zero gaps, done-condition 1 of the issue).
• config.json is consulted only for what tensors cannot say (head count, image size, the unpadded MLP widths) and refused where the two cannot both be right (heads × head_dim ≠ width, image ≠ grid × patch, another width / block count / position count, text hidden_size ≠ what the merger projects to, an MLP stated wider than its tensors).
• The MLPs are stored padded, and the reader verifies it. The model's MLP widths are 4304 and 17216 (what config.json states); the container stores them 4608 and 17280 wide with the extra rows zero. Every size comes from the tensor; the reader checks that everything past the stated width is zero in every block and carries the model's width beside the stored one.

It is a reader only. No forward runs the tower yet.

Correction at a8c38f4 (second commit)

The first commit, and the #1788 thread before it, called config.json stale about those two widths. That was wrong, and I found it by doing the check the issue's done-condition 1 asks for: the self-check against a named reference.

The 1B container's __metadata__ names mmproj-model-f16.gguf with a SHA-256. That is the file of that name in openbmb/MiniCPM-V-4.7-1B-gguf (downloaded, sha256sum matches: fb9cc047…5a900). I rebuilt every pack tensor from the GGUF and compared BF16 bit patterns:

1B pack  vs its source:   pack tensors compared 347 of 347 {'exact': 263, 'padded_exact': 84, 'diff': 0}
   27 x [4304, 1152] -> [4608, 1152]  lead-identical, pad all +0.0      (blocks fc1 weight)
   27 x [1152, 4304] -> [1152, 4608]  lead-identical, pad all +0.0      (blocks fc2 weight)
   27 x [4304]       -> [4608]        lead-identical, pad all +0.0      (blocks fc1 bias)
    1 x [17216, 4608] -> [17280, 4608], 1 x [1152, 17216] -> [1152, 17280], 1 x [17216] -> [17280]   (vit_merger)
35B pack vs its source (openbmb/MiniCPM-V-4.7-35B-A3B-gguf, sha256 bff3b688…5d20f1):  263 / 84 / 0, the same
35B pack vs the 1B's source (control):                                                 0 / 0 / 347
source F32 tensors that are not bf16-exact: 0
source tensors with no counterpart in the pack: v.tok_embd_sep

So the source's ffn tensors are 4304 and 17216 wide, and the pack pads them to the next multiple of 128 with +0.0. attn.qkv is the source's attn_q, attn_k, attn_v stacked in that order; merger.norm is v.post_ln. That comparison was a script run by hand (it needs 2.2 GB of source files), not a test in the tree; the docs say so.

Measured on this box (both published packs, device-free)

$ 1bit npu-pack vision ~/scratch-packs/MiniCPM-V-4.7-1B-NPU2
vision component  .../MiniCPM-V-4.7-1B-NPU2/vision_weight.q4nx
tensors           347, all BF16, 1135109120 B of payload, 0 B unaccounted
patch embed       14 x 14 pixels x 3 channels -> 1152
positions         4900 (a 70 x 70 grid; image 980)
blocks            27 at width 1152, 16 heads x 72, MLP 4304 (4608 stored)
vit_merger        attention at 1152, MLP 4608 -> 17216 (17280 stored) -> 1152
merger            4608 -> 4608 -> 1024 (the text model's hidden size)
source_file       mmproj-model-f16.gguf
source_sha256     fb9cc047a3cdad5317fd8433a4aeda2d22f00f02ca4bd93b34ad84165b25a900
note              config.json states MINICPM_V_VISION_MLP_INTERMEDIATE_SIZE 4304; model.visual.blocks.*.mlp.linear_fc1.weight has 4608 rows, the last 304 all zero: the container pads the model's width, and sizes come from the tensor
note              config.json states MINICPM_V_VIT_MERGER_INTERMEDIATE_SIZE 17216; model.visual.vit_merger.mlp.linear_fc1.weight has 17280 rows, the last 64 all zero: the container pads the model's width, and sizes come from the tensor

The 35B-A3B pack prints the same tower with merger 4608 -> 4608 -> 2048 and no provenance lines: its container carries no __metadata__.

Test

npu_vision_pack (new, registered in ctest, built with onebit): crafted containers with the published packs' tensor names and shape relations, one edit per arm.

$ ./npu_vision_pack_test
  (ONEBIT_MINICPM_PACK not set: the published-pack arm did not run)
vision pack: 78 checks, 0 failed
$ ONEBIT_MINICPM_PACK=~/scratch-packs/MiniCPM-V-4.7-1B-NPU2 ./npu_vision_pack_test        -> 87 checks, 0 failed
$ ONEBIT_MINICPM_PACK=~/scratch-packs/MiniCPM-V-4.7-35B-A3B-NPU2 ./npu_vision_pack_test   -> 87 checks, 0 failed

Mutants of the reader, each rebuilt and run:

mutant	result
overlap check removed	3 FAIL
unknown tensors accepted	3 FAIL
block MLP width taken from config.json	1 FAIL
extent not checked against the shape	4 FAIL (survived until I added an arm that keeps the shape over fewer bytes)
padding check skips the linear_fc2 columns	2 FAIL
any narrower config treated as padding	2 FAIL

CI does not have the packs, so the published-pack arm runs only where ONEBIT_MINICPM_PACK is set; the crafted arms run everywhere.

Not in this PR

A host forward for this tower, image preprocessing and slicing, the <|image_pad|> join, and anything on the device. Our llama.cpp fork has a minicpmv4_6 projector but no minicpmv4_7 one (which is what both source GGUFs declare), so there is no forward oracle on the box yet.

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

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at d51f4ef

[bong-water-water-bong]

VERDICT: READY at d51f4ef

This is the vision half of F1, and the scoping is right: "First scoped PR of the device-free vision half; the issue stays open and my comment there lists what remains." And the premise is the one my own census confirmed from the other side — "FastFlowLM's MiniCPM-V 4.7 packs are two conta[iners]" — which is exactly the multi-component shape I hit on Gemma4-E2B-IT-NPU2 (model.q4nx 4.67 GB text + vision_weight.q4nx 337 MB + audio_weight.q4nx 952 MB).

"Every tensor placed" is the right property to establish before any forward runs, because a vision component's failure mode is not a crash: it is a tower that runs with a tensor in the wrong slot and produces plausible pooler output. Placing every tensor, and saying which ones have no home, converts that into an inventory question.

Two asks, both from the F1 census (engine#1786):

1. vision_weight.q4nx should be reported through the same genre arm as model.q4nx — the census found 13 self-describing containers, 14 gguf-converted and 2 with no metadata at all (d3-edge, d3-flash), so the vision container may be a different genre from its text sibling in the same pack; a reader that assumes they match will read one of them blind;
2. report the component's dims when it has them — for the self-describing genre the geometry is already in the header, and for Gemma4-E2B the vision tower's own hidden/block/merger numbers are the ones the forward needs.

No other asks.

[bong-water-water-bong]

New head a8c38f4: a correction, not a response to review. The first commit said config.json was stale about two MLP widths; a bit-for-bit comparison of both packs against their source GGUFs shows the config holds the model's widths and the container zero-pads them. The reader now verifies the padding and carries both widths; the body has the measurement. The READY at d51f4ef does not cover this commit.

ready for review at a8c38f4

[bong-water-water-bong]

VERDICT at a8c38f4: READY

Checked at a8c38f4. A three-way merge onto main is clean. ci_targets_check is at baseline 9 with none new, tests_referenced_check passes, and copyright rc is 0.

• npu_vision_pack_test: "78 checks, 0 failed". With ONEBIT_MINICPM_PACK=~/scratch-packs/MiniCPM-V-4.7-1B-NPU2 the published-pack arm runs too: "87 checks, 0 failed", with both padding notes.

• The reader on the four vision_weight.q4nx files on this box:

  pack	rc	result
  MiniCPM-V-4.7-1B-NPU2	0	347 tensors, all BF16, 0 B unaccounted; 27 blocks at 1152 (16 × 72), MLP 4304 (4608 stored), vit_merger 17216 (17280 stored), merger → 1024; source mmproj-model-f16.gguf with the sha256 the description quotes
  MiniCPM-V-4.7-35B-A3B-NPU2	0	the same tower, merger → 2048 (its text hidden size)
  gemma4-e2b-npu2	1	refused: "has no tensor model.visual.patch_embed.proj.weight"
  Qwen3.8-27B-NPU2	refused	"patch_embed.proj.weight is [1152, 3, 2, 16, 16], not [hidden, channels, patch, patch]"

  So the two towers it's for read with consistent geometry, and two different towers are refused by name, not misread.

• The padding check can fail. With is_zero made to return true (any tail accepted as padding), the suite fails "not padding: one nonzero value past the stated width keeps the tensor's width" and "… the note says so". Restored, it's 78/78.

Read: is_zero treats ±0.0 as zero. A stated width larger than the tensor is refused. A smaller one is taken as padding only when every element past it is zero in fc1 rows, fc1 bias and fc2 columns, across every block; otherwise the tensor's width stands, with a note. The correction at a8c38f4 (the widths are padded, config.json is right) is the strongest part of the PR, because it rests on a bit-exact rebuild from the source GGUF the container names, with the other model's source as a control.

[bong-water-water-bong]

VERDICT at 56110f5: READY

The new head is a merge of main (#2024, #2029, #2033 and others). The PR's own diff is identical to a8c38f4, which I passed, and a three-way merge onto current main is clean. At 56110f5, npu_vision_pack_test with ONEBIT_MINICPM_PACK=~/scratch-packs/MiniCPM-V-4.7-1B-NPU2 gives "87 checks, 0 failed".

[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 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 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 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 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

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Tensor Overlap Check The code checks for overlapping tensors in the payload by sorting tensor offsets and ensuring no overlap occurs. However, the check only ensures that no two tensors overlap, but does not validate that the tensor data is correctly aligned with its specified shape and data offsets. If a tensor's data is misaligned or does not match its declared shape, it could lead to incorrect interpretation of data or memory access violations. // ── no two tensors share a byte ── { std::vector<std::pair<uint64_t, const std::string*>> order; for (const auto& [name, t] : tensors_) order.push_back({t.offset, &name}); std::sort(order.begin(), order.end()); uint64_t end = 0, covered = 0; const std::string* prev = nullptr; for (const auto& [off, name] : order) { if (prev && off < end) fail(*name + " overlaps " + *prev + " in the payload"); end = off + tensors_.at(*name).bytes; covered += tensors_.at(*name).bytes; prev = name; } unaccounted_ = payload_ - covered; } Config Cross-Check The code cross-checks configuration values against tensor shapes, but it does not validate that the tensor data itself matches the expected values derived from the configuration. If the tensor data is inconsistent with the configuration, it could lead to incorrect model behavior or runtime errors. // ── config.json: what the tensors cannot say, and what ties the tower to something outside it ── geom_.model_intermediate = geom_.intermediate; geom_.model_vit_merger_intermediate = geom_.vit_merger_intermediate; if (config_json.empty()) return; const json c = json::parse(config_json, nullptr, false); if (c.is_discarded() || !c.is_object()) fail("the config beside it is not a JSON object"); const json vc = c.contains("vision_config") && c["vision_config"].is_object() ? c["vision_config"] : json::object(); const auto field = [&](const char* key, int* out) { const std::string k = std::string("MINICPM_V_") + key; if (!vc.contains(k) || !vc[k].is_number_integer()) return false; *out = as_int(vc[k].get<int64_t>()); return true; }; const auto refuse = [&](const char* key, int stated, const std::string& held) { fail("config.json states MINICPM_V_" + std::string(key) + " " + std::to_string(stated) + " but " + held); }; int v = 0; if (field("VISION_EMBED_DIM", &v) && v != geom_.hidden) refuse("VISION_EMBED_DIM", v, "the patch embedding is " + std::to_string(geom_.hidden) + " wide"); if (field("VISION_NUM_LAYERS", &v) && v != geom_.layers) refuse("VISION_NUM_LAYERS", v, "the container holds " + std::to_string(geom_.layers) + " blocks"); if (field("PATCH_SIZE", &v) && v != geom_.patch) refuse("PATCH_SIZE", v, "the patch embedding's kernel is " + std::to_string(geom_.patch)); if (field("VISION_NUM_POSITION_EMBEDDINGS", &v) && v != geom_.pos_rows) refuse("VISION_NUM_POSITION_EMBEDDINGS", v, "the position table has " + std::to_string(geom_.pos_rows) + " rows"); if (field("VISION_OUT_HIDDEN_SIZE", &v) && v != geom_.out_hidden) refuse("VISION_OUT_HIDDEN_SIZE", v, "the merger projects to " + std::to_string(geom_.out_hidden)); if (field("MERGER_HIDDEN_SIZE", &v) && v != geom_.merge_hidden) refuse("MERGER_HIDDEN_SIZE", v, "four tokens of this tower are " + std::to_string(geom_.merge_hidden) + " wide"); if (field("IMAGE_SIZE", &v)) { geom_.image_size = v; if (v != geom_.grid * geom_.patch) refuse("IMAGE_SIZE", v, "the position table is a " + std::to_string(geom_.grid) + " x " + std::to_string(geom_.grid) + " grid of " + std::to_string(geom_.patch) + "-pixel patches (" + std::to_string(geom_.grid * geom_.patch) + " pixels)"); } int heads = 0, head_dim = 0; const bool have_heads = field("VISION_NUM_HEADS", &heads), have_dim = field("VISION_HEAD_DIM", &head_dim); if (have_heads && !have_dim && heads > 0 && geom_.hidden % heads == 0) head_dim = geom_.hidden / heads; if (have_heads) { if (heads <= 0 || int64_t(heads) * head_dim != H) refuse("VISION_NUM_HEADS", heads, "head_dim " + std::to_string(head_dim) + " times that is not the tower's width " + std::to_string(geom_.hidden)); geom_.heads = heads; geom_.head_dim = head_dim; } if (c.contains("hidden_size") && c["hidden_size"].is_number_integer() && c["hidden_size"].get<int64_t>() != O) fail("config.json states the text model's hidden_size " + std::to_string(c["hidden_size"].get<int64_t>()) + " but the merger projects to " + std::to_string(geom_.out_hidden) + ": this tower is not this text model's");