Skip to content

chore: Refactor sampler parallelism and add example-model workflows#39

Merged
KOVALW merged 7 commits intowtk-mpfrom
cdc-as81-parallel-calibration
Mar 31, 2026
Merged

chore: Refactor sampler parallelism and add example-model workflows#39
KOVALW merged 7 commits intowtk-mpfrom
cdc-as81-parallel-calibration

Conversation

@cdc-as81
Copy link
Copy Markdown
Collaborator

@cdc-as81 cdc-as81 commented Mar 26, 2026

Summary

  • Refactor ABCSampler into smaller execution components for particle evaluation, progress/reporting, run-state tracking, and batch/particlewise generation.
  • Replace process-based parallelism with thread-pool plus async orchestration, preserving repeatable results across serial and parallel runs and removing pickling/fork requirements for model runners.
  • Add sampler ergonomics including a configurable default worker count, clean verbose suppression, and explicit run-state reset/archive bookkeeping.
  • Improve the bundled example-model workflow with runnable calibration/benchmark docs, workspace import support, and benchmark output handling.

Changes

  • Added ParticleEvaluator, SamplerRunState, SamplerReporter, typed sampler request/result carriers, and dedicated generation runners.
  • Updated sampler result construction to use typed generator history and per-run success/attempt bookkeeping.
  • Adjusted the example-model entrypoints so the repo can run python -m example_model, creates benchmarks/ automatically, and ignores generated benchmark artifacts.
  • Added conftest.py path setup and expanded tests around repeatability, runner behavior, worker defaults, output suppression, and error cleanup.

@cdc-as81 cdc-as81 requested a review from KOVALW March 26, 2026 22:11
Copy link
Copy Markdown
Collaborator

@KOVALW KOVALW left a comment

Choose a reason for hiding this comment

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

I see no performance regression with these changes, the results are identifcal to those on wtk-mp, and the error message handling is helpful. I do think that this PR reveals 1) there are limits to reproducibility in the batching routine that are taken care of in the particle assignment routine, and 2) the Sampler class is doing heavy lifting that might require defining ParticleExecutor or Generation classes that contain some of the functionality. I'm comfortable merging these changes but want to flag these as issues

results1 = sampler.run_parallel_batches(
max_workers=2, chunksize=2, batchsize=4
)
results2 = sampler.run_parallel_batches(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This test fails if batchsize is not the same. I would at least assert that chunksize and max_workers should be different so that we know the results are repeatable across different parallelization schemes in the case of shifting computational resources. For example, run_parallel and run_serial produce the same results regardless of max_workers specified in run_parallel.

This may suggest that we should be passing the spawned seed sequences for each desired accepted particle to the batch runner as well

@cdc-as81 cdc-as81 marked this pull request as draft March 30, 2026 12:38
@KOVALW KOVALW marked this pull request as ready for review March 31, 2026 20:19
Copy link
Copy Markdown
Collaborator

@KOVALW KOVALW left a comment

Choose a reason for hiding this comment

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

I've added some small changes and this LGTM. I'll make an issue for reproducibility across batch processing particle requests to take advantage of the slot class

@KOVALW KOVALW merged commit 62fc2af into wtk-mp Mar 31, 2026
2 checks passed
@KOVALW KOVALW deleted the cdc-as81-parallel-calibration branch March 31, 2026 20:21
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.

3 participants