Skip to content

model-saver: fix per-layer array lengths for MTP models - #30891

Draft
ravel7524 wants to merge 1 commit into
ggml-org:masterfrom
ravel7524:fix-model-saver-per-layer-arrays
Draft

ravel7524 wants to merge 1 commit into
ggml-org:masterfrom
ravel7524:fix-model-saver-per-layer-arrays

Conversation

@ravel7524

Copy link
Copy Markdown
Contributor

Overview

This PR fixes GGUF save/reload failures for models with MTP (nextn) layers. llama_model_save_to_file() wrote per-layer arrays for the main layers only, while the loader expects all layers, including MTP.

Per-layer arrays now use n_layer_all, except attention.indexer.types, which keeps the main-layer length and is always an array. All four readers (glm-dsa, glm5-next, hy-v4 and dots3note) accept arrays; dots3note requires one.

The saver emits this key for every architecture: its scalar-to-array encoding changed in 132/134 generated GGUFs, with identical tensor bytes; architectures that do not read the key ignore it.

Additional information

Reproduction: save Qwen3.5-0.8B Q8_0 with llama_model_save_to_file(), then reload it: qwen35.attention.recurrent_layers has wrong array length; expected 25, got 24. With the new regression test applied to baseline master f711c8f30, test-llama-archs -a qwen35 exits 255 with expected 3, got 2; it passes with the fix.

CPU testing:

  • Full test-llama-archs: 139/139 passed, seeds 1 and 42, including MTP coverage for qwen35 and mixed-indexer glm-dsa.
  • Public Qwen3.5-0.8B Q8_0 saves and reloads with MTP loading enabled and disabled.

The regression dates from the hparams refactor #24060. #30880 overlaps the saver and test refactor; I will rebase this fix if that lands first.

Requirements

  • Yes, I have read and agree with the contributing guidelines.
  • AI usage disclosure: YES, AI was used for a Workflow, implementation, tests, validation, and review.

@github-actions github-actions Bot added the testing Everything test related label Oct 20, 2026
@m4zar

m4zar commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Tested this PR against a case not covered in the description: the Gemma 4 assistant (E4B, Q8_0, AtomicChat/gemma-4-E4B-it-assistant-GGUF). In this model, nextn_predict_layers == block_count, resulting in n_layer() == 0.

  • On master: The saver drops head_count, head_count_kv, and feed_forward_length. The saved file fails to load.
  • With this PR: All three keys are preserved with their original values, and the model reloads successfully with matching hparams.
gemma-4-E4B-it-assistant.Q8_0.gguf (block_count 4, nextn_predict_layers 4, n_layer() 0)

key                                      original   saved (master)   saved (#30891)
gemma4-assistant.attention.head_count    u32 4      missing          u32 4
gemma4-assistant.attention.head_count_kv u32 2      missing          u32 2
gemma4-assistant.feed_forward_length     u32 2048   missing          u32 2048

reload, master:  check_tensor_dims: tensor 'blk.0.attn_q.weight' has wrong shape; expected 256, 0, got 256, 1024
reload, #30891:  loads; n_head 4, n_head_kv 2, n_ff 2048, n_embd_head_k 512 (same as original)
tensors: 49/49 identical (name, type, shape, sha256) in all three files

@ravel7524
ravel7524 force-pushed the fix-model-saver-per-layer-arrays branch from 7c02d60 to be2ed4e Compare November 6, 2026 06:47

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants