IQ1_XS, IQ1_XXS, IQ1_XXXS carried onto b10775 - #171
Conversation
Three sub-IQ1_S data types: IQ1_S's algorithm with the codebook the mapping step may choose from cut to a subset of iq1s_grid, and the index stored in fewer bits. IQ1_XS id 64 46 B / 256 1024-entry grid 10-bit index 1.4375 bpw IQ1_XXS id 65 42 B / 256 512-entry grid 9-bit index 1.3125 bpw IQ1_XXXS id 66 38 B / 256 256-entry grid 8-bit index 1.1875 bpw Dequantization is IQ1_S's rule throughout, y[j] = d*(2*ls + 1)*(grid[idx][j] + delta). GGML_TYPE_COUNT is 67; 42..63 are left for upstream, which already uses 42 for Q2_0, so a stock build rejects these files rather than misreading them. CPU, CUDA dequant, MMVQ and MMQ. The MMQ path targets the current ggml_cuda_mmq_util_funcs / ggml_cuda_mmq_config API and registers the types in every per-architecture config table; Blackwell falls through to Ampere and needs none. scripts/iqxs_gen_tables.py regenerates all six codebook tables in ggml-common.h byte-identically from the three committed .npy grids, so the tables are reproducible rather than pasted. Tested on B200, CUDA 13, sm_100. test-backend-ops -o MUL_MAT passes 2/2 backends both default and with GGML_CUDA_FORCE_MMQ=1, 11 cases per type matching IQ1_S. Note the default run never enters the new MMQ kernels, since small n routes to MMVQ.
# Conflicts: # ggml/src/ggml-common.h
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 95e08993a6
ℹ️ 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".
| // IQ1_S spends 1.3750 of its 1.5625 bpw on an 11-bit index into a 2048-entry grid, i.e. 88% of | ||
| // the whole budget, so the index width is the only field worth shrinking. Every other field | ||
| // keeps its IQ1_S meaning exactly: `d` is the fp16 super-block scale, the 3-bit sub-block code | ||
| // gives `dl = d*(2*ls+1)`, and the sign bit selects `delta = +/-IQ1S_DELTA`. Dequantisation is | ||
| // therefore identical to `dequantize_row_iq1_s` apart from how the index is reassembled. |
There was a problem hiding this comment.
Replace this long, hard-wrapped explanation with a concise description of the non-obvious storage invariant. The current 11-line block splits sentences across comment lines and includes design rationale that the adjacent field comments already convey, contrary to the repository's explicit comment requirements.
AGENTS.md reference: AGENTS.md:L74-L78
Useful? React with 👍 / 👎.
| GGML_TYPE_IQ1_XS = 64, // 1.4375 bpw, 1024-entry grid | ||
| GGML_TYPE_IQ1_XXS = 65, // 1.3125 bpw, 512-entry grid | ||
| GGML_TYPE_IQ1_XXXS = 66, // 1.1875 bpw, 256-entry grid | ||
| GGML_TYPE_COUNT = 67, |
There was a problem hiding this comment.
When a GGUF or API caller supplies one of the reserved values 43 through 63, raising GGML_TYPE_COUNT to 67 makes the value pass existing < GGML_TYPE_COUNT checks even though its type_traits entry is zero-initialized. For example, the range check in gguf.cpp:714-721 now treats such an ID as defined, while ggml_validate_row_data() can evaluate nbytes % ggml_type_size(type) with a zero divisor. Preserve the sparse IDs while adding an explicit defined-type validity check so reserved values are rejected safely.
Useful? React with 👍 / 👎.
| case LLAMA_FTYPE_MOSTLY_IQ1_XS: name = LLAMA_FTYPE_PREFIX "IQ1_XS - 1.4375 bpw"; break; | ||
| case LLAMA_FTYPE_MOSTLY_IQ1_XXS: name = LLAMA_FTYPE_PREFIX "IQ1_XXS - 1.3125 bpw"; break; | ||
| case LLAMA_FTYPE_MOSTLY_IQ1_XXXS: name = LLAMA_FTYPE_PREFIX "IQ1_XXXS - 1.1875 bpw"; break; |
There was a problem hiding this comment.
When general.file_type is absent, llama_model_loader infers the value from the most common tensor type using the switch at src/llama-model-loader.cpp:750-782, but that switch has no cases for these three newly supported tensor types. Such a model is consequently reported as guessed F32, and an only_copy quantization writes that incorrect value back at llama-quant.cpp:974-1011; add the corresponding inference cases alongside the existing IQ1_S and IQ1_M mappings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46cbf0e957
ℹ️ 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".
| case GGML_TYPE_IQ1_XS: | ||
| case GGML_TYPE_IQ1_XXS: | ||
| case GGML_TYPE_IQ1_XXXS: |
There was a problem hiding this comment.
On an sm_60 CUDA build processing an MoE MUL_MAT_ID batch above the MMVQ limit, these cases make ggml_cuda_should_use_mmq() return true through the pre-DP4A MoE path, but mmq-config-pascal-older.cuh has no configuration for any of the new types. Consequently mul_mat_q_switch_J() leaves J_best at zero and aborts; add matching Pascal-older configurations or exclude these types from MMQ on that architecture.
Useful? React with 👍 / 👎.
|
Superseded by #61, which now carries this exact payload on the same base.
Consolidating onto #61 keeps the original PR number, its 7 comments and 2 reviews, and removes the need for a second PR per base tag. The pin in #174 points at #61 now. |
1 participant
Re-carries the work from #91 onto
b10775so the nightly has something current to pin.#91 is merged into fork master, which per
scripts/unsloth/pr-set.jsondrops it from the nightly: the release tree is an upstream tag plus pins, and master is not in it. So the pin stays listed until the change lands upstream, and it was still pointing atc86ed2699, which forked from0666ad2b2on 08-10.By
b10775that pin no longer merges:The three
NGRID_*defines go in immediately above#if defined(GGML_COMMON_IMPL_C), and upstream has since widened that line to#if defined(GGML_COMMON_IMPL_C) || defined(GGML_COMMON_IMPL_CPP). An add sitting on top of an edit is not somethingadditive_merge.pycan prove is a pure add/add, and it is right to refuse it. Resolved by hand: keep the three defines, keep upstream's widened guard.The defines are plain macros outside the guard, so widening it does not change what they mean.
That was the only conflict.
git diff b10775..HEADis 41 files, +2334/-25, the same delta as #91.Base branch is
base/upstream-67a17c17c, which isb10775verbatim, so the diff here is the quant types and nothing else.