Skip to content

Commit 9567144

Browse files
justinchubyCopilotCopilot
authored
Emit GGUF zero points explicitly (#459)
## 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: - Q4_0: `(q - 8) * scale` - Q8_0: `(q - 128) * scale` Mobius previously marked those GGUF imports as symmetric, so `QuantizedLinear` and `QuantizedEmbedding` omitted the optional `zero_points` input for `MatMulNBits` and `GatherBlockQuantized`. That is not portable: `GatherBlockQuantized` defaults diverge between ORT CPU and ORT CUDA when `zero_points` is 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 `GatherBlockQuantized` and `MatMulNBits` receive the intended values instead of relying on EP defaults. ## Regression test Added a single-node `GatherBlockQuantized` runtime regression: - qweight nibbles are all `10` - scales are known constants - explicit zero point is `8` - output is asserted against hand-computed `(q - 8) * scale` - rerunning with zero point `0` is asserted not to match I 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: - Q4_0 `MatMulNBits` nodes have the explicit fourth input - Q4_0 `GatherBlockQuantized` has the explicit fourth input - Q4_0 embedding zero points are packed as `0x88` ## Quantization types checked Direct GGUF repackers already produce explicit zero-points for supported direct formats: - Q4_0: zp=8 - Q4_1: per-block affine zp - Q4_K: requantized per-block affine zp - Q8_0: zp=128 - Q1_0: zp=1; Tencent Q1_0 uses its custom zp path Q5_0/Q5_1/Q5_K/Q6_K are not direct `MatMulNBits` repack 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 passed` - `python -m pytest src\mobius\integrations\gguf\ src\mobius\_configs\ src\mobius\_model_package_test.py` -> `304 passed` - `python -m ruff check src\mobius\integrations\gguf\_builder.py src\mobius\integrations\gguf\_builder_test.py` -> passed - `python -m ruff format --check src\mobius\integrations\gguf\_builder.py src\mobius\integrations\gguf\_builder_test.py` -> passed Fresh conversion validated with `C:\Users\justinchu\dev\models-gguf\qwen2.5-0.5b-instruct-q4_0.gguf`: ```powershell python -m mobius build-gguf C:\Users\justinchu\dev\models-gguf\qwen2.5-0.5b-instruct-q4_0.gguf --output C:\Users\justinchu\dev\models\qwen2.5-0.5b-q4_0-mobius --keep-quantized --dtype f16 --ep cuda ``` Converted graph verification: - `GatherBlockQuantized` input count: 4 - all `MatMulNBits` input counts: 4 - zero-point initializers: 170 Generation checks on the freshly converted model, no post-hoc graph patching: - ORT CPU via onnx-genai CLI: `The capital of France is Paris. It is the largest city in` - ORT CUDA via onnx-genai CLI (`ONNX_GENAI_ORT_LIB_DIR` pointed at the installed onnxruntime-gpu package, CUDA/cuDNN DLL dirs on PATH): `The capital of France is Paris. It is the largest city in` - native CUDA via onnx-genai CLI (`--features native-cuda`, CUDA/cuDNN DLL dirs on PATH): `The capital of France is Paris. It is the largest city in` ## Upstream 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. --------- Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
1 parent 1d2a28b commit 9567144

2 files changed

Lines changed: 122 additions & 10 deletions

File tree

src/mobius/integrations/gguf/_builder.py

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -528,19 +528,25 @@ def _detect_quant_params(gguf_model, gguf_arch: str) -> tuple[int, int, bool]:
528528
map_gguf_to_hf_names,
529529
)
530530

