Skip to content

Commit d0145dc

Browse files
committed
feat: thread doc_ids into Engram block for cross-document masking
`HybridTinyBlock` was calling `EngramBranch(x)` without document IDs even though the underlying branch already supports `doc_ids=` for both the n-gram causal-local-average and the depthwise SiLU conv path. In packed batches that meant the n-gram aggregation silently bled across document boundaries. This change: * Adds `doc_ids: mx.array | None = None` to `HybridTinyBlock.__call__` / `route_delta` and threads it only to the engram backend (concept is order-invariant, attention has its own additive-mask channel, mamba3/moe/m2rnn do not yet support doc-aware routing in this repo). * In `HybridTinyLM.decoder_hidden_states`, passes the validated `(B, S)` int32 `document_ids` returned by `_validate_document_ids` into engram layers in both the regular and grad-checkpoint paths. * Mirrors the nanochat convention (`nanochat/unified_superblock.py: self.engram(x_norm, doc_ids=doc_ids)`) for kwarg name and shape. Tests (`tests/test_engram_doc_ids.py`): * doc_ids actually changes hidden states for `pattern="N"` (both with and without the conv kernel branch active). * `document_ids` is a no-op for engram-free, attention-free stacks. * Regression: omitting the kwarg yields byte-identical output to `doc_ids=None` and shape/dtype are unchanged. * `_validate_document_ids` still fails closed on wrong rank, wrong shape, and negative IDs. Refs: cppmega-mlx-m26 follow-up (cppmega-mlx-x0z)
1 parent 6de7e39 commit d0145dc

2 files changed

Lines changed: 240 additions & 3 deletions

File tree

cppmega_mlx/models/hybrid_lm.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -522,8 +522,18 @@ def route_delta(
522522
*,
523523
kv_cache: ContiguousKVCache | None = None,
524524
attention_layer_idx: int | None = None,
525+
doc_ids: mx.array | None = None,
525526
) -> mx.array:
526-
"""Return this route's pre-residual contribution for regression tests."""
527+
"""Return this route's pre-residual contribution for regression tests.
528+
529+
``doc_ids`` is the raw ``(B, S)`` int32 document boundary tensor. Only
530+
the ``engram`` backend currently consumes it (to prevent n-gram
531+
aggregation from crossing packed document boundaries). The
532+
``attention`` backend has its own additive-mask channel (``mask``) for
533+
the same purpose, and the remaining backends (``mamba3``, ``moe``,
534+
``m2rnn``, ``concept``) do not yet support document-aware routing in
535+
this repo, so they silently ignore ``doc_ids``.
536+
"""
527537

528538
self.validate_backend()
529539
if kv_cache is not None and self.backend != "attention":
@@ -550,7 +560,7 @@ def route_delta(
550560
else:
551561
delta, _ = m2rnn(x)
552562
elif self.backend == "engram":
553-
delta = cast(EngramBranch, self.block)(x)
563+
delta = cast(EngramBranch, self.block)(x, doc_ids=doc_ids)
554564
elif self.backend == "concept":
555565
delta = cast(ConceptBlock, self.block)(x)
556566
else: # pragma: no cover - self.backend is fixed during construction.
@@ -732,7 +742,12 @@ def decoder_hidden_states(
732742
)
733743
if self.config.grad_checkpoint:
734744
for layer in self.layers:
735-
hidden_states = mx.checkpoint(layer)(hidden_states, mask)
745+
if layer.backend == "engram" and document_ids is not None:
746+
hidden_states = mx.checkpoint(layer)(
747+
hidden_states, mask, doc_ids=document_ids
748+
)
749+
else:
750+
hidden_states = mx.checkpoint(layer)(hidden_states, mask)
736751
else:
737752
attention_layer_idx = 0
738753
for layer in self.layers:
@@ -744,6 +759,10 @@ def decoder_hidden_states(
744759
attention_layer_idx=attention_layer_idx if kv_cache is not None else None,
745760
)
746761
attention_layer_idx += 1
762+
elif layer.backend == "engram":
763+
hidden_states = layer(
764+
hidden_states, mask, doc_ids=document_ids
765+
)
747766
else:
748767
hidden_states = layer(hidden_states, mask)
749768
return self.norm(hidden_states)

tests/test_engram_doc_ids.py

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
"""Tests that ``HybridTinyBlock`` / ``HybridTinyLM`` thread ``document_ids``
2+
into the Engram (``N`` symbol) block so that n-gram aggregation respects
3+
packed-document boundaries.
4+
5+
Mirrors the kwarg convention used in nanochat
6+
(``nanochat/engram.py::EngramBranch.forward(self, x, doc_ids=None)`` and
7+
``nanochat/unified_superblock.py`` which calls ``self.engram(x_norm, doc_ids=doc_ids)``):
8+
the parent stack carries the raw ``(B, S)`` int document IDs and only the
9+
engram branch consumes them via the ``doc_ids`` kwarg.
10+
"""
11+
12+
from __future__ import annotations
13+
14+
import mlx.core as mx
15+
import pytest
16+
17+
from cppmega_mlx.models.hybrid_lm import (
18+
HybridTinyBlock,
19+
HybridTinyConfig,
20+
HybridTinyLM,
21+
)
22+
from cppmega_mlx.nn.engram import EngramBranch
23+
24+
25+
def _engram_only_config() -> HybridTinyConfig:
26+
return HybridTinyConfig(
27+
vocab_size=16,
28+
hidden_size=8,
29+
pattern="N",
30+
depth=1,
31+
num_attention_heads=2,
32+
max_seq_length=8,
33+
moe_num_experts=2,
34+
moe_top_k=1,
35+
moe_expert_hidden_size=4,
36+
moe_shared_expert_hidden_size=None,
37+
m2rnn_k_head_dim=2,
38+
m2rnn_v_head_dim=2,
39+
engram_ngram_orders=(2, 3),
40+
engram_conv_kernel=0,
41+
)
42+
43+
44+
def _make_engram_model(*, conv_kernel: int = 0) -> HybridTinyLM:
45+
"""Build an engram-only model and randomize ``out_proj`` so the branch is
46+
not a degenerate identity / all-zero map at init (the upstream
47+
``EngramBranch`` deliberately zero-inits ``out_proj`` so the residual
48+
branch is identity at step 0)."""
49+
50+
cfg = HybridTinyConfig(
51+
vocab_size=16,
52+
hidden_size=8,
53+
pattern="N",
54+
depth=1,
55+
num_attention_heads=2,
56+
max_seq_length=8,
57+
moe_num_experts=2,
58+
moe_top_k=1,
59+
moe_expert_hidden_size=4,
60+
moe_shared_expert_hidden_size=None,
61+
m2rnn_k_head_dim=2,
62+
m2rnn_v_head_dim=2,
63+
engram_ngram_orders=(2, 3),
64+
engram_conv_kernel=conv_kernel,
65+
)
66+
model = HybridTinyLM(cfg)
67+
engram = model.layers[0].block
68+
assert isinstance(engram, EngramBranch)
69+
mx.random.seed(7)
70+
engram.out_proj.weight = mx.random.normal(
71+
engram.out_proj.weight.shape, dtype=engram.out_proj.weight.dtype
72+
) * 0.3
73+
return model
74+
75+
76+
# ---------------------------------------------------------------------------
77+
# (1) doc_ids actually flow through the model into the engram block
78+
# ---------------------------------------------------------------------------
79+
80+
81+
def test_engram_doc_ids_change_hidden_states():
82+
model = _make_engram_model()
83+
mx.random.seed(0)
84+
input_ids = mx.random.randint(0, 16, shape=(1, 6))
85+
document_ids = mx.array([[0, 0, 0, 1, 1, 1]], dtype=mx.int32)
86+
87+
out_no_docs = model.decoder_hidden_states(input_ids)
88+
out_with_docs = model.decoder_hidden_states(input_ids, document_ids=document_ids)
89+
mx.eval(out_no_docs, out_with_docs)
90+
91+
# The two outputs must differ — the engram n-gram averages span the
92+
# 0->1 doc boundary in the unmasked case and get clipped in the masked
93+
# case. We check absolute diff at the positions that the order=2/3
94+
# average windows touch around the boundary (positions 3 and 4).
95+
diff = mx.max(mx.abs(out_no_docs - out_with_docs), axis=-1)
96+
mx.eval(diff)
97+
assert float(diff[0, 3].item()) > 1e-4, (
98+
"engram delta at position 3 should change when its order>=2 average "
99+
"stops pulling in the prior document's tokens"
100+
)
101+
102+
103+
def test_engram_doc_ids_flow_through_conv_path():
104+
"""Same as above but with the depthwise causal conv branch active so the
105+
other doc_ids-aware code path inside ``EngramBranch`` is exercised too."""
106+
107+
model = _make_engram_model(conv_kernel=3)
108+
mx.random.seed(1)
109+
input_ids = mx.random.randint(0, 16, shape=(1, 6))
110+
document_ids = mx.array([[0, 0, 0, 1, 1, 1]], dtype=mx.int32)
111+
112+
out_no_docs = model.decoder_hidden_states(input_ids)
113+
out_with_docs = model.decoder_hidden_states(input_ids, document_ids=document_ids)
114+
mx.eval(out_no_docs, out_with_docs)
115+
116+
diff = mx.max(mx.abs(out_no_docs - out_with_docs))
117+
mx.eval(diff)
118+
assert float(diff.item()) > 1e-4
119+
120+
121+
# ---------------------------------------------------------------------------
122+
# (2) No engram in the stack -> document_ids has no effect on non-attention
123+
# hidden states (we use a pattern that contains no attention layer so
124+
# the attention additive-mask path is not exercised either).
125+
# ---------------------------------------------------------------------------
126+
127+
128+
def test_document_ids_noop_when_no_engram_no_attention():
129+
cfg = HybridTinyConfig(
130+
vocab_size=16,
131+
hidden_size=8,
132+
pattern="C",
133+
depth=1,
134+
num_attention_heads=2,
135+
max_seq_length=8,
136+
moe_num_experts=2,
137+
moe_top_k=1,
138+
moe_expert_hidden_size=4,
139+
moe_shared_expert_hidden_size=None,
140+
m2rnn_k_head_dim=2,
141+
m2rnn_v_head_dim=2,
142+
concept_num_concepts=4,
143+
concept_num_heads=2,
144+
)
145+
model = HybridTinyLM(cfg)
146+
assert tuple(layer.backend for layer in model.layers) == ("concept",)
147+
148+
mx.random.seed(2)
149+
input_ids = mx.random.randint(0, 16, shape=(1, 6))
150+
document_ids = mx.array([[0, 0, 0, 1, 1, 1]], dtype=mx.int32)
151+
152+
out_no_docs = model.decoder_hidden_states(input_ids)
153+
out_with_docs = model.decoder_hidden_states(input_ids, document_ids=document_ids)
154+
mx.eval(out_no_docs, out_with_docs)
155+
156+
assert mx.allclose(out_no_docs, out_with_docs, atol=0.0, rtol=0.0).item()
157+
158+
159+
# ---------------------------------------------------------------------------
160+
# (3) Regression: calling the block / model without doc_ids still produces
161+
# the same shape and dtype it always did.
162+
# ---------------------------------------------------------------------------
163+
164+
165+
def test_engram_block_without_doc_ids_unchanged_shape_dtype():
166+
cfg = _engram_only_config()
167+
layer = cfg.expanded_pattern().layers[0]
168+
block = HybridTinyBlock(layer, cfg)
169+
assert block.backend == "engram"
170+
171+
x = mx.random.normal((2, 6, cfg.hidden_size))
172+
delta_no_docs = block.route_delta(x, mask=None)
173+
delta_kwarg_none = block.route_delta(x, mask=None, doc_ids=None)
174+
mx.eval(delta_no_docs, delta_kwarg_none)
175+
176+
assert delta_no_docs.shape == x.shape
177+
assert delta_no_docs.dtype == x.dtype
178+
# Explicit doc_ids=None must be byte-identical to omitting the kwarg.
179+
assert mx.array_equal(delta_no_docs, delta_kwarg_none).item()
180+
181+
182+
def test_hybrid_lm_forward_without_document_ids_runs():
183+
model = _make_engram_model()
184+
input_ids = mx.array([[1, 2, 3, 4, 5, 6]], dtype=mx.int32)
185+
out = model(input_ids)
186+
mx.eval(out)
187+
assert out.shape == (1, 6, model.config.vocab_size)
188+
189+
190+
# ---------------------------------------------------------------------------
191+
# (4) Wrong-shape document_ids fails closed via _validate_document_ids
192+
# ---------------------------------------------------------------------------
193+
194+
195+
def test_document_ids_wrong_shape_raises():
196+
model = _make_engram_model()
197+
input_ids = mx.array([[1, 2, 3, 4, 5, 6]], dtype=mx.int32)
198+
# Mismatched seq length: input is (1, 6), document_ids is (1, 5)
199+
bad_doc_ids = mx.array([[0, 0, 0, 1, 1]], dtype=mx.int32)
200+
with pytest.raises(ValueError, match="document_ids shape"):
201+
model.decoder_hidden_states(input_ids, document_ids=bad_doc_ids)
202+
203+
204+
def test_document_ids_wrong_rank_raises():
205+
model = _make_engram_model()
206+
input_ids = mx.array([[1, 2, 3, 4, 5, 6]], dtype=mx.int32)
207+
# Rank 1 instead of (B, S)
208+
bad_doc_ids = mx.array([0, 0, 0, 1, 1, 1], dtype=mx.int32)
209+
with pytest.raises(ValueError, match="document_ids must be shaped"):
210+
model.decoder_hidden_states(input_ids, document_ids=bad_doc_ids)
211+
212+
213+
def test_document_ids_negative_raises():
214+
model = _make_engram_model()
215+
input_ids = mx.array([[1, 2, 3, 4, 5, 6]], dtype=mx.int32)
216+
bad_doc_ids = mx.array([[0, 0, -1, 1, 1, 1]], dtype=mx.int32)
217+
with pytest.raises(ValueError, match="non-negative"):
218+
model.decoder_hidden_states(input_ids, document_ids=bad_doc_ids)

0 commit comments

Comments
 (0)