Thin trainer-only checkpoint state: a committed storage sweep - #992
Closed
claude-spd1 wants to merge 2 commits into
Closed
Thin trainer-only checkpoint state: a committed storage sweep#992claude-spd1 wants to merge 2 commits into
claude-spd1 wants to merge 2 commits into
Conversation
added 2 commits
July 27, 2026 11:17
`ckpts/<step>/training` — optimizer moments, persistent adversaries, step counter — is 80-90% of a PD checkpoint and is read by nothing but trainer resume. A run at `keep_last_n_checkpoints: 40` therefore carries ~40 copies of a trajectory tail with one legitimate reader. Three cluster-wide sweeps have gone at this by hand with scripts that were never committed; on cw-east it is 13.8 TB across two live runs today. The sweep drops `training` and nothing else, so every consumer path (`open_jax_run` restores the `decomposition` item alone) still loads a thinned step. What it forfeits is resumability, which `--keep-newest` bounds: runs named in a live SLURM job's stdout are floored at RESUME_WINDOW whatever the flag says, and each item is renamed out of the checkpoint tree before deletion so a concurrent reader never sees a half-deleted ocdbt manifest. Crew-Address: agent/r3hd
`keep_last_n_checkpoints` was `PositiveInt | None = None`, documented as "`None` (the default) keeps all checkpoints — the conservative choice", while both composition roots asserted it non-None. So the documented default was unreachable, and the field read as an optimization a run could skip rather than the storage decision it is: the two runs holding 13.8 TB of dead trajectory tail today are at `40` three days after the team settled on keeping one. Making it required deletes three asserts and forces every run to state its own retention. All 472 pinned `launch_config.yaml` on cw-east already set it, so in-flight runs resume unaffected. Crew-Address: agent/r3hd
Collaborator
Author
|
Wrong repo — every live pd job's worktree points at |
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.
ckpts/<step>/training— optimizer moments, persistent adversaries, step counter — is 80–90% of a PD checkpoint and has exactly one reader: trainer resume.open_jax_runrestores thedecompositionitem alone (a fullTrainStaterestore OOMs a single consumer GPU), and the pd-cold R2 archive already defines a deliverable asdecomposition+_CHECKPOINT_METADATA+launch_config.yaml, nevertraining.So a run at
keep_last_n_checkpoints: 40carries ~40 copies of a trajectory tail nobody will read. On cw-east that is 13.8 TB across two live runs right now, on a mount at 95%. This is board task:551 — and the fourth cluster-wide sweep of this kind. The previous three were hand-written scripts that were never committed; this one is the operation, committed.Thinning removes
ckpts/<step>/trainingand nothing else, so every consumer path still loads a thinned step. What it forfeits is resumability, and three guards bound that:--keep-newest(defaultRESUME_WINDOW = 2) leaves the newest checkpoints resumable. Runs named in the stdout of a live SLURM job are floored atRESUME_WINDOWhowever low the flag is set — the flag can only make a dead run thinner, never a live one.renamed out of the checkpoint tree onto a sibling path beforermtree, so a concurrent reader sees the item whole or absent, never a half-deleted ocdbt manifest.st_uidon the run dir, as in the earlier sweeps: shared run roots have mixed owners.Safe under a live trainer because the trainer builds its
CheckpointManageronce and neverreload()s; orbax decides retention off its in-memory list and prunes with a whole-dirrmtreethat tolerates a missing subdir._CHECKPOINT_METADATAis deliberately left listing both items — it is read only by an argument-less composite restore, which nothing in this repo issues.Dry run against cw-east's
$PARAM_DECOMP_OUT_DIR(7.8 s over 962 run dirs): 13.810 TB at the default, 19.793 TB at--keep_newest=0.Crew-Address: agent/r3hd
Second commit, added after review on the task thread:
keep_last_n_checkpointswasPositiveInt | None = None, documented as "None(the default) keeps all checkpoints — the conservative choice", while both composition roots asserted it non-None. The documented default was unreachable, and the field read as an optimization a run could skip rather than the storage decision it is. It is now required; three asserts go. All 472 pinnedlaunch_config.yamlon cw-east already set it, so in-flight runs resume unaffected. A sweep every 20 h treats the symptom forever; this is the part that stops the offenders existing.make testgreen (562 passed, 11 skipped, 11 xfailed); ruff clean; basedpyright unchanged against baseline.