531-
# Symmetry of each supported GGUF quantization type.
531+
# Whether the graph can omit zero_points for each supported GGUF type.
532532
#
533533
# Mainline Q1_0 (1-bit binary) is repacked into 2-bit MatMulNBits
534534
# with zp=1 — see _repack_q1_0. Tencent's custom Q1_0 (2-bit SEQ,
535535
# 512-elt blocks) is inflated to 4-bit MatMulNBits with zp=3 — see
536536
# parse_tencent_q1_0_tensor — because the ORT CPU unpacked-float-zp
537537
# path is currently only implemented for bits=4, and the half-integer
538538
# SEQ offset 1.5 cannot be expressed with integer zp at bits=2.
539-
type_symmetry: dict = {
540-
GGMLQuantizationType.Q4_0: True,
539+
#
540+
# Q4_0 and Q8_0 are symmetric formats, but their GGUF dequantization
541+
# formulas are still ``(q - 8) * scale`` and ``(q - 128) * scale``.
542+
# Emit those zero_points explicitly: GatherBlockQuantized has diverging
543+
# CPU/CUDA defaults when the input is omitted, which corrupts embeddings
544+
# on CUDA before the first decoder layer runs.
545+
type_can_omit_zero_points: dict = {
546+
GGMLQuantizationType.Q4_0: False,
541547
GGMLQuantizationType.Q4_1: False,
542548
GGMLQuantizationType.Q4_K: False,
543-
GGMLQuantizationType.Q8_0: True,
549+
GGMLQuantizationType.Q8_0: False,
544550
GGMLQuantizationType.Q1_0: False,
545551
}
546552

@@ -561,17 +567,26 @@ def _detect_quant_params(gguf_model, gguf_arch: str) -> tuple[int, int, bool]:
561567
{qtype: count for qtype, count in counts.items() if _native_block_format(qtype)}
562568
)
563569
if native_counts:
564-
asymmetric_types = {"Q2_K", "Q4_1", "Q4_K", "Q5_1", "Q5_K"}
565-
is_sym = not any(
566-
getattr(qtype, "name", None) in asymmetric_types
570+
explicit_zero_point_types = {
571+
"Q1_0",
572+
"Q2_K",
573+
"Q4_0",
574+
"Q4_1",
575+
"Q4_K",
576+
"Q5_1",
577+
"Q5_K",
578+
"Q8_0",
579+
}
580+
can_omit_zero_points = not any(
581+
getattr(qtype, "name", None) in explicit_zero_point_types
567582
for qtype in counts
568583
if qtype not in native_counts
569584
)
570585
logger.info(
571586
"Native GGUF quant types present; using 4-bit/block-32 module "
572587
"scaffolding for non-native quantized tensors",
573588
)
574-
return 4, 32, is_sym
589+
return 4, 32, can_omit_zero_points
575590

576591
# Q4_K_M is deliberately a mixed preset. Depending on tensor dimensions
577592
# and importance it may contain mostly Q5_0 plus Q4_K, Q6_K, and Q8_0.
@@ -598,7 +613,7 @@ def _detect_quant_params(gguf_model, gguf_arch: str) -> tuple[int, int, bool]:
598613
params = repack_quant_params(dominant_value)
599614
assert params is not None
600615
bits, block_size = params
601-
is_sym = type_symmetry[dominant]
616+
is_sym = type_can_omit_zero_points[dominant]
602617

603618
# Tencent Q1_0 files reuse the Q1_0 type id but ship a different
604619
# on-disk layout (2-bit SEQ, 512-element blocks, fp16 scale per block).

src/mobius/integrations/gguf/_builder_test.py

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,72 @@
88
from pathlib import Path
99

1010
import numpy as np
11+
import onnx_ir as ir
12+
import onnxruntime as ort
1113
import pytest
1214

1315

16+
def _run_gather_block_quantized(
17+
tmp_path: Path,
18+
*,
19+
zero_point: int,
20+
) -> np.ndarray:
21+
"""Run a tiny GatherBlockQuantized graph with a controlled zero point."""
22+
qweight = np.full((2, 16), 0xAA, dtype=np.uint8)
23+
scales = np.array([[0.5], [0.25]], dtype=np.float16)
24+
zero_points = np.full((2, 1), zero_point, dtype=np.uint8)
25+
26+
def _const(name: str, arr: np.ndarray) -> ir.Value:
27+
value = ir.Value(name=name)
28+
tensor = ir.tensor(arr)
29+
value.const_value = tensor
30+
value.shape = ir.Shape(arr.shape)
31+
value.dtype = tensor.dtype
32+
return value
33+
34+
input_ids = ir.Value(
35+
name="input_ids",
36+
shape=ir.Shape([2]),
37+
type=ir.TensorType(ir.DataType.INT64),
38+
)
39+
output = ir.Value(
40+
name="output",
41+
shape=ir.Shape([2, 32]),
42+
type=ir.TensorType(ir.DataType.FLOAT16),
43+
)
44+
qweight_init = _const("qweight", qweight)
45+
scales_init = _const("scales", scales)
46+
zero_points_init = _const("zero_points", zero_points)
47+
node = ir.Node(
48+
"com.microsoft",
49+
"GatherBlockQuantized",
50+
inputs=[
51+
qweight_init,
52+
input_ids,
53+
scales_init,
54+
zero_points_init,
55+
],
56+
outputs=[output],
57+
attributes=ir.convenience.convert_attributes(
58+
{"bits": 4, "block_size": 32, "gather_axis": 0, "quantize_axis": 1}
59+
),
60+
)
61+
graph = ir.Graph(
62+
inputs=[input_ids],
63+
outputs=[output],
64+
nodes=[node],
65+
initializers=[qweight_init, scales_init, zero_points_init],
66+
opset_imports={"": 18, "com.microsoft": 1},
67+
name="gbq_zero_point",
68+
)
69+
model = ir.Model(graph, ir_version=10)
70+
path = tmp_path / f"gbq_zp_{zero_point}.onnx"
71+
ir.save(model, path)
72+
session = ort.InferenceSession(str(path), providers=["CPUExecutionProvider"])
73+
(result,) = session.run(None, {"input_ids": np.array([0, 1], dtype=np.int64)})
74+
return result
75+
76+
1477
def _write_quantized_gguf(
1578
path: Path,
1679
*,
@@ -311,6 +374,20 @@ def test_model_has_matmulnbits_ops(self, q4_0_gguf: Path):
311374
f"Expected MatMulNBits in ops, got: {sorted(op_types)}"
312375
)
313376

377+
def test_q4_0_matmulnbits_has_explicit_zero_points(self, q4_0_gguf: Path):
378+
"""GGUF Q4_0 projections explicitly encode zp=8 instead of EP defaults."""
379+
from mobius.integrations.gguf import build_from_gguf
380+
381+
model = build_from_gguf(q4_0_gguf, keep_quantized=True)["model"]
382+
nodes = [node for node in model.graph if node.op_type == "MatMulNBits"]
383+
assert nodes
384+
for node in nodes:
385+
assert len(node.inputs) == 4
386+
zero_point_name = node.inputs[3].name
387+
assert zero_point_name.endswith(".zero_points")
388+
zero_points = model.graph.initializers[zero_point_name]
389+
np.testing.assert_array_equal(zero_points.const_value.numpy(), 0x88)
390+
314391
def test_native_blocks_emit_block_quantized_matmul_and_preserve_bytes(
315392
self,
316393
native_block_gguf: tuple[Path, str, int, int],
@@ -380,6 +457,7 @@ def test_quantized_embedding_uses_gatherblockquantized(self, q4_0_embedding_gguf
380457
gather_nodes = [node for node in model.graph if node.op_type == "GatherBlockQuantized"]
381458
assert len(gather_nodes) == 1
382459
assert gather_nodes[0].domain == "com.microsoft"
460+
assert len(gather_nodes[0].inputs) == 4
383461

384462
qweight = model.graph.initializers["model.embed_tokens.qweight"]
385463
assert qweight.dtype == ir.DataType.UINT8
@@ -388,8 +466,26 @@ def test_quantized_embedding_uses_gatherblockquantized(self, q4_0_embedding_gguf
388466
256,
389467
2,
390468
]
469+
zero_points = model.graph.initializers["model.embed_tokens.zero_points"]
470+
assert zero_points.dtype == ir.DataType.UINT8
471+
assert list(zero_points.shape) == [256, 1]
472+
np.testing.assert_array_equal(zero_points.const_value.numpy(), 0x88)
391473
assert "model.embed_tokens.weight" not in model.graph.initializers
392474

475+
def test_gatherblockquantized_zero_point_dequantizes_q4_0(self, tmp_path: Path):
476+
"""GatherBlockQuantized output must match GGUF Q4_0's ``(q - 8) * scale``."""
477+
actual = _run_gather_block_quantized(tmp_path, zero_point=0x08).astype(np.float32)
478+
expected = np.stack(
479+
[
480+
np.full(32, (10 - 8) * 0.5, dtype=np.float32),
481+
np.full(32, (10 - 8) * 0.25, dtype=np.float32),
482+
]
483+
)
484+
np.testing.assert_allclose(actual, expected)
485+
486+
wrong = _run_gather_block_quantized(tmp_path, zero_point=0x00).astype(np.float32)
487+
assert not np.allclose(wrong, expected)
488+
393489
def test_tied_quantized_embedding_drives_matmulnbits_head(
394490
self, q4_0_tied_embedding_gguf: Path
395491
):
@@ -401,6 +497,7 @@ def test_tied_quantized_embedding_drives_matmulnbits_head(
401497
assert op_types.count("GatherBlockQuantized") == 1
402498
assert "MatMulNBits" in op_types
403499
assert "model.embed_tokens.qweight" in model.graph.initializers
500+
assert "model.embed_tokens.zero_points" in model.graph.initializers
404501
assert not any(name.startswith("lm_head.") for name in model.graph.initializers)
405502

406503
def test_untied_quantized_head_uses_q4_matmulnbits(
@@ -479,7 +576,7 @@ def test_detect_quant_params(self, q4_0_gguf: Path):
479576
bits, block_size, is_sym = _detect_quant_params(gguf_model, gguf_model.architecture)
480577
assert bits == 4
481578
assert block_size == 32
482-
assert is_sym is True
579+
assert is_sym is False
483580

484581
def test_embedding_quantization_check_is_metadata_only(self, monkeypatch):
485582
"""Embedding compatibility does not read or repack tensor data."""

0 commit comments

Comments
 (0)