Skip to content

Preserve retained samples in thinned MCMC checkpoints - #451

Open
antonsoo wants to merge 1 commit into
sblunt:v4from
antonsoo:fix/thinned-periodic-samples
Open

antonsoo wants to merge 1 commit into
sblunt:v4from
antonsoo:fix/thinned-periodic-samples

Conversation

@antonsoo

@antonsoo antonsoo commented Oct 27, 2026 •

Copy link
Copy Markdown

Periodic saving counts raw MCMC iterations when slicing a chain that stores only every thin-th iteration. With ptemcee's preallocated arrays, this includes unwritten rows and advances the saved prefix past real samples.

On current v4, a GJ504 run with 16 walkers, 12 iterations, thin=2 and periodic_save_freq=2 saves 96 rows, but 80 have zero sampled parameters. Count retained steps consistently instead. Before the first retained step, save the current walker positions without adding posterior samples. Also synchronize final restart positions when a run ends between retained steps.

The regressions compare actual HDF5 checkpoint and final sample/likelihood pairs with the live sampler, including multiple temperatures and non-divisible step counts. Local MCMC/results checks pass on Python 3.10/NumPy 1.26 and Python 3.12/NumPy 2.5. Real bundled GJ504 and HD206893b workloads repair 48 of 96 cases per profile; the 48 working cases and all 96 seeded sampler trajectories stay exact. Live and saved-file continuation checks pass, and a wheel built from clean source reproduces the source workload.

Related to #394. #446's broader save refactor rebuilds final results, but its intermediate checkpoints still use raw iteration counts and reproduce this error. This patch addresses the retained-prefix calculation; it overlaps that saving code.

@sblunt @semaphoreP, could you review this change?

Periodic saving uses raw sampler iterations to slice a chain that stores only every thin-th step. This includes preallocated future samples and advances the saved prefix too far, corrupting both checkpoints and final results. Count retained steps consistently, save walker positions before the first retained step, and synchronize the final restart positions.

This branch has not been deployed

No deployments
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