Skip to content

Commit 2c6802e

Browse files
Egor Rumiantsevclaude
andcommitted
fix(model): zero masked pair slots before the reverse gather
Masked pair slots left each GNN layer holding nonzero features, since the transformer returns edge + attn + mlp(...) unmasked. The reverse gather is the only consumer of a masked slot, so a reverse pointing at the padded sentinel picked those features up, and the output then depended on the packed width -- a caller-side shape choice, not physics. Reachable wherever a reverse lands on the sentinel: under truncate_edges overflow, or through pack_edges on a neighbour list trimmed asymmetrically upstream (a per-center knn cap), where it happens with overflow=False and no diagnostic. Guard it structurally: padded slots must leave the backbone at exactly zero, plus a width-invariance check over an orphaned reverse. Note that a fresh init leaves every bias zero, which zeroes the sentinel and hides the leak, so the invariance test shifts parameters off init. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b20653c commit 2c6802e

2 files changed

Lines changed: 79 additions & 1 deletion

File tree

src/petjax/model.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ def __call__(
216216
)(node, edge, cutoffs_tokens, mask)
217217

218218
# Message passing (feedforward mixing)
219-
edge_flat = edge.reshape(P, d_pet)
219+
edge_flat = edge.reshape(P, d_pet) * pair_mask[..., None]
220220
reversed_flat = edge_flat[reverse]
221221
combined = jnp.concatenate([edge_flat, reversed_flat], axis=-1)
222222
combined = nn.LayerNorm(name=f"comb_norms_{layer_idx}")(combined)

tests/test_select.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,81 @@ def test_pack_edges_forward_smoke(structure):
112112
params = model.init(jax.random.key(0), **packed)
113113
energy = model.apply(params, **packed)
114114
assert bool(jnp.all(jnp.isfinite(energy)))
115+
116+
117+
def _orphaned(structure):
118+
"""Copy of `structure` with one direction of one pair dropped.
119+
120+
Mimics an upstream per-center trim (or an overflow drop): the surviving
121+
direction keeps a `reverse` pointing at the padded sentinel pair, so the
122+
forward gathers a masked slot.
123+
"""
124+
structure = {k: (v.copy() if hasattr(v, "copy") else v) for k, v in structure.items()}
125+
sentinel = structure["pair_mask"].shape[0] - 1
126+
real = np.flatnonzero(structure["pair_mask"][:sentinel])
127+
kept = int(real[0])
128+
dropped = int(structure["reverse"][kept])
129+
assert dropped != kept
130+
131+
structure["pair_mask"][dropped] = False
132+
structure["reverse"][kept] = sentinel
133+
return structure
134+
135+
136+
def _backbone():
137+
from petjax.model import Backbone
138+
139+
return Backbone(
140+
d_pet=8,
141+
d_node=8,
142+
d_feedforward=8,
143+
num_heads=2,
144+
num_attention_layers=1,
145+
num_gnn_layers=2,
146+
cutoff=CUTOFF,
147+
)
148+
149+
150+
def _run_backbone(structure, k, shift=0.0):
151+
"""Backbone on `structure` packed to width `k`.
152+
153+
`shift` offsets every parameter: a fresh init leaves all biases zero, which
154+
happens to zero the padded sentinel slot and hide any leak through it.
155+
"""
156+
backbone = _backbone()
157+
packed, overflow = _pack(structure, k)
158+
assert not bool(overflow)
159+
inputs = {key: value for key, value in packed.items() if key != "pair_cutoffs"}
160+
params = backbone.init(jax.random.key(0), **inputs, pair_cutoffs=None)
161+
if shift:
162+
params = jax.tree.map(lambda x: x + shift, params)
163+
node, messages, _ = backbone.apply(params, **inputs, pair_cutoffs=None)
164+
return np.asarray(node), np.asarray(messages), np.asarray(packed["pair_mask"])
165+
166+
167+
def test_masked_slots_carry_no_features(structure):
168+
"""Padded pair slots leave the backbone at exactly zero.
169+
170+
The reverse gather is the one consumer of a masked slot, so anything left
171+
there leaks into real edges whose reciprocal was trimmed upstream.
172+
"""
173+
_, messages, pair_mask = _run_backbone(structure, _max_count(structure) + 5)
174+
assert np.all(messages[~pair_mask] == 0.0)
175+
176+
177+
def test_backbone_invariant_to_packed_width(structure):
178+
"""`k` is a padding choice: widening it must not move real features.
179+
180+
Runs with an orphaned reverse -- the one case that reads a masked slot --
181+
and with shifted parameters, so the sentinel slot is not incidentally zero.
182+
"""
183+
orphaned = _orphaned(structure)
184+
k = _max_count(orphaned)
185+
186+
node_tight, msg_tight, mask_tight = _run_backbone(orphaned, k, shift=0.3)
187+
node_wide, msg_wide, mask_wide = _run_backbone(orphaned, k + 11, shift=0.3)
188+
189+
np.testing.assert_allclose(node_tight, node_wide, rtol=1e-5, atol=1e-6)
190+
np.testing.assert_allclose(
191+
msg_tight[mask_tight], msg_wide[mask_wide], rtol=1e-5, atol=1e-6
192+
)

0 commit comments

Comments
 (0)