Skip to content

Commit eb823d1

Browse files
committed
feat(e7-18): thread pre_norm/post_norm through _build_attention + _build_mlp
Completes Spec-v2 §3.5 requirement that E7-6 left half-done. E7-6 (c1bc6f7) shipped norm_validation rules and the BrickContextPanel UI dropdown, but the builders themselves hardcoded RMSNorm — a user picking 'layernorm' or 'none' in the UI had zero effect on the model. cppmega_v4/models/unified_superblock_v4.py: - New _make_norm(kind, dim, eps) helper returns nn.RMSNorm / nn.LayerNorm / None; raises on unknown kind. - _build_attention now reads params.pre_norm (default 'none' for legacy compat) and params.post_norm (default 'rmsnorm' for legacy compat); applies pre_norm to input before Q/K/V proj, post_norm to output projection result. self.norm alias preserved so existing checkpoint state-dict keys still load. - _build_mlp gains the same pre_norm/post_norm wiring (defaults 'none' / 'none' since the legacy MLP had no internal norm wrapper; callers typically wrap at the block level). Tests (+15 pytest): - _make_norm returns the right class for each kind, raises on unknown - attention with 5 norm combinations (legacy default + 4 mixes) produces finite forward output - mlp with 3 norm combos (none+none / rmsnorm+layernorm / layernorm+rmsnorm) produces finite output - attention default == legacy (pre=None, post=RMSNorm) - attention post_norm='layernorm' instantiates nn.LayerNorm - mlp default has no pre/post norms (legacy) Regression: pytest 2321 (was 2306; +15) / 2 skip / 15 xfail / 0 fail (excl. pre-existing path_d). Closes cppmega-mlx-bb0.18.
1 parent 3e82818 commit eb823d1

2 files changed

Lines changed: 141 additions & 11 deletions

File tree

cppmega_v4/models/unified_superblock_v4.py

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,25 +74,36 @@ def _build_mlp(hidden_size: int, params: dict) -> nn.Module:
7474

7575
intermediate = params.get("intermediate_size", 4 * hidden_size)
7676
activation = params.get("activation", "glu")
77+
norm_eps = params.get("norm_eps", 1e-6)
78+
pre_norm_kind = params.get("pre_norm", "none")
79+
post_norm_kind = params.get("post_norm", "none")
7780

7881
class _MLP(nn.Module):
7982
def __init__(self):
8083
super().__init__()
8184
self.gate = nn.Linear(hidden_size, intermediate, bias=False)
8285
self.up = nn.Linear(hidden_size, intermediate, bias=False)
8386
self.down = nn.Linear(intermediate, hidden_size, bias=False)
87+
self.pre_norm = _make_norm(pre_norm_kind, hidden_size, norm_eps)
88+
self.post_norm = _make_norm(post_norm_kind, hidden_size, norm_eps)
8489

8590
def __call__(self, x):
91+
if self.pre_norm is not None:
92+
x = self.pre_norm(x)
8693
up = self.up(x)
8794
if activation == "glu":
88-
return self.down(mx.sigmoid(self.gate(x)) * up)
89-
if activation in IS_GATED and IS_GATED[activation]:
90-
return self.down(apply_activation(activation, up,
91-
gate=self.gate(x)))
92-
if activation in IS_GATED:
93-
return self.down(apply_activation(activation, up))
94-
# Unknown name — fall back to GLU to keep training alive.
95-
return self.down(mx.sigmoid(self.gate(x)) * up)
95+
y = self.down(mx.sigmoid(self.gate(x)) * up)
96+
elif activation in IS_GATED and IS_GATED[activation]:
97+
y = self.down(apply_activation(activation, up,
98+
gate=self.gate(x)))
99+
elif activation in IS_GATED:
100+
y = self.down(apply_activation(activation, up))
101+
else:
102+
# Unknown name — fall back to GLU to keep training alive.
103+
y = self.down(mx.sigmoid(self.gate(x)) * up)
104+
if self.post_norm is not None:
105+
y = self.post_norm(y)
106+
return y
96107
return _MLP()
97108

98109

@@ -363,11 +374,30 @@ def _build_mamba3(hidden_size: int, params: dict) -> nn.Module:
363374
return Mamba3ReferenceBlock(Mamba3Config(**cfg_kwargs))
364375

365376

377+
def _make_norm(kind: str, dim: int, eps: float):
378+
"""E7-6/E7-18: return RMSNorm / LayerNorm / None per kind."""
379+
if kind == "rmsnorm":
380+
return nn.RMSNorm(dim, eps=eps)
381+
if kind == "layernorm":
382+
return nn.LayerNorm(dim, eps=eps)
383+
if kind == "none":
384+
return None
385+
raise ValueError(f"unknown norm kind {kind!r}; "
386+
"use 'rmsnorm' / 'layernorm' / 'none'")
387+
388+
366389
def _build_attention(hidden_size: int, params: dict) -> nn.Module:
367-
"""Standard multi-head self-attention (causal). Used for `attention`."""
390+
"""Standard multi-head self-attention (causal). Used for `attention`.
391+
392+
Honors params.pre_norm (default 'none', matches legacy behavior)
393+
and params.post_norm (default 'rmsnorm', also matches legacy).
394+
See cppmega_v4/buildspec/norm_validation.py for diagnostics.
395+
"""
368396
num_heads = params.get("num_heads", max(1, hidden_size // 64))
369397
head_dim = params.get("head_dim", hidden_size // num_heads)
370398
norm_eps = params.get("norm_eps", 1e-6)
399+
pre_norm_kind = params.get("pre_norm", "none")
400+
post_norm_kind = params.get("post_norm", "rmsnorm")
371401

372402
class _SelfAttn(nn.Module):
373403
def __init__(self):
@@ -376,11 +406,16 @@ def __init__(self):
376406
self.k_proj = nn.Linear(hidden_size, num_heads * head_dim, bias=False)
377407
self.v_proj = nn.Linear(hidden_size, num_heads * head_dim, bias=False)
378408
self.o_proj = nn.Linear(num_heads * head_dim, hidden_size, bias=False)
379-
self.norm = nn.RMSNorm(hidden_size, eps=norm_eps)
409+
self.pre_norm = _make_norm(pre_norm_kind, hidden_size, norm_eps)
410+
# 'norm' alias preserves state-dict keys from earlier
411+
# checkpoints where only one (post) norm existed.
412+
self.norm = _make_norm(post_norm_kind, hidden_size, norm_eps)
380413
# Zero-init out so the block is identity at init.
381414
self.o_proj.weight = mx.zeros_like(self.o_proj.weight)
382415

383416
def __call__(self, x):
417+
if self.pre_norm is not None:
418+
x = self.pre_norm(x)
384419
B, S, _ = x.shape
385420
q = self.q_proj(x).reshape(B, S, num_heads, head_dim)
386421
k = self.k_proj(x).reshape(B, S, num_heads, head_dim)
@@ -396,7 +431,10 @@ def __call__(self, x):
396431
w = mx.softmax(scores.astype(mx.float32), axis=-1).astype(scores.dtype)
397432
o = mx.matmul(w, v)
398433
o = mx.transpose(o, (0, 2, 1, 3)).reshape(B, S, num_heads * head_dim)
399-
return self.norm(self.o_proj(o))
434+
o = self.o_proj(o)
435+
if self.norm is not None:
436+
o = self.norm(o)
437+
return o
400438

401439
return _SelfAttn()
402440

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
"""E7-6-builder tests: pre_norm/post_norm params actually thread into
2+
_build_attention and _build_mlp (Spec-v2 §3.5)."""
3+
4+
from __future__ import annotations
5+
6+
import mlx.core as mx
7+
import mlx.nn as nn
8+
import pytest
9+
10+
from cppmega_v4.models.unified_superblock_v4 import (
11+
BLOCK_BUILDERS, _make_norm,
12+
)
13+
14+
15+
HIDDEN = 32
16+
17+
18+
def test_make_norm_returns_rmsnorm():
19+
n = _make_norm("rmsnorm", HIDDEN, 1e-6)
20+
assert isinstance(n, nn.RMSNorm)
21+
22+
23+
def test_make_norm_returns_layernorm():
24+
n = _make_norm("layernorm", HIDDEN, 1e-5)
25+
assert isinstance(n, nn.LayerNorm)
26+
27+
28+
def test_make_norm_returns_none_for_none():
29+
assert _make_norm("none", HIDDEN, 1e-6) is None
30+
31+
32+
def test_make_norm_rejects_unknown_kind():
33+
with pytest.raises(ValueError, match="unknown norm kind"):
34+
_make_norm("batchnorm", HIDDEN, 1e-6)
35+
36+
37+
@pytest.mark.parametrize("pre,post", [
38+
("none", "rmsnorm"), # legacy default
39+
("rmsnorm", "none"),
40+
("layernorm", "rmsnorm"),
41+
("rmsnorm", "rmsnorm"),
42+
("layernorm", "layernorm"),
43+
])
44+
def test_attention_with_norm_combos_forward(pre, post):
45+
"""Each valid norm combination produces finite forward output."""
46+
mlp = BLOCK_BUILDERS["attention"](HIDDEN, {
47+
"pre_norm": pre, "post_norm": post,
48+
})
49+
x = mx.random.normal((1, 4, HIDDEN), key=mx.random.key(0))
50+
y = mlp(x)
51+
assert y.shape == (1, 4, HIDDEN)
52+
assert mx.isfinite(y).all().item()
53+
54+
55+
@pytest.mark.parametrize("pre,post", [
56+
("none", "none"),
57+
("rmsnorm", "layernorm"),
58+
("layernorm", "rmsnorm"),
59+
])
60+
def test_mlp_with_norm_combos_forward(pre, post):
61+
"""MLP same — pre/post wrap the activation core."""
62+
mlp = BLOCK_BUILDERS["mlp"](HIDDEN, {
63+
"intermediate_size": 64,
64+
"pre_norm": pre, "post_norm": post,
65+
"activation": "swiglu",
66+
})
67+
x = mx.random.normal((1, 4, HIDDEN), key=mx.random.key(0))
68+
y = mlp(x)
69+
assert y.shape == (1, 4, HIDDEN)
70+
assert mx.isfinite(y).all().item()
71+
72+
73+
def test_attention_default_matches_legacy_only_post_norm():
74+
"""Without pre_norm/post_norm params, behavior matches legacy
75+
(single RMSNorm at output, no pre-norm)."""
76+
mlp = BLOCK_BUILDERS["attention"](HIDDEN, {})
77+
assert mlp.pre_norm is None
78+
assert isinstance(mlp.norm, nn.RMSNorm)
79+
80+
81+
def test_attention_layernorm_post_switch_instantiated():
82+
mlp = BLOCK_BUILDERS["attention"](HIDDEN, {
83+
"post_norm": "layernorm",
84+
})
85+
assert isinstance(mlp.norm, nn.LayerNorm)
86+
87+
88+
def test_mlp_default_no_norms_legacy_behavior():
89+
"""MLP legacy default: no pre/post norm wrappers (caller wraps)."""
90+
mlp = BLOCK_BUILDERS["mlp"](HIDDEN, {})
91+
assert mlp.pre_norm is None
92+
assert mlp.post_norm is None

0 commit comments

Comments
 (0)