[NNX] Emergency checkpoint: build the manager with a Linen-layout abstract#4372
Draft
ecnal-cienet wants to merge 2 commits into
Draft
[NNX] Emergency checkpoint: build the manager with a Linen-layout abstract#4372ecnal-cienet wants to merge 2 commits into
ecnal-cienet wants to merge 2 commits into
Conversation
0e7124b to
03d1b36
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
fc2d7ec to
8cf016e
Compare
The NNX-only rngs/dropout state is stripped from the Linen on-disk layout, so on resume it was refilled from a base key(0) -- every preemption reset the dropout RNG stream even though the dataset iterator was restored. Persist that state as a separate nnx_aux composite item (the items payload stays byte-identical to the Linen layout, preserving interchangeability) and restore it when present, falling back to deterministic defaults for Linen-trained or pre-fix checkpoints. Emergency managers keep the default-fill path. Also add checkpoint_missing_param_policy for a weight the checkpoint doesn't carry. Weights are filtered by Variable type (nnx.Param via nnx.split_state) so only real weights are checked -- rngs/dropout/batch stats are never flagged. "warn" (default) logs a warning naming the weight and keeps its init value; "error" raises.
…t abstract pure_nnx saves in the on-disk checkpoint layout (Linen params/opt_state/step plus an nnx_aux subtree) for both the regular and emergency managers. But the emergency manager is constructed with the NNX-native abstract (model/optimizer) from get_abstract_state, and unlike the regular manager it bakes that abstract in and restores against it, ignoring the restore-time item. So on resume Orbax compared a checkpoint-layout tree on disk against an NNX target and raised 'User-provided restore item and on-disk value metadata tree structures do not match', failing the EMC resume-from-GCS job. Convert the abstract with to_checkpoint_dict in create_orbax_emergency_checkpoint_manager so the baked-in blueprint matches what maybe_save_checkpoint writes; the restore path reshapes it back to NNX. Adds a mock guard (asserts the manager gets the checkpoint layout, not NNX roots) plus an end-to-end save/restore round trip through the real emergency manager. Stacked on the to_checkpoint_dict / nnx_aux format change.
8cf016e to
1c96f5c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The EMC resume-from-GCS test (
maxtext_emc_resume_from_gcs) fails when a pure_nnx run restores from step 99:What's happening: pure_nnx saves checkpoints in the on-disk checkpoint layout (
params/opt_state/stepplus annnx_auxsubtree, viato_checkpoint_dict) so old Linen checkpoints stay compatible. Both the regular and the emergency manager save this way. The difference is that when we build the emergency manager we hand it the NNX abstract state (model/optimizer), and the emergency manager keeps that abstract and restores against it — it does not use the item we pass at restore time. So it ends up matching a checkpoint-layout tree on disk against an NNX tree, and Orbax complains.This is also why the earlier fix (0da53c3) didn't help: it converts the restore-time item, but the emergency manager never looks at that item.
Fix: convert the abstract with
to_checkpoint_dictinsidecreate_orbax_emergency_checkpoint_managerwhen it's annnx.State, so what the manager holds matches what's on disk. Restore still reshapes back to NNX afterwards, so the state we train on is NNX. On-disk format is unchanged, so regular and EMC checkpoints are still interchangeable. 5 lines, in the constructor.Tests
Reproduced and verified on our v6e-8 (jax 0.10.0, same as the failing job): before the fix the save→restore hits the exact same ValueError, after the fix it restores fine.
TestEmergencyManagerAbstractLayout— mocks the Orbax manager and checks it gets a Linen abstract (params/opt_state/step), not the NNXmodel/optimizer. Runs anywhere, so CI catches it if someone breaks this again.TestEmergencySaveRestoreRoundTrip— full save→restore through the real emergency manager, checks it comes back as NNX with the right weights. Skips if the EMC manager can't be built.Checklist
Before submitting this PR, please make sure (put X in square brackets):
gemini-reviewlabel.