[bong-water-water-bong]

3c placement, second piece — the decision an import actually iterates: per tensor, with that tensor's own layer type

#2285 (merged) mapped a role to a slot. A writer does not iterate roles, it iterates tensors, and a container mixes layer types — so the destination depends on the tensor's own layer. That is what this adds:

PlacementReport container_placement(const std::vector<Q4nxTensor>& tensors,
                                    const std::vector<std::string>& layer_types);

One PlacementRow per tensor: tensor, layer, layer_type, role, slot, why. A tensor with no layer (embed, final_norm, lm_head) reports empty layer and type and still gets its slot, because those roles do not need the second key.

The arm that matters is the pair — the same role landing in two places on two layers:

model.layers.0.self_attn.qkv_proj.weight   (linear_attention) -> linear_attn.qkv_proj.weight
model.layers.1.self_attn.qkv_proj.weight   (full_attention)   -> ""   + why
model.layers.1.self_attn.q_proj.weight                        -> self_attn.q_proj.weight
model.layers.0.mlp.gate_up_proj.weight                        -> ""   (a role this slice still refuses)
model.layers.1.unknown_thing.weight                           -> ""   (no role at all)

Mutant — use layer_types[0] for every tensor, i.e. forget whose layer it is — fails three arms:

FAIL placement: the same role on the FULL-attention layer has no destination
FAIL placement: and says why
FAIL placement: unplaced counts the two decisions, not the vocabulary gap

Three counts kept apart on purpose, because they mean three different things:

count	means	who fixes it
placed	has a destination	—
unplaced	a decision made deliberately, with the reason in why	the next slice, on purpose
no_role	a vocabulary gap — the classifier never named it	a recogniser

placed + unplaced + no_role == tensors.size() is asserted, and a row exists for every tensor. That accounting is the property that makes an import trustworthy: nothing is silently skipped, and "I could not decide" never renders as zero.

Scope: this is the decision, not the write. It computes where each tensor goes; emitting the destination pack, and the eight role refusals that need shape measurements (gate_up_proj's split, the shared-expert family), are the next slices. No device access, no container imported.

npu_pack_import_test passes; tools/copyright.py --check clean.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Layer Type Parsing Logic The logic for extracting layer numbers and types from tensor names may not correctly handle all tensor naming conventions, potentially leading to incorrect layer type assignments. This could cause placement decisions to be based on incorrect layer types, especially in cases where the tensor name does not follow the expected pattern of .layers.<number>. or .layer.<number>. std::string::size_type pos = t.name.find(".layers."); std::string::size_type off = 8; if (pos == std::string::npos) { pos = t.name.find(".layer."); off = 7; } if (pos != std::string::npos) { const std::string::size_type start = pos + off; std::string::size_type end = start; while (end < t.name.size() && t.name[end] >= '0' && t.name[end] <= '9') ++end; if (end > start) { const int64_t layer = std::atoll(t.name.substr(start, end - start).c_str()); row.layer = std::to_string(layer); if (layer >= 0 && layer < int64_t(layer_types.size())) row.layer_type = layer_types[size_t(layer)]; } } Unplaced Counting The counting of unplaced tensors assumes that all roles that do not map to a slot are deliberate decisions, but this may not always be the case. If a role is valid but the layer type does not provide a destination, it might be a configuration issue rather than an intentional decision, which could lead to incorrect placement reporting. if (row.slot.empty()) { const auto why = table.why.find(row.role); row.why = why != table.why.end() ? why->second : "the layer type does not give this role a destination"; ++out.unplaced;

[bong-water-water-bong]

VERDICT: READY at 22ac9c4 — the unit of decision moved from role to tensor, which is the unit the writer actually iterates, and the refusal examples are exactly the two shapes that would otherwise be guessed.

"A writer does not iterate roles, it iterates tensors"

That is the whole justification, and it is a fact about the loop rather than about the model:

One PlacementRow per tensor: tensor, layer, layer_type, role, slot, why

Carrying layer_type per row is what makes #2285's role→slot map usable in a container that mixes layer types — and carrying why is what makes a refusal inspectable rather than a blank. Five fields, each with a question it answers: what is it (tensor), where is it (layer, layer_type), what is it for (role), where does it go (slot), and why — the last being the one that turns a debugging session into a glance.

The two refusal examples are the right two

model.layers.0.mlp.gate_up_proj.weight    -> ""   (a role this slice still refuses)
model.layers.1.unknown_thing.weight       -> ""   (no role at all)

• the first is a known role, deliberately unplaced — npu-pack: role -> engine slot for the roles the layer type does not change (engine#1788, 3c placement) #2285's gate_up_proj case, where the split is unmeasured;
• the second is no role at all, i.e. the container has a tensor the classifier has never seen.

Both produce an empty slot, and the parenthetical shows they are different kinds of empty — which matters because the repairs differ (measure a split vs extend the classifier). A single "unplaced" bucket would lose that.

One ask

say what the writer does with an empty slot — skip the tensor (leaving the destination unfilled, which the engine must then refuse), or abort the import? Those are different guarantees: abort is safe and blocks a whole pack on one unknown tensor; skip is permissive and defers the failure to load time, where it presents as a wrong answer rather than a missing field. One line, because "refuses to guess" is only as strong as what happens next.

[bong-water-water-bong]

What this can and cannot be verified against at this layer, stated up front

Two things a reviewer will reasonably ask, answered from the code rather than left to discover:

1. The pack leg cannot cover a container, so the synthetic arms are the whole evidence here. npu_pack_import_test gates its real-pack leg on pack/model.onnx and is_oga_npu_pack(), and a container has model.q4nx by design — so none of the five containers on this box can drive this function through that leg. I checked before asserting the arms are enough:

SKIPPED, no pack directory given        # and with a container path it would still skip: no model.onnx

That means the five containers' 2,000-odd real tensors do not exercise container_placement yet. The synthetic fixture deliberately contains the shapes that matter (a role on two different layer types, a refused role, an unrecognised name), but a real-pack run arrives with the writer, which is the consumer.

2. It is not wired into info, and that is deliberate. I tried adding container_placement_put/held/unnamed to the plan struct first and reverted it: the place those numbers would be emitted reads locals in a different function, and plan fields that are set but never printed are precisely the defect I raised on #2277 ("set and never read"). A number in the plan that nothing renders is worse than no number, because it looks like evidence. The consumer of this function is the writer — the next slice — and it will read the rows, not a count.

If you would rather it surface in info now, say so and I will do it as its own small change on top, with the JSON emission and an arm that asserts the emitted value; that keeps the wiring reviewable instead of smuggled in here.

[bong-water-water-bong]

VERDICT: READY -- the per-tensor shapes are right, and the two spelling traps are handled; one uniqueness check is missing at the composite key

The two arms that matter are both here. "A row per tensor, so nothing is silently skipped" (rows.size() == v.size()) plus placed + unplaced + no_role == v.size() makes the report a partition of the container, which is the property that lets unplaced mean something: without it, an empty unplaced could mean "everything placed" or "the loop forgot half the tensors". And the pair is the arm this whole layered-key stack exists for:

model.layers.0.self_attn.qkv_proj.weight (linear_attention) -> linear_attn.qkv_proj.weight
model.layers.1.self_attn.qkv_proj.weight (full_attention)   -> ""  + why (>10 chars)

the same role landing in two different places on two layers, one destined and one refused with a reason. That is the second key demonstrated end to end rather than described.

Two traps I expected and found handled.

1. Layer-index extraction accepts both spellings: find(".layers.") with an offset of 8, falling back to find(".layer.") with 7. That matters because #2277 exists partly because MiniCPM-V spells its blocks layer.N; a placement layer that assumed layers. would file every MiniCPM tensor as layer-less and silently place it by role alone.
2. Layer-less tensors still get their slot and say so (layer.empty() && layer_type.empty() for embed/final_norm/lm_head), rather than being treated as unplaceable because they have no type.

One check I would add, and it is at the composite key. row.slot is layer-relative (self_attn.q_proj.weight) and row.layer carries the index, so the identity of a destination is the pair (layer, slot) -- and nothing asserts that pair is unique across rows. Two tensors in the same layer that both elide to the same name would produce identical (layer, slot) and the import would write one over the other with no diagnostic; the spelling work in #2277 makes that reachable, since layers.1.x and layer.1.x are exactly the shapes a container mixes. One arm:

// two rows must not claim one destination
std::set<std::pair<std::string,std::string>> seen;
for (const auto & r : r.rows) if (!r.slot.empty() && !seen.insert({r.layer, r.slot}).second) { /* report both tensors */ }

Either assert uniqueness or report it the way spans are reported in the writer (named, not resolved) -- but a silent last-writer-wins is the one outcome this stack has otherwise been careful to avoid.

Not blocking, and the PR is a clean second piece: #2285 mapped role -> slot, this answers per tensor with that tensor's own layer type, and unplaced + why keeps the refusals visible. The (layer, slot) check is the same invariant as #2285's no two roles share a destination, one level down -- where the key has two parts instead of one.