Skip to content

refactor(utils): split checkpoints.py into 3 focused modules - #28

Merged
NorbertRop merged 1 commit into
mainfrom
refactor/checkpoints-split
May 26, 2026
Merged

NorbertRop merged 1 commit into
mainfrom
refactor/checkpoints-split

Conversation

@NorbertRop

Copy link
Copy Markdown
Contributor

Summary

`src/jaxpot/utils/checkpoints.py` had grown to 636 lines mixing three independent concerns. Split into three focused modules following the same pattern as PR #19 (train_selfplay split) and PR #24 (visualizer split).

File layout

File Owns Size
`utils/lr_schedule.py` (NEW) `create_lr_schedule(cfg) -> optax.Schedule` 79 lines
`utils/checkpoints.py` (shrinks) `Checkpoint` dataclass + `CheckpointManager` class. Atomic publish save/resume. 364 lines
`utils/best_checkpoints.py` (NEW) `BestCheckpointEntry`, `BestCheckpointSelection`, `configure_best_checkpoint_selection`, `BestCheckpointManager`. Metric-based top-k tracker. 225 lines

Each module has one cohesive responsibility. The original 636 lines becomes 668 across three files (slight overhead for new module docstrings + minor import duplication).

Callers updated

  • `train_selfplay.py` — import split (1 import block → 2).
  • `src/jaxpot/training/setup.py` — import split (1 block → 3 for best_checkpoints / checkpoints / lr_schedule).
  • `src/jaxpot/training/checkpointing.py` — import split (1 block → 2).
  • `tests/test_train_hydra_integration.py` — import split (1 line → 2).

Five other callers (`web/checkpoint_loader.py`, `utils/eval_cli.py`, `scripts/eval_pgx_policy.py`, `tests/test_checkpoint_roundtrip.py`, `tests/test_checkpoints_atomicity.py`) only use `CheckpointManager` — unchanged.

Tests

`tests/test_utils_checkpoints_imports.py` (NEW, 3 tests) — smoke test pinning the public surface of each new module and asserting the `Checkpoint`, `BestCheckpointEntry`, `BestCheckpointSelection` dataclass fields haven't drifted.

What did NOT change

Test plan

  • `uv run pytest tests/` — 236 passed, 2 skipped (was 233 + 3 new smoke tests).
  • All five external callers re-imported correctly; no `ImportError` at collect time.

🤖 Generated with Claude Code

The 636-line utils/checkpoints.py mixed three concerns:

- LR schedule construction (create_lr_schedule, ~60 lines).
- Atomic checkpoint save/resume (Checkpoint dataclass + CheckpointManager
  class, ~360 lines).
- Metric-based top-k best-checkpoint tracking (BestCheckpointEntry,
  BestCheckpointSelection, configure_best_checkpoint_selection,
  BestCheckpointManager, ~210 lines).

Split into:
- src/jaxpot/utils/lr_schedule.py (NEW): create_lr_schedule.
- src/jaxpot/utils/checkpoints.py (shrinks to 364 lines): Checkpoint +
  CheckpointManager.
- src/jaxpot/utils/best_checkpoints.py (NEW): BestCheckpointEntry,
  BestCheckpointSelection, configure_best_checkpoint_selection,
  BestCheckpointManager.

Five callers (train_selfplay, training/setup, training/checkpointing,
tests/test_train_hydra_integration, plus the new smoke test) updated
to import from the new locations. No behavior change.

New tests/test_utils_checkpoints_imports.py pins the public surface
of each module so a typo or accidental removal surfaces at collect
time rather than downstream.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@NorbertRop
NorbertRop merged commit e36467b into main May 26, 2026
1 check passed
@NorbertRop
NorbertRop deleted the refactor/checkpoints-split branch May 26, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant