Skip to content

Commit 308051c

Browse files
committed
fix(mlx): wrong gated-delta grads for batch rows past the first
mx .at[:, t].add scatters read the update tensor with a wrong batch stride on the Metal stream, silently corrupting d_q/d_k/d_v/d_g/d_beta for every batch element except index 0. Collect per-step grads in lists and stack instead; verified against plain autodiff at B in {2,3,4}. The underlying Metal scatter indexing bug is fixed upstream in ml-explore/mlx#3483 but is not in any release yet (latest: 0.31.2). The list+stack form could revert to the scatter once a fixed mlx is the minimum supported version, though it is also the cheaper op here.
1 parent d5224f4 commit 308051c

1 file changed

Lines changed: 20 additions & 10 deletions

File tree

unsloth_zoo/gated_delta_vjp.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,16 @@ def _chunked_vjp(primals, cotangents, outputs):
122122
# BPTT: `d_state` is the cotangent w.r.t. the RETURNED state at the
123123
# current step (= input to step t+1). Starts at d_state_out, then
124124
# propagates through the recurrence + mask.
125-
d_q = mx.zeros_like(q_c)
126-
d_k = mx.zeros_like(k_c)
127-
d_v = mx.zeros_like(v_c)
128-
d_g = mx.zeros_like(g_c)
129-
d_beta = mx.zeros_like(beta_c)
125+
# Per-step grads are collected in lists and stacked afterwards: each
126+
# t is produced exactly once, and mx `.at[:, t].add` scatters read
127+
# the update with a wrong batch stride (wrong grads for every batch
128+
# row past the first; verified against plain autodiff on mlx 0.31).
129+
# Fixed upstream in ml-explore/mlx#3483, not yet in any release.
130+
d_q_steps = []
131+
d_k_steps = []
132+
d_v_steps = []
133+
d_g_steps = []
134+
d_beta_steps = []
130135
d_state = d_state_out
131136

132137
for t in range(chunk_T - 1, -1, -1):
@@ -171,7 +176,7 @@ def _chunked_vjp(primals, cotangents, outputs):
171176
+ dy_t[..., None].astype(mx.float32) * q_t[..., None, :].astype(mx.float32)
172177
)
173178
d_q_t = (dy_t[..., None].astype(mx.float32) * state_new).sum(axis=-2)
174-
d_q = d_q.at[:, t].add(d_q_t.astype(d_q.dtype))
179+
d_q_steps.append(d_q_t.astype(q_c.dtype))
175180

176181
# state_new = state_decayed + k[..., None, :] * delta[..., None]
177182
d_kd = d_state_new
@@ -203,14 +208,19 @@ def _chunked_vjp(primals, cotangents, outputs):
203208
d_g_t = d_decay
204209

205210
d_k_t = d_k_t_from_update + d_k_t_from_kv
206-
d_k = d_k.at[:, t].add(d_k_t.astype(d_k.dtype))
207-
d_v = d_v.at[:, t].add(d_v_t.astype(d_v.dtype))
208-
d_g = d_g.at[:, t].add(d_g_t.astype(d_g.dtype))
209-
d_beta = d_beta.at[:, t].add(d_beta_t.astype(d_beta.dtype))
211+
d_k_steps.append(d_k_t.astype(k_c.dtype))
212+
d_v_steps.append(d_v_t.astype(v_c.dtype))
213+
d_g_steps.append(d_g_t.astype(g_c.dtype))
214+
d_beta_steps.append(d_beta_t.astype(beta_c.dtype))
210215

211216
# d_state_prev = recurrence-derived gradient + mask passthrough.
212217
d_state = d_state_prev_via_recurrence + d_state_prev_passthrough
213218

219+
d_q = mx.stack(d_q_steps[::-1], axis=1)
220+
d_k = mx.stack(d_k_steps[::-1], axis=1)
221+
d_v = mx.stack(d_v_steps[::-1], axis=1)
222+
d_g = mx.stack(d_g_steps[::-1], axis=1)
223+
d_beta = mx.stack(d_beta_steps[::-1], axis=1)
214224
d_mask = mx.zeros_like(mask_chunk) if mask_chunk is not None else None
215225
return d_q, d_k, d_v, d_g, d_beta, d_state, d_mask
216226

0 commit comments

Comments
 (0)