Spare 1-D norm weights from a blanket --type - #1861
Conversation
tensor_should_be_converted has no rule for 1-D weights, so a per-channel norm
scale survives a blanket --type only by accident: when its length does not
divide the quant block size (FLUX q_norm/k_norm are [128] and 128 % 256 != 0),
or when one of the FLUX-era name rules happens to match it.
MiniMax-H3's per-block norms are [5376], and 5376 % 256 == 0, so a blanket
--type q4_K quantizes 105 of them. The result loads and renders a plausible
video, so a "does it run" check passes it, but scored against a bf16 render of
the same prompt and seed it is destroyed.
A 1-D weight is a per-channel gain, never a matmul weight. Every channel of a
block would share one scale and one min, and a gain vector has no reason to be
locally smooth, so quantizing it buys almost nothing and costs a lot: on this
model holding all 211 1-D tensors adds 0.77 MiB to a 10.60 GiB file, 0.007%.
Measured on minimax_h3_fl2va_pruned, converted from the same bf16 checkpoint
with a blanket --type q4_K and no --tensor-type-rules, rendered at 640x384,
25 frames, 4 steps, cfg 1.0, seed 1234, --rng cpu, and scored against a bf16
render of that same prompt and seed:
1-D tensor types PSNR SSIM LPIPS
before F32 5, F16 51, BF16 106, Q4_K 105 9.87 0.074 0.981
after F32 5, F16 51, BF16 211 22.22 0.841 0.292
SSIM 0.074 means the output is essentially uncorrelated with the reference.
The 1-D rule alone is sufficient. A build made with
--tensor-type-rules 'norm[0-9]*\.weight$=bf16,condition_proj\.weight$=bf16'
produces the same 1-D layout and scores 21.52 / 0.838 / 0.299, so nothing here
depends on also sparing condition_proj.weight, which is 2-D and a separate
quality choice. Peak VRAM is unchanged at 16.83 GiB.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24a4aff97b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // crushed to 4 bits by a blanket --type. The result still loads and still renders a | ||
| // plausible image, so a "does it run" check passes it, while measured against a bf16 | ||
| // render of the same prompt and seed it is destroyed: PSNR 9.87 / SSIM 0.074 / | ||
| // LPIPS 0.981, against 22.22 / 0.841 / 0.292 with this rule in place. |
There was a problem hiding this comment.
Remove the model-specific history and PSNR/SSIM/LPIPS results from this comment: these non-reproducible measurements document this particular change rather than an enduring constraint and will become misleading as models or quantizers evolve. Keep only the invariant that quantizing one-dimensional gain vectors damages quality; the detailed evidence belongs in the commit description, documentation, or a regression test.
AGENTS.md reference: AGENTS.md:L127-L134
Useful? React with 👍 / 👎.
| // Pass, do not convert. A 1-D weight is a per-channel scale (LayerNorm/RMSNorm gain), | ||
| // never a matmul weight, so quantizing it buys almost nothing and costs a lot: every |
There was a problem hiding this comment.
Correct the claim that every one-dimensional weight is a normalization gain: this repository also creates one-dimensional learned class embeddings (src/model/te/clip.hpp), pad tokens (src/model/diffusion/z_image.hpp), and modulation offsets (src/model/diffusion/krea2.hpp). The new condition intentionally exempts all of these from quantization, so documenting a narrower invariant misstates the behavior and may cause a maintainer to incorrectly replace the shape check with a norm-name check.
AGENTS.md reference: AGENTS.md:L129-L132
Useful? React with 👍 / 👎.
| if (type != GGML_TYPE_COUNT) { | ||
| if (ggml_is_quantized(type) && tensor_storage.ne[0] % ggml_blck_size(type) != 0) { | ||
| // Pass, do not convert | ||
| } else if (ggml_is_quantized(type) && tensor_storage.n_dims <= 1) { |
There was a problem hiding this comment.
Do not use TensorStorage::n_dims here as proof that a GGUF tensor is logically one-dimensional. src/model_io/gguf_io.cpp populates it with ggml_n_dims(dummy), which drops a trailing singleton dimension, so a real 2-D matmul such as Krea2's Linear(text_layers, 1) projector is read back with n_dims == 1 and silently excluded from a requested blanket quantization. This makes GGUF-to-GGUF conversion or runtime --type handling depend on the input container rather than the tensor's logical shape.
AGENTS.md reference: AGENTS.md:L178-L180
Useful? React with 👍 / 👎.
The prebuilt pipeline builds leejet's source at an aged release tag, not this fork's master, so the three MiniMax-H3 fixes on master reach nobody: every Studio user installs a binary that still aborts on the default cfg-scale, still aborts on --vae-on-cpu, and still quantizes H3's 1-D norms into an output uncorrelated with its own bf16 reference. Building from master instead would throw away the reason the pipeline is shaped this way, which is that what we publish should be traceable to a specific upstream release. So keep the upstream tag as the base and carry the delta explicitly: - patches/ holds one file per fix, each with its upstream pull request in the header. All three are open on leejet: leejet#1861, leejet#1862, leejet#1863. - resolve applies them to the checked-out tag, after running git apply --check over the whole set so a stale patch stops the run before the tree is half modified. That failure is the signal to delete the patch (upstream merged it) or refresh it (upstream moved the code). - a non-empty set moves the published tag to <upstream tag>-u<id>, where id is the sha256 prefix of the concatenated patches. The tag then says whether a box is stock, and a changed patch set republishes rather than matching an existing release and skipping. - the manifest and the release notes both record the applied list. An empty patches/ leaves the tag and every asset name exactly as they are today. Verified by running the resolve step against master-813-bfbef5b with gh stubbed: all three patches apply, the tag becomes master-813-bfbef5b-u<id>, and the stamped source tarball contains the fixes.
The pin was master-812-ea7f0c8, a stock upstream build, and on a stock build H3 does not work: it aborts on the default --cfg-scale, aborts again on --vae-on-cpu, and its 1-D norms are quantized into an output uncorrelated with its own bf16 reference (LPIPS 0.981). The Studio side worked around the first by pinning cfg to 1.0 and the second by dropping the flag; the third had no workaround on the consumer side at all. All three are fixed in unslothai/stable-diffusion.cpp and open upstream as leejet/stable-diffusion.cpp#1861, unslothai#1862 and unslothai#1863. The mirror's prebuilt pipeline now applies them on top of the aged upstream tag it already builds, and marks such a build with a -u<id> suffix naming the patch set, so master-813-bfbef5b-u0665242 is upstream master-813-bfbef5b plus those three patches and nothing else. The patches are deleted once upstream releases them, at which point this pin goes back to a plain tag. Verified on the published Linux x86_64 asset, not on a local build: both new error strings are in the shipped sd-cli, and running it on a q4_K H3 denoiser without --mode vid_gen now exits 1 with the instruction instead of core dumping on a ggml assert. test_video_backend's fake engine returned the old tag as its version string, which read like a second pin; it only needs a non-None value, so it now says so.
1 participant
A blanket `--type` currently quantizes 1-D weights whenever their length happens to divide the block size. A 1-D weight is a per-channel gain (LayerNorm/RMSNorm), never a matmul weight, so every channel of a block shares one scale and one min while a gain vector has no reason to be locally smooth.
Until now these survived only by accident: FLUX q_norm/k_norm are `[128]` and `128 % 256 != 0`, so the existing block-size check spared them. MiniMax-H3 norms are `[5376]` and `5376 % 256 == 0`, so a blanket `--type q4_K` crushed 106 norm scales to 4 bits.
The failure is quiet, which is what makes it worth guarding. The model still loads and still renders a plausible-looking video, so a "does it run" check passes. Measured against a bf16 render of the same prompt and seed:
An LPIPS of 0.981 is essentially uncorrelated with the reference.
The rule sits beside the existing `.bias` and `.scale` exclusions and costs almost nothing in size: on H3 the 106 spared tensors are 5376 floats each.