Skip to content

Commit 83e6e5c

Browse files
authored
Fix: packed matmul disabled when q_norm/k_norm is set (Gemma-3 garbage output)
Restores the `not q_norm` / `not k_norm` guards for `use_packed_matmul` in `base.py make_attention_init` that were dropped during the v0.9.0→branch refactor. Without these guards, Gemma-3 (q_norm=True, k_norm=True) had packed matmul enabled, causing `make_qk_norm` to apply layer-norm to the full packed-QKV tensor (instead of Q alone) and to use an empty string as the k_path input — producing a malformed ONNX graph and garbage output. Added `TestPackedMatmulQKNorm` regression tests to prevent recurrence.
1 parent a8927d4 commit 83e6e5c

2 files changed

Lines changed: 89 additions & 0 deletions

File tree

src/python/py/models/builders/base.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,10 +474,14 @@ def make_attention_init(self, config):
474474
# Packed MatMul with LoRA/QLoRA is not currently supported
475475
# use_packed_matmul can be overrided by upstream quantization choice
476476
# (e.g., when q_proj, k_proj, v_proj have different quantization settings)
477+
# Packed MatMul is also not supported when QK normalization is used,
478+
# because q_norm/k_norm must be applied to the individual Q and K tensors
477479
self.attention_attrs["use_packed_matmul"] = (
478480
self.ep not in ["dml"]
479481
and not self.matmul_attrs["use_lora"]
480482
and not self.extra_options.get("disable_qkv_fusion", False)
483+
and not self.attention_attrs["q_norm"]
484+
and not self.attention_attrs["k_norm"]
481485
)
482486

483487
# Some EPs don't support fusing rotary embeddings inside GQA yet

test/python/builder/test_gemma.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,91 @@ def test_previous_formula_was_inverted(self):
217217
)
218218

219219

220+
# ---------------------------------------------------------------------------
221+
# use_packed_matmul + q_norm/k_norm regression tests
222+
#
223+
# Root cause: make_attention_init() in base.py computed use_packed_matmul
224+
# without checking q_norm / k_norm. For Gemma-3 (q_norm=True, k_norm=True)
225+
# this caused packed matmul to be enabled. make_qk_norm then operated on the
226+
# packed QKV tensor (instead of the individual Q tensor) and used "" (empty
227+
# string) as the k_path input — producing a malformed graph and garbage output.
228+
#
229+
# Fix: use_packed_matmul is now False whenever q_norm or k_norm is True,
230+
# matching v0.9.0 behaviour.
231+
# ---------------------------------------------------------------------------
232+
233+
234+
def _make_attention_init_state(ep: str, q_norm: bool, k_norm: bool):
235+
"""Build a minimal stub that can exercise base.Model.make_attention_init."""
236+
import types
237+
238+
base = _load_builder_module("base")
239+
model = base.Model.__new__(base.Model)
240+
241+
model.ep = ep
242+
model.io_dtype = base.ir.DataType.FLOAT
243+
model.extra_options = {}
244+
model.matmul_attrs = {"use_lora": False}
245+
model.input_names = {"position_ids": "position_ids"}
246+
model.attention_attrs = {
247+
"q_norm": q_norm,
248+
"k_norm": k_norm,
249+
}
250+
model.num_attn_heads = 8
251+
model.num_kv_heads = 4
252+
model.head_size = 256
253+
return model
254+
255+
256+
class TestPackedMatmulQKNorm:
257+
"""use_packed_matmul must be False when q_norm or k_norm is enabled."""
258+
259+
def test_gemma3_cpu_fp32_disables_packed_matmul(self):
260+
"""Gemma-3 (q_norm=k_norm=True) must NOT use packed matmul (CPU fp32 GQA)."""
261+
model = _make_attention_init_state("cpu", q_norm=True, k_norm=True)
262+
base_module = _load_builder_module("base")
263+
base_module.Model.make_attention_init(model, config=None)
264+
assert not model.attention_attrs["use_packed_matmul"], (
265+
"use_packed_matmul must be False when q_norm=True to avoid applying "
266+
"qk-norm to the packed QKV tensor instead of the individual Q/K tensors"
267+
)
268+
269+
def test_gemma2_cpu_fp32_uses_packed_matmul(self):
270+
"""Gemma-2 (q_norm=k_norm=False) SHOULD use packed matmul (CPU fp32 GQA)."""
271+
model = _make_attention_init_state("cpu", q_norm=False, k_norm=False)
272+
base_module = _load_builder_module("base")
273+
base_module.Model.make_attention_init(model, config=None)
274+
assert model.attention_attrs["use_packed_matmul"], (
275+
"use_packed_matmul must be True for models without q_norm/k_norm"
276+
)
277+
278+
def test_q_norm_alone_disables_packed_matmul(self):
279+
"""q_norm=True alone is sufficient to disable packed matmul."""
280+
model = _make_attention_init_state("cpu", q_norm=True, k_norm=False)
281+
base_module = _load_builder_module("base")
282+
base_module.Model.make_attention_init(model, config=None)
283+
assert not model.attention_attrs["use_packed_matmul"]
284+
285+
def test_k_norm_alone_disables_packed_matmul(self):
286+
"""k_norm=True alone is sufficient to disable packed matmul."""
287+
model = _make_attention_init_state("cpu", q_norm=False, k_norm=True)
288+
base_module = _load_builder_module("base")
289+
base_module.Model.make_attention_init(model, config=None)
290+
assert not model.attention_attrs["use_packed_matmul"]
291+
292+
def test_previous_code_incorrectly_enabled_packed_matmul_for_gemma3(self):
293+
"""Demonstrates that the old code always set use_packed_matmul=True on CPU fp32,
294+
regardless of q_norm/k_norm — which caused garbage output for Gemma-3."""
295+
# Old formula (pre-fix): only checks ep and lora/disable flags
296+
old_use_packed_matmul = lambda ep, q_norm, k_norm: ( # noqa: E731
297+
ep not in ["dml"]
298+
# missing: and not q_norm and not k_norm
299+
)
300+
assert old_use_packed_matmul("cpu", q_norm=True, k_norm=True) is True, (
301+
"Old formula enabled packed matmul even with q_norm=True — this is the bug"
302+
)
303+
304+
220305
class TestGemma3IsLocal:
221306
def test_reads_layer_types_from_config(self):
222307
"""Gemma3 is_local uses layer_types when available."""

0 commit comments

Comments
 (0)