Emit GGUF zero points explicitly - #459
Conversation
Treat GGUF Q4_0 and Q8_0 as requiring explicit zero-point initializers so MatMulNBits and GatherBlockQuantized do not rely on execution-provider-specific defaults. Add a GatherBlockQuantized dequantization regression that checks the zero-point value, not only node wiring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes GGUF Q4_0/Q8_0 correctness by ensuring the ONNX graph always supplies explicit zero_points inputs to MatMulNBits and GatherBlockQuantized, avoiding provider-dependent defaults (notably CPU vs CUDA divergence) that can corrupt embeddings.
Changes:
- Update GGUF quant-param detection so Q4_0/Q8_0 no longer allow omitting
zero_points(forcing explicit zp initializers). - Add/extend GGUF build tests to assert explicit
.zero_pointsinitializers and 4-input contrib ops, plus a runtime regression forGatherBlockQuantizeddequantization semantics.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/mobius/integrations/gguf/_builder.py |
Changes quant-param detection logic to force explicit zero-points for Q4_0/Q8_0 (and updates related naming/semantics). |
src/mobius/integrations/gguf/_builder_test.py |
Adds assertions + a runtime regression to verify correct zero-point handling for MatMulNBits / GatherBlockQuantized. |
Suppressed comments (1)
src/mobius/integrations/gguf/_builder.py:535
- The return contract is still documented as
is_symmetric, but the implementation now treats the third return value as “can omit zero_points” (and Q4_0/Q8_0 now returnFalsedespite being symmetric formats). Updating the docstring helps prevent future callers from misusing this boolean.
# Whether the graph can omit zero_points for each supported GGUF type.
#
# Mainline Q1_0 (1-bit binary) is repacked into 2-bit MatMulNBits
# with zp=1 — see _repack_q1_0. Tencent's custom Q1_0 (2-bit SEQ,
# 512-elt blocks) is inflated to 4-bit MatMulNBits with zp=3 — see
|
@copilot do not use onnx or helper module, use onnx-ir instead for building the test model. Also fix lint and unit tests |
Replace onnx.helper/numpy_helper model construction in the GatherBlockQuantized regression with onnx_ir APIs and validate all MatMulNBits nodes carry explicit zero_points initializers. Signed-off-by: GitHub <noreply@github.com> Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
|
|
Bind qweight/scales/zero_points initializers to named ir.Value objects and reuse them for both node inputs and graph initializers. Signed-off-by: GitHub <noreply@github.com> Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Addressed in commits f77907f and d5182bc. The GatherBlockQuantized runtime regression now builds the model with |
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Summary
This is the clean-main rebuild of the urgent correctness fix from #458. It contains only the GGUF zero-point fix, based on current
origin/main.GGUF Q4_0 and Q8_0 are symmetric formats, but their dequantization formulas still require non-zero zero points:
(q - 8) * scale(q - 128) * scaleMobius previously marked those GGUF imports as symmetric, so
QuantizedLinearandQuantizedEmbeddingomitted the optionalzero_pointsinput forMatMulNBitsandGatherBlockQuantized. That is not portable:GatherBlockQuantizeddefaults diverge between ORT CPU and ORT CUDA whenzero_pointsis omitted, corrupting CUDA embeddings before the first decoder layer runs.This PR makes GGUF Q4_0/Q8_0 emit explicit zero-point initializers, so both
GatherBlockQuantizedandMatMulNBitsreceive the intended values instead of relying on EP defaults.Regression test
Added a single-node
GatherBlockQuantizedruntime regression:108(q - 8) * scale0is asserted not to matchI also temporarily flipped the test's zero point to
0; it failed with a 100% output mismatch, so the test checks the value and not just the presence of the input.Synthetic GGUF tests now also assert:
MatMulNBitsnodes have the explicit fourth inputGatherBlockQuantizedhas the explicit fourth input0x88Quantization types checked
Direct GGUF repackers already produce explicit zero-points for supported direct formats:
Q5_0/Q5_1/Q5_K/Q6_K are not direct
MatMulNBitsrepack targets here; when they appear in mixed presets they go through dequantize+requantize/native-block fallback paths, which produce explicit zero-points where needed.Validation
python -m pytest src\mobius\integrations\gguf\_builder_test.py -q->33 passedpython -m pytest src\mobius\integrations\gguf\ src\mobius\_configs\ src\mobius\_model_package_test.py->304 passedpython -m ruff check src\mobius\integrations\gguf\_builder.py src\mobius\integrations\gguf\_builder_test.py-> passedpython -m ruff format --check src\mobius\integrations\gguf\_builder.py src\mobius\integrations\gguf\_builder_test.py-> passedFresh conversion validated with
C:\Users\justinchu\dev\models-gguf\qwen2.5-0.5b-instruct-q4_0.gguf:Converted graph verification:
GatherBlockQuantizedinput count: 4MatMulNBitsinput counts: 4Generation checks on the freshly converted model, no post-hoc graph patching:
The capital of France is Paris. It is the largest city inONNX_GENAI_ORT_LIB_DIRpointed at the installed onnxruntime-gpu package, CUDA/cuDNN DLL dirs on PATH):The capital of France is Paris. It is the largest city in--features native-cuda, CUDA/cuDNN DLL dirs on PATH):The capital of France is Paris. It is the largest city inUpstream ORT issue
Reported the provider default divergence here: microsoft/onnxruntime#31692
Relationship to #458
#458 was accidentally based on another in-flight feature branch and includes unrelated commits. This PR is the clean-main replacement for the urgent zero-point correctness fix only.