Skip to content

Commit 4f5cbd2

Browse files
authored
Fix Gemma 4 KV-shared layers creating unused projections (#1158)
1 parent 3cd9a52 commit 4f5cbd2

2 files changed

Lines changed: 87 additions & 6 deletions

File tree

mlx_lm/models/gemma4_text.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ def __init__(self, config: ModelArgs, layer_idx: int):
180180
self.layer_idx = layer_idx
181181
self.layer_type = config.layer_types[layer_idx]
182182
self.is_sliding = self.layer_type == "sliding_attention"
183+
self.has_kv = layer_idx < config.num_hidden_layers - config.num_kv_shared_layers
183184

184185
self.head_dim = (
185186
config.global_head_dim
@@ -202,14 +203,18 @@ def __init__(self, config: ModelArgs, layer_idx: int):
202203
self.scale = 1.0
203204

204205
self.q_proj = nn.Linear(dim, self.n_heads * self.head_dim, bias=False)
205-
self.k_proj = nn.Linear(dim, self.n_kv_heads * self.head_dim, bias=False)
206-
if not self.use_k_eq_v:
207-
self.v_proj = nn.Linear(dim, self.n_kv_heads * self.head_dim, bias=False)
206+
if self.has_kv:
207+
self.k_proj = nn.Linear(dim, self.n_kv_heads * self.head_dim, bias=False)
208+
if not self.use_k_eq_v:
209+
self.v_proj = nn.Linear(
210+
dim, self.n_kv_heads * self.head_dim, bias=False
211+
)
208212
self.o_proj = nn.Linear(self.n_heads * self.head_dim, dim, bias=False)
209213

210214
self.q_norm = nn.RMSNorm(self.head_dim, eps=config.rms_norm_eps)
211-
self.k_norm = nn.RMSNorm(self.head_dim, eps=config.rms_norm_eps)
212-
self.v_norm = RMSNormNoScale(self.head_dim, eps=config.rms_norm_eps)
215+
if self.has_kv:
216+
self.k_norm = nn.RMSNorm(self.head_dim, eps=config.rms_norm_eps)
217+
self.v_norm = RMSNormNoScale(self.head_dim, eps=config.rms_norm_eps)
213218

214219
# RoPE (with partial rotation support)
215220
layer_key = "sliding_attention" if self.is_sliding else "full_attention"
@@ -238,6 +243,10 @@ def __call__(
238243

239244
if shared_kv is not None:
240245
keys, values = shared_kv
246+
elif not self.has_kv:
247+
raise ValueError(
248+
f"Layer {self.layer_idx} is a KV-shared layer but received no shared_kv"
249+
)
241250
else:
242251
keys = self.k_proj(x).reshape(B, L, self.n_kv_heads, self.head_dim)
243252
values = keys

tests/test_models.py

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import mlx.core as mx
77
import mlx.nn as nn
8-
from mlx.utils import tree_map
8+
from mlx.utils import tree_flatten, tree_map
99

1010
from mlx_lm.models import rope_utils
1111
from mlx_lm.models.base import create_causal_mask, scaled_dot_product_attention
@@ -1547,6 +1547,78 @@ def test_gemma4_quantized_embedding_preserves_lookup_scale(self):
15471547
mx.allclose(logits, mx.ones((1, 1, 4), dtype=mx.float32) * 32.0)
15481548
)
15491549

1550+
def test_gemma4_kv_shared_layers_omit_kv_projections(self):
1551+
"""KV-shared layers must not create k_proj/v_proj/k_norm/v_norm so that
1552+
models saved without redundant weights (e.g. via transformers
1553+
save_pretrained) can be loaded with strict=True."""
1554+
from mlx_lm.models import gemma4_text
1555+
1556+
args = gemma4_text.ModelArgs(
1557+
model_type="gemma4_text",
1558+
hidden_size=128,
1559+
num_hidden_layers=10,
1560+
intermediate_size=256,
1561+
num_attention_heads=4,
1562+
head_dim=32,
1563+
global_head_dim=64,
1564+
rms_norm_eps=1e-6,
1565+
vocab_size=1000,
1566+
vocab_size_per_layer_input=1000,
1567+
num_key_value_heads=1,
1568+
num_kv_shared_layers=4,
1569+
hidden_size_per_layer_input=32,
1570+
sliding_window=8,
1571+
sliding_window_pattern=5,
1572+
final_logit_softcapping=30.0,
1573+
layer_types=[
1574+
"sliding_attention",
1575+
"sliding_attention",
1576+
"sliding_attention",
1577+
"sliding_attention",
1578+
"full_attention",
1579+
"sliding_attention",
1580+
"sliding_attention",
1581+
"sliding_attention",
1582+
"sliding_attention",
1583+
"full_attention",
1584+
],
1585+
rope_parameters={
1586+
"full_attention": {
1587+
"partial_rotary_factor": 0.25,
1588+
"rope_theta": 1000000.0,
1589+
},
1590+
"sliding_attention": {
1591+
"rope_theta": 10000.0,
1592+
},
1593+
},
1594+
)
1595+
model = gemma4_text.Model(args)
1596+
1597+
# Non-shared layers (0-5) should have KV projections
1598+
for i in range(6):
1599+
attn = model.model.layers[i].self_attn
1600+
self.assertTrue(attn.has_kv)
1601+
self.assertTrue(hasattr(attn, "k_proj"))
1602+
self.assertTrue(hasattr(attn, "k_norm"))
1603+
1604+
# Shared layers (6-9) should NOT have KV projections
1605+
for i in range(6, 10):
1606+
attn = model.model.layers[i].self_attn
1607+
self.assertFalse(attn.has_kv)
1608+
self.assertFalse(hasattr(attn, "k_proj"))
1609+
self.assertFalse(hasattr(attn, "k_norm"))
1610+
self.assertFalse(hasattr(attn, "v_proj"))
1611+
1612+
# Verify the model can load weights that omit shared-layer KV params
1613+
weights = dict(tree_flatten(model.parameters()))
1614+
kv_keys = [
1615+
k for k in weights if "k_proj" in k or "v_proj" in k or "k_norm" in k
1616+
]
1617+
for k in kv_keys:
1618+
# All KV keys should belong to non-shared layers (0-5)
1619+
layer_idx = int(k.split("layers.")[1].split(".")[0])
1620+
self.assertLess(layer_idx, 6)
1621+
15501622
def test_gemma4_input_embeddings_reconstruct_per_layer_inputs(self):
15511623
from mlx_lm.models import gemma4_text
15521624

0 commit comments

Comments
 (0)