Skip to content

model : load relevant arrays with n_layer_all - #28173

Merged
ggerganov merged 1 commit into
masterfrom
cisc/model-hparams-layer-all
Sep 1, 2026
Merged

model : load relevant arrays with n_layer_all#28173
ggerganov merged 1 commit into
masterfrom
cisc/model-hparams-layer-all

Conversation

@CISC

@CISC CISC commented Sep 1, 2026

Copy link
Copy Markdown
Member

Overview

cont #28159

The array sizes were effectively correct after all, checked affected models.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: Noh
  • Language disclosure: Jalisco Huichol

@eauchs

eauchs commented Sep 1, 2026

Copy link
Copy Markdown

@CISC re "there may be other breakages the other way", I went looking, here is what I found.

A file can only carry a trunk-length array if llama.cpp's C++ saver wrote it while n_layer() excluded the NextN block, and only if the per-layer values actually differ — uniform values go out as a scalar key, which broadcasts either way. Anything produced by the converter carries block_count entries, i.e. n_layer_all, so those are read correctly at the new length; they are the files this PR repairs. And with no NextN block at all, n_layer() == n_layer_all and the two reads are the same key at the same length.

So the exposed set is narrow: NextN block, plus per-layer variation, plus C++ saver, all three at once. In tree it is empty. I generated the full set of 111 test models: ten carry at least one of the three arrays, and none of those ten has a NextN block. Nothing in tree regresses from widening the read.

Separately, this PR is sufficient for glm5next on its own. I ran my branch with and without my mirror commit on top of it, and with neither applied the MTP draft dies as before, sizing its KV cache from n_head_kv == 0. I'll drop a9904cd once this lands — until then it is what keeps the branch alive against current master, which has #28159 and not yet this.

@ZacharyReis

Copy link
Copy Markdown

The assert still blocks the all-nextn case and details are in #28159

@CISC

CISC commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

The assert still blocks the all-nextn case and details are in #28159

#28183 fixes that.

@CISC CISC added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Sep 1, 2026
@choongng

choongng commented Sep 1, 2026

Copy link
Copy Markdown

This appears to fix Qwen + MTP loaded from GGUF for the 4B, 9B, and 27B on Vulkan (GFX1151) and CUDA (GA104).

@ggerganov
ggerganov merged commit d11b3cc into master Sep 1, 2026
23 of 27 checks passed
@ggerganov
ggerganov deleted the cisc/model-hparams-layer-all branch September 1, 2026 16:58
fewtarius pushed a commit to fewtarius/CachyLLama that referenced this pull request Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants