Skip to content

Add checkpointing support for long-running mechanism synthesis in dpsynth: - #166

Open
copybara-service[bot] wants to merge 1 commit into
mainfrom
cl/972268686
Open

Add checkpointing support for long-running mechanism synthesis in dpsynth:#166
copybara-service[bot] wants to merge 1 commit into
mainfrom
cl/972268686

Conversation

@copybara-service

Copy link
Copy Markdown

Add checkpointing support for long-running mechanism synthesis in dpsynth:

  • 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.

…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 MuhammadNiazAli left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 MuhammadNiazAli left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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