From 419b881c02dcbdd7ecc47d2b5c935e269642269d Mon Sep 17 00:00:00 2001 From: Xuan-Son Nguyen Date: Mon, 27 Jul 2026 12:00:56 +0200 Subject: [PATCH] docs: add exception about weight folding (#26168) * docs: add exception about weight folding * add example --- docs/development/HOWTO-add-model.md | 2 ++ skills/add-new-model/SKILL.md | 1 + 2 files changed, 3 insertions(+) diff --git a/docs/development/HOWTO-add-model.md b/docs/development/HOWTO-add-model.md index 632e79881a..102f479eb0 100644 --- a/docs/development/HOWTO-add-model.md +++ b/docs/development/HOWTO-add-model.md @@ -144,6 +144,8 @@ Examples: - Gemma 3 folds the `1 +` of its `norm(1 + weight)` normalization into the weights at conversion time, so the graph just does a plain RMS norm. - Qwen3-Next applies its tensor permutation during conversion (in `modify_tensors`), so the graph can consume the already-permuted weights directly. +Exception: a plain `weight * scale` with a constant scale is usually better left to inference time rather than folded into the weight at conversion. The scale conceptually applies to the activation, not the weight, so folding it into the weight can hurt numerical stability, and it shifts the weight's value range in a way that can make quantization worse. In this case, write the scale to GGUF as its own metadata key (e.g. `%s.attention.output_scale`, `%s.attention.value_scale`, `%s.embedding_scale`) and apply it in the graph, instead of pre-multiplying the weight tensor during conversion. + ### Working with ggml_rope_ext PyTorch implementations usually prefer explicitly calculating `freq_cis`/`sin`/`cos` components. However, in llama.cpp, most RoPE operations can be handled via `ggml_rope_ext`, which does not require a sin/cos matrix. This saves memory while allowing the GGML RoPE kernel to be fused with other ops. diff --git a/skills/add-new-model/SKILL.md b/skills/add-new-model/SKILL.md index 68be866c7b..f76d1abfd7 100644 --- a/skills/add-new-model/SKILL.md +++ b/skills/add-new-model/SKILL.md @@ -76,6 +76,7 @@ These recur often enough in review comments on past add-model PRs that they're w - Don't ship unfinished or unverified speculative-decoding (e.g. MTP) scaffolding in the base model PR - if it hasn't actually been confirmed to work, pull it out and land it as its own follow-up. - Conversion code should call into the base class's existing hparam logic (e.g. `super().set_gguf_parameters()`) rather than re-deriving it - large blocks of code that duplicate what `TextModel`/`MmprojModel` already provide will get flagged as redundant. - Do constant tensor modifications (e.g. `norm(1 + weight)`) and permutations/chunking at conversion time, not in the graph - see HOWTO-add-model.md's "Prefer conversion-time tensor modifications" tip (Gemma 3 folds its `1 +` into the weights, Qwen3-Next permutes in `modify_tensors`). Doing these at runtime in the graph is very likely to be rejected as over-complicated; if you genuinely can't do it at conversion time, open a discussion first explaining why rather than implementing it in the graph. + - Exception: a plain `weight * scale` with a constant scale is usually better applied at inference time instead of being folded into the weight at conversion. The scale conceptually applies to the activation, not the weight, so folding it in can hurt numerical stability, and it shifts the weight's value range in a way that can make quantization worse. ## Validation checklist