Skip to content

Commit a8427cc

Browse files
committed
Fix NNX MTP zero-loss, qwix MTP crash, and DeepSeek scan-axis sharding corruption
Three independent bugs in the NNX path, surfaced by running DeepSeek-V3 with multi-token prediction. mtp_loss was silently 0. mtp_losses/mtp_acceptance subclass nnx.Intermediate and nnx type filters match by isinstance, so the generic nnx.pop(model, nnx.Intermediate) in loss_fn consumed them before the dedicated pop ran. That pop then returned empty and calculate_mtp_loss fell through to its 0.0 default. Pop the subclasses first. Linen is unaffected; it keys mutable collections by name. qwix quantization crashed with "roll requires ndarray, got NoneType". NNX modules are stateful, so quantize_model must run a real forward pass, but only tokens, positions and segment ids were passed and the MTP block reads the decoder targets unconditionally. Pass dummy targets, restoring the semantics of the is_initializing() guard the NNX port left commented out. Linen never traces a forward during quantization, so it never hit this. DeepSeek params were left replicated, tripping assert_params_sufficiently_sharded once the crash above was fixed. _create_scanned_layers records each stack's axis name in nnx.PARTITION_NAME ("dense_layers"/"moe_layers"), but the scan-axis round-trip hardcoded the literal "layers". The name never matched, so the fallback stripped a real logical axis instead of the scan axis and a bogus "layers" was inserted: ('embed','dense_layers','mlp') became ('dense_layers','layers','mlp'). 'embed' is the only carrier of fsdp, hence P(None, ...). This is neither an MTP nor a qwix bug: it corrupts on every forward for multi-stack models, but is normally invisible because the sharding snapshot is taken before the first train step. qwix makes it fatal only because it quantizes inside model construction, within the traced snapshot. Resolve the axis name per-variable from nnx.PARTITION_NAME, and raise on inconsistent metadata rather than blindly stripping an axis, as flax's own nnx.spmd.remove_axis does. Verified on deepseek3-tiny (v6e-8) with sharding_tolerance untouched: nnx+mtp and nnx+mtp+qwix now both report mtp_loss 1.224, matching the Linen reference. The sharding metadata round-trip goes from 29 of 32 params corrupted to 0. gpt3-52k, gpt3-52k+qwix, gemma2-2b and deepseek3-tiny with scan_layers=false all train clean. Add regression tests for all three. multi_token_prediction_test.py drives the real loss_fn and asserts a non-zero MTP loss; quantizations_nnx_mtp_test.py quantizes an MTP model through qwix without crashing; maxtext_utils_nnx_test.py checks the scan-axis helpers keep the real logical axis. Each fails on the pre-fix code.
1 parent c7a5a39 commit a8427cc

7 files changed

Lines changed: 360 additions & 99 deletions

File tree

src/maxtext/layers/nnx_decoders.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -998,13 +998,9 @@ def layer_fn(carry, scanned_vars):
998998
final_carry, scanned_state = jax.lax.scan(layer_fn_wrapped, x_in, (params, state))
999999
returned_kv_stacked = None
10001000

1001-
# Ensure metadata rank matches the stacked values
1002-
scanned_state = maxtext_utils_nnx.nnx_add_scan_axis(scanned_state, "layers", 0)
1003-
1004-
if scan_axis != 0:
1005-
new_params, new_rest = scanned_state.split(nnx.Param, ...)
1006-
new_params = maxtext_utils_nnx.nnx_sync_moveaxis(new_params, 0, scan_axis)
1007-
scanned_state = nnx.merge_state(new_params, new_rest)
1001+
# Move the scan axis to each variable's param_scan_axis and restore its name
1002+
# in the sharding metadata. jax.lax.scan emits it at position 0.
1003+
scanned_state = maxtext_utils_nnx.nnx_add_and_sync_scan_axis(scanned_state, "layers")
10081004

10091005
returned_kv_stacked = None
10101006

src/maxtext/layers/quantizations.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,13 +861,20 @@ def maybe_quantize_model(model, config):
861861
dummy_tokens = jnp.ones(input_shape, dtype=jnp.int32)
862862
dummy_positions = jnp.ones(input_shape, dtype=jnp.int32)
863863
dummy_segment_ids = jnp.ones(input_shape, dtype=jnp.int32)
864+
# The MTP block reads the decoder targets, so the qwix forward pass needs them.
865+
# The Linen path supplies them from the is_initializing() guard in Transformer.
866+
dummy_targets = {}
867+
if config.mtp_num_layers > 0:
868+
dummy_targets["decoder_target_tokens"] = jnp.ones(input_shape, dtype=jnp.int32)
869+
dummy_targets["decoder_target_mask"] = jnp.ones(input_shape, dtype=jnp.int32)
864870
model = qwix.quantize_model(
865871
model,
866872
quantization_provider,
867873
dummy_tokens,
868874
dummy_positions,
869875
dummy_segment_ids,
870876
enable_dropout=False,
877+
**dummy_targets,
871878
)
872879
# Qwix quantization runs a forward pass during tracing, which sows transient nnx.Intermediate variables
873880
# (e.g. max_logits from QK-Clip, MTP losses) into the model. Popping them here prevents structural mismatches

src/maxtext/trainers/pre_train/train.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -197,18 +197,22 @@ def loss_fn(model, config, data, dropout_rng, params, sparsity_state=None, is_tr
197197
decoder_target_tokens=data["targets"],
198198
decoder_target_mask=data["targets_segmentation"],
199199
)
200+
# mtp_losses and mtp_acceptance subclass nnx.Intermediate, and nnx type filters match
201+
# subclasses. Pop them before the generic Intermediate pop below, which would otherwise
202+
# take them too and leave the MTP loss silently reading as 0.
203+
mtp_losses_state, mtp_acceptance_state = None, None
204+
if config.mtp_num_layers > 0:
205+
mtp_losses_state = nnx.pop(model, mtp_losses)
206+
mtp_acceptance_state = nnx.pop(model, mtp_acceptance)
207+
200208
intermediates = nnx.pop(model, nnx.Intermediate)
201209
intermediate_outputs = intermediates.to_pure_dict()
202210

203-
# MTP sows mtp_losses/mtp_acceptance as custom Variable subclasses, not
204-
# Intermediate, so the nnx.pop above misses them. Pop them here under their
205-
# collection names so calculate_mtp_loss / calculate_mtp_acceptance_rate
206-
# find them. Otherwise the MTP loss is silently zeroed. They are also
207-
# excluded from the returned state below so they don't leak into
208-
# out_shardings.
209-
if config.mtp_num_layers > 0:
210-
intermediate_outputs["mtp_losses"] = nnx.pop(model, mtp_losses).to_pure_dict()
211-
intermediate_outputs["mtp_acceptance"] = nnx.pop(model, mtp_acceptance).to_pure_dict()
211+
# Store them under the collection name so calculate_mtp_loss and
212+
# calculate_mtp_acceptance_rate find them at the same path as the Linen collections.
213+
if mtp_losses_state is not None and mtp_acceptance_state is not None:
214+
intermediate_outputs["mtp_losses"] = mtp_losses_state.to_pure_dict()
215+
intermediate_outputs["mtp_acceptance"] = mtp_acceptance_state.to_pure_dict()
212216

213217
if (config.use_indexer and not config.indexer_sparse_training) and is_train:
214218
# In Dense Warm-up stage, we skip main model loss calculation for efficiency.

src/maxtext/utils/maxtext_utils_nnx.py

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -218,64 +218,64 @@ def nnx_update_sharding_meta(variable, transform_fn):
218218
return variable
219219

220220

221-
def nnx_sync_moveaxis(tree, from_axis, to_axis):
222-
"""Moves an axis in both values and sharding metadata of nnx.Variables."""
223-
if from_axis == to_axis:
224-
return tree
225-
226-
def _op(x):
227-
is_var = isinstance(x, nnx.Variable)
228-
val = x.get_value() if is_var else x
229-
if not hasattr(val, "shape"):
230-
return x
231-
232-
new_val = jnp.moveaxis(val, from_axis, to_axis)
233-
if not is_var:
234-
return new_val
235-
236-
def move_fn(l):
237-
while len(l) < val.ndim:
238-
l.append(None)
239-
if len(l) > max(from_axis, to_axis):
240-
l.insert(to_axis, l.pop(from_axis))
241-
return l
242-
243-
return nnx_update_sharding_meta(x.replace(value=new_val), move_fn)
244-
245-
return jax.tree.map(_op, tree, is_leaf=lambda x: isinstance(x, nnx.Variable) or hasattr(x, "shape"))
246-
247-
248221
def nnx_remove_scan_axis(tree, name="layers"):
249222
"""Removes the given scan axis from the PartitionSpec."""
250223

251224
def _op(x):
252225
if not isinstance(x, nnx.Variable):
253226
return x
254227

228+
# Scanned stacks record their own axis name, such as "dense_layers" or "moe_layers",
229+
# so prefer it over the caller's default. Otherwise the name never matches and the
230+
# check below strips a real logical axis instead of the scan axis.
231+
axis_name = x.get_metadata().get(nnx.PARTITION_NAME, name)
232+
255233
def remove_fn(l):
256-
if name in l:
257-
l.remove(name)
258-
while len(l) > x.get_value().ndim:
259-
l.pop(0)
234+
removed = axis_name in l
235+
if removed:
236+
l.remove(axis_name)
237+
if len(l) > x.get_value().ndim:
238+
if removed:
239+
raise ValueError(
240+
f"Sharding names {l} still exceed value rank {x.get_value().ndim} after removing scan axis "
241+
f"{axis_name!r}; the partition metadata is inconsistent."
242+
)
243+
raise ValueError(
244+
f"Scan axis {axis_name!r} not found in sharding names {l} for a rank-{x.get_value().ndim} value; "
245+
"the partition metadata is inconsistent."
246+
)
260247
return l
261248

262249
return nnx_update_sharding_meta(x, remove_fn)
263250

264251
return jax.tree.map(_op, tree, is_leaf=lambda x: isinstance(x, nnx.Variable))
265252

266253

267-
def nnx_add_scan_axis(tree, name="layers", pos=0):
268-
"""Adds the given scan axis to the PartitionSpec at the specified position."""
254+
def nnx_add_and_sync_scan_axis(tree, name="layers", pos=0):
255+
"""Restores the scan axis on each variable's value and sharding metadata.
256+
257+
jax.lax.scan stacks its outputs with the scan axis at position 0. For each
258+
variable this moves that axis to the variable's own param_scan_axis (falling
259+
back to pos when the metadata is absent) and inserts the matching axis name at
260+
the same position, so the value and its sharding metadata stay aligned.
261+
"""
269262

270263
def _op(x):
271264
if not isinstance(x, nnx.Variable):
272265
return x
273266

267+
axis_name = x.get_metadata().get(nnx.PARTITION_NAME, name)
268+
target = x.get_metadata().get("param_scan_axis", pos)
269+
270+
val = x.get_value()
271+
if target != 0 and hasattr(val, "ndim") and val.ndim > target:
272+
x = x.replace(value=jnp.moveaxis(val, 0, target))
273+
274274
def add_fn(l):
275-
if name not in l:
275+
if axis_name not in l:
276276
while len(l) < x.get_value().ndim - 1:
277277
l.append(None)
278-
l.insert(pos, name)
278+
l.insert(target, axis_name)
279279
else:
280280
while len(l) < x.get_value().ndim:
281281
l.append(None)

tests/unit/maxtext_utils_nnx_test.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,5 +203,68 @@ def test_nnx_ensure_scan_leading_axis_mixed(self):
203203
self.assertEqual(broadcast_state["raw_array"].shape, (10,))
204204

205205

206+
def _make_scanned_param(shape, out_sharding, partition_name):
207+
"""Build an nnx.Param mirroring a scanned decoder variable.
208+
209+
DeepSeek stacks name their scan axis via nnx.PARTITION_NAME ("dense_layers" or
210+
"moe_layers"), so the variable carries that name in its metadata rather than
211+
the caller literal "layers". Eager sharding is disabled because the sliced
212+
value rank is intentionally one less than the metadata length.
213+
"""
214+
with nnx.use_eager_sharding(False):
215+
return nnx.Param(jnp.zeros(shape), out_sharding=out_sharding, **{nnx.PARTITION_NAME: partition_name})
216+
217+
218+
class TestScanAxisMetadata(unittest.TestCase):
219+
"""Lock in per-variable scan-axis resolution for DeepSeek-style stacks."""
220+
221+
def test_nnx_remove_scan_axis_preserves_real_leading_axis(self):
222+
"""nnx_remove_scan_axis must drop the stack's own scan axis, not "embed"."""
223+
# Metadata carries the real fsdp axis "embed" plus the scan axis
224+
# "dense_layers"; the sliced value is rank-2, one less than the 3 names.
225+
param = _make_scanned_param((4, 8), ("embed", "dense_layers", "mlp"), "dense_layers")
226+
state = nnx.State({"kernel": param})
227+
228+
result = maxtext_utils_nnx.nnx_remove_scan_axis(state, "layers")
229+
230+
out_sharding = result["kernel"].get_metadata().get("out_sharding")
231+
# The scan axis is gone and the real leading logical axis survives. The
232+
# unfixed fallback pops index 0 and strips "embed" to ("dense_layers", "mlp").
233+
self.assertEqual(tuple(out_sharding), ("embed", "mlp"))
234+
235+
def test_nnx_remove_scan_axis_moe_layers(self):
236+
"""The same resolution holds for the "moe_layers" stack axis name."""
237+
param = _make_scanned_param((4, 8), ("embed", "moe_layers", "mlp"), "moe_layers")
238+
state = nnx.State({"kernel": param})
239+
240+
result = maxtext_utils_nnx.nnx_remove_scan_axis(state, "layers")
241+
242+
out_sharding = result["kernel"].get_metadata().get("out_sharding")
243+
self.assertEqual(tuple(out_sharding), ("embed", "mlp"))
244+
245+
def test_nnx_remove_scan_axis_raises_on_inconsistent_metadata(self):
246+
"""A rank mismatch with no matching scan axis is an error, not a silent pop."""
247+
# No name in the metadata matches the resolved scan axis, and the metadata
248+
# is one longer than the value rank; the fixed code raises instead of
249+
# blindly popping a real axis.
250+
param = _make_scanned_param((4, 8), ("embed", "mlp", "vocab"), "dense_layers")
251+
state = nnx.State({"kernel": param})
252+
253+
with self.assertRaises(ValueError):
254+
maxtext_utils_nnx.nnx_remove_scan_axis(state, "layers")
255+
256+
def test_nnx_add_and_sync_scan_axis_uses_partition_name(self):
257+
"""nnx_add_and_sync_scan_axis must insert the stack's own axis name, not "layers"."""
258+
# Stacked value is rank-3; metadata holds the two sliced logical axes.
259+
param = _make_scanned_param((2, 4, 8), ("embed", "mlp"), "dense_layers")
260+
state = nnx.State({"kernel": param})
261+
262+
result = maxtext_utils_nnx.nnx_add_and_sync_scan_axis(state, "layers", 0)
263+
264+
out_sharding = result["kernel"].get_metadata().get("out_sharding")
265+
# The unfixed code inserts the literal "layers" here.
266+
self.assertEqual(tuple(out_sharding), ("dense_layers", "embed", "mlp"))
267+
268+
206269
if __name__ == "__main__":
207270
unittest.main()

0 commit comments

Comments
 (0)