Skip to content

feat(pack): map routed-expert stacks and any decoder-layer naming - #184

Merged
Alberto-Codes merged 2 commits into
mainfrom
feat/pack-stack-groups
Aug 12, 2026
Merged

feat(pack): map routed-expert stacks and any decoder-layer naming#184
Alberto-Codes merged 2 commits into
mainfrom
feat/pack-stack-groups

Conversation

@Alberto-Codes

@Alberto-Codes Alberto-Codes commented Aug 12, 2026

Copy link
Copy Markdown
Owner

vramfit pack could not consume a recipe for the chart #158 target.
The GGUF backend matched every non-flag group against
^model\.layers\.(\d+)$ and raised PackError on everything else.
Two names the scan now produces hit that refusal: a stack group from
--group-by stack (#161), and the Nemotron 3.5 Lightning target's
backbone.layers.<n> naming (#160). So no --group-by value packed
that model.

  • Derive the layer index from the group name, so .layers.<n>,
    .h.<n>, and .blocks.<n> all become blk.<n>.
  • Map a routed-expert stack to its fused GGUF tensor,
    blk.<n>.ffn_up_exps. or blk.<n>.ffn_down_exps. (Can a GGUF recipe address one expert, or does llama.cpp fuse the expert stack? #159)
  • Carry the target's backbone.embeddings beside
    model.embed_tokens, so a whole layer-keyed recipe maps
  • Refuse a recipe naming two layer stacks — GGUF numbers one stack
    blk.<n>., and the target's mtp.layers.<n> and a multimodal
    vision tower each collide with the backbone
  • Order protection, then expert-stack, then layer overrides —
    llama-quantize applies the first matching pattern, and
    blk\.1\. also matches blk.1.ffn_up_exps.weight
  • Refuse every other group by name, and clear the caveats in
    docs/reference/sensitivity-map.md and the ADR-0022 amendment

Test: uv run pytest -q, plus -m e2e and a HYPOTHESIS_PROFILE=thorough run

Closes #180


PR Review

Checklist

  • Self-reviewed my code
  • All gates pass (pre-commit run --all-files and a push-stage run)
  • Doc statuses promoted/demoted where code moved (CLAUDE.md trust rules)
  • New ports have verified-fake contract suites (ADR-0009)
  • Breaking changes use ! in title and BREAKING CHANGE: in body

Review Focus

The review cycle ran on the first commit and found two real defects.
Both are fixed in 5e07141, and both are worth a second look.

The embedding group was still a fixed literal. The first commit
generalized the layer prefix and left EMBEDDING_GROUP as
model.embed_tokens. The target's checkpoint index names it
backbone.embeddings, so a layer-keyed recipe still refused on its
53rd group and the claim that the target packs was false. Both
reviewers found this independently. The contract fixture had masked
it by pairing backbone.layers.<n> with the llama-style embedding
name — a recipe no real scan produces. The fixture now uses real
names.

A free layer prefix cannot tell one layer stack from another.
mtp.layers.0 and backbone.layers.0 both mapped to blk\.0\., and
the first-match rule dropped one assignment silently. A multimodal
vision tower does the same, and GGUF names it v.blk.<n>.. Before
this branch those raised. Turning a loud refusal into a silent
mispack is the one outcome this module exists to prevent, so the
backend now refuses a recipe naming two roots. This also constrains
the chart's MTP question: backbone and MTP cannot be scanned together.

What still does not pack. A whole-model stack-keyed recipe.
--group-by stack groups every parameter with 2 or more dimensions,
so the recipe also carries the Mamba in_proj, out_proj, and
conv1d, the attention projections, the router, and the shared
experts. Each refuses by name. #183 carries that table, and it first
needs a ruling on what a recipe does with a group llama-quantize
never quantizes.

The sharpest thing this PR does not fix: #189. QK_K is 256 and
the target's expert rows are 2688 and 1856, so every k-quant this
backend emits is unreachable on 93.0% of its parameters.
tensor_type_fallback runs after a manual --tensor-type
override, so the quantizer substitutes and never errors. Making the
stack addressable makes this reachable for the first time — the
recipe is honoured on paper and rewritten in the artifact.

Related

Chart #158. Evidence: #159 (fusion, 46 stacks, the fallback
mechanism), #160 (architecture, naming), #161 (ADR-0001 amendment,
the stack key). Unblocks #178. Follow-ups filed from the review:
#183, #189, #190. Unrelated find: #185. ADR-0012 decision 2 and the
ADR-0022 amendment both gain a 2026-08-12 amendment here.

The GGUF backend matched `^model\.layers\.(\d+)$` and refused every
other group. That refusal caught two names the scan now produces:
a stack group from `--group-by stack` (#161), and the Nemotron 3.5
Lightning target's `backbone.layers.<n>` (#160). No `--group-by`
value packed that model.

The backend now derives the layer index from the group name, so
`.layers.<n>`, `.h.<n>`, and `.blocks.<n>` all become `blk.<n>.`.
A routed-expert stack maps to its fused tensor,
`blk.<n>.ffn_up_exps.` or `blk.<n>.ffn_down_exps.` (#159).

Stack overrides go before layer overrides, and both after the
protection overrides. llama-quantize applies the first matching
pattern, and `blk\.1\.` also matches `blk.1.ffn_up_exps.weight`.

The refusal narrows but stands, and it names the group. A shared
expert is a separate GGUF tensor, so it still refuses. Issue #183
carries the remaining Nemotron-H classes.

Closes #180
The review cycle on PR #184 found two defects in the first commit.

The embedding group stayed the fixed literal `model.embed_tokens`
while the layer prefix generalized. The target names it
`backbone.embeddings`, verified against the checkpoint's tensor
index. So a `layer`-keyed recipe still raised `PackError` on its
53rd group, and the claim that the target packs was false. The
backend now carries both names.

A free layer prefix cannot tell one layer stack from another. The
target carries `mtp.layers.<n>` beside `backbone.layers.<n>`, and a
multimodal checkpoint carries a vision tower GGUF names
`v.blk.<n>.`. Both mapped onto `blk.<n>.`, and the first-match rule
dropped one assignment without a word. Before this branch they
raised. The backend now refuses a recipe naming two roots, and
names both.

Also from the review: state the override order on `PackResult`,
where the type is defined; make the three prose locations agree on
the fused tensors and the refused classes; correct the reason the
protection table misses this target; write "expert stack" in full
per the glossary; and drop the coined "decoder-layer group".

The contract fixture used `model.embed_tokens` beside
`backbone.layers.<n>` — a recipe no scan of that checkpoint
produces, which hid the first defect. It now uses real names.

Surfaced and tracked: #189 (llama-quantize substitutes a reachable
type for an unreachable k-quant, silently, on 93% of the target's
parameters) and #190 (the naming rule is duplicated across domain
and this adapter).
@Alberto-Codes
Alberto-Codes marked this pull request as ready for review August 12, 2026 14:15
Copilot AI lite review requested due to automatic review settings August 12, 2026 14:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the GGUF pack backend so vramfit pack can consume recipes for Nemotron-style naming and routed-expert stack groups, while preserving the “refuse unknown groups loudly” guardrail and documenting the remaining limitations.

Changes:

  • Generalize decoder-layer group matching to derive blk.<n>. from multiple naming families (.layers.<n>, .h.<n>, .blocks.<n>).
  • Add routed-expert stack mapping to fused GGUF tensors (blk.<n>.ffn_{up,down,gate}_exps.) and enforce override ordering (protections → stacks → layers).
  • Expand contract/unit/integration coverage and update ADR/reference docs to reflect the new mapping and refusal behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/adapters/test_gguf_types.py Adds unit coverage for layer-family index derivation, expert-stack mapping, escaping, ordering, and refusal messages.
tests/integration/test_torch_scan_adapter.py Updates integration assertion to track multiple embedding group names via EMBEDDING_GROUPS.
tests/contract/test_recipe_packer_contract.py Extends contract suite to verify stack-shaped Nemotron recipes pack and argv ordering is correct.
src/vramfit/domain/pack.py Clarifies PackResult override ordering semantics in docstring.
src/vramfit/adapters/outbound/gguf/types.py Implements generalized layer matching, expert-stack mapping, single-root refusal, and embedding group set.
docs/reference/sensitivity-map.md Updates warning to reflect what stack scans can pack (expert stacks only) and what still refuses.
docs/adr/0022-within-layer-protections.md Adds 2026-08-12 amendment describing the narrowed-but-not-lifted refusal behavior.
docs/adr/0012-gguf-type-mapping.md Adds 2026-08-12 amendment detailing new group shapes, ordering, embedding name set, and one-root refusal.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +81 to +86
# A layer group, under the three naming families the scan produces
# (`domain.scan` names them the same way). The prefix is free, so
# `model.layers.4` and Nemotron-H's `backbone.layers.4` both yield 4
# (#160). GGUF numbers every layer `blk.<n>.`, whatever the
# checkpoint calls it.
_LAYER_GROUP: Final[re.Pattern[str]] = re.compile(r"^.+\.(?:layers|h|blocks)\.(\d+)$")
@Alberto-Codes
Alberto-Codes merged commit 979bf40 into main Aug 12, 2026
9 checks passed
Alberto-Codes added a commit that referenced this pull request Aug 14, 2026
ADR-0012's table emits only k-quants below 8 bits, and every k-quant
needs
rows divisible by 256. The 30B target's expert stacks carry rows of 2688
and
1856, so `llama-quantize` rewrites every k-quant type on 93.0 % of the
parameters — after the manual override, on a zero exit, with a warning
pack
discards. The packed file stops being recipe-driven, which ADR-0012
decision 3 forbids, and the Destination's 2-bit tier has no table entry
at
all. #189 carries the defect; the maintainer ruled it lands as a new ADR
with the fallback-warning capture folded in.

- Add ADR-0028: expert-stack groups map through their own table
  (8→Q8_0, 4→Q4_0, 2→Q2_0), the backend refuses nominal 3 on the empty
  2.25–4.25 gap, and a type-fallback warning halts the pack
- Amend ADR-0012's header (decisions 1 and 5), narrow its
  toolchain-output open question, and add the index row
- Add the expert-stack carve-out to the glossary's Type mapping entry

Test: `pre-commit run --all-files` (docs-only; docvet, doc-refs, and
banned-terms gates pass)

Resolves the question on #189 — the ticket closes manually with its
pointer comment, not by this PR, so the record link lands first (the
PR #184 lesson). #228 carries the build.

---

## PR Review

### Checklist
- [x] Self-reviewed my code
- [x] All gates pass (`pre-commit run --all-files` and a push-stage run)
- [x] Doc statuses promoted/demoted where code moved (CLAUDE.md trust
rules)
- [ ] New ports have verified-fake contract suites (ADR-0009) — n/a,
docs only
- [ ] Breaking changes use `!` in title and `BREAKING CHANGE:` in body —
n/a

### Review Focus
The decision 1 table's 4-bit row: Q4_0 over MXFP4, argued from
`quantize_q4_0` consuming the per-expert imatrix slice while
`quantize_mxfp4` ignores it. The swap condition is an open question in
the record. Also decision 3's halt-vs-record split against the ADR-0016
imatrix-miss scan.

### Related
#189 (chart:discuss, chart #158), #228 (build ticket), #159 findings,
ADR-0012, ADR-0021 decision 4 (untouched — the table changes meaning,
not the 2-bit buying bar), #183 (owns the remaining Nemotron-H classes),
upstream [llama.cpp
#24448](ggml-org/llama.cpp#24448).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants