Add checkpointing support for long-running mechanism synthesis in dpsynth: - #166
Add checkpointing support for long-running mechanism synthesis in dpsynth:#166copybara-service[bot] wants to merge 1 commit into
Conversation
…ynth: - Introduce `Checkpointer` (`dpsynth/checkpoint.py`) leveraging `etils.epath` and `mbi` pytree serialization to persist and restore intermediate mechanism artifacts (.npz files) when a `working_dir` is specified, and acting as a transparent no-op when disabled (`working_dir=None`). - Add `working_dir` property to `MechanismConfig` and `SWIFTConfig`. - Implement stage-level resume in `SWIFT`: - Fast-path resume to synthetic data generation when both `model.npz` and `measurements.npz` exist. - Intermediate resume reusing cached exact marginals (`marginals.npz`) to skip `from_projectable`. - Checkpointing `measurements.npz` after query measurement and `model.npz` after PGM estimation. - Add unit tests for `Checkpointer` and stage resumes in `SWIFT`. PiperOrigin-RevId: 972268686
MuhammadNiazAli
left a comment
There was a problem hiding this comment.
The import reorganization and new checkpoint_lib import look clean. The new working_dir config field is well-documented in the docstring, including the None-disables-checkpointing behavior, which is good.
I've only reviewed swift.py here there are 3 more changed files I haven't looked at yet, so holding off on approving until I go through those too.
MuhammadNiazAli
left a comment
There was a problem hiding this comment.
The two-stage resume logic in swift.py reads well:
Full resume (model.npz + measurements.npz both exist) skips straight to synthesis using the cached model/measurements, correctly falling back to initial_measurements over the checkpointed ones when both are available (total_src = initial_measurements if initial_measurements else measurements).
Partial resume (marginals only) — loads marginals.npz if present, otherwise computes via mbi.CliqueVector.from_projectable and saves it before continuing. The assert answers is not None after load is a nice defensive touch even though exists() already gated it.
One thing I couldn't verify from the diff alone: checkpointer = checkpoint_lib.Checkpointer(self.config.working_dir) is constructed unconditionally, and the docstring says working_dir=None disables checkpointing. Worth confirming Checkpointer(None) actually makes exists() always return False and save()/load() no-ops otherwise this line would break for anyone not opting into checkpointing.
Also noticed in the full-resume branch, phase_times stays {} since none of the common.timed(...) blocks run that's expected, but if any downstream diagnostics code assumes specific phase keys are always present, it'd be worth a quick check.
test_checkpointing_saves_and_resumes in swift_test.py is a good addition — real temp dir, real save/load round-trip rather than mocking. The unused-looking from etils import epath import is that used later in the test for path handling, or is it dead? Couldn't tell from what's shown.
Add checkpointing support for long-running mechanism synthesis in dpsynth:
Checkpointer(dpsynth/checkpoint.py) leveragingetils.epathandmbipytree serialization to persist and restore intermediate mechanism artifacts (.npz files) when aworking_diris specified, and acting as a transparent no-op when disabled (working_dir=None).working_dirproperty toMechanismConfigandSWIFTConfig.SWIFT:model.npzandmeasurements.npzexist.marginals.npz) to skipfrom_projectable.measurements.npzafter query measurement andmodel.npzafter PGM estimation.Checkpointerand stage resumes inSWIFT.