handley-lab / anesthetic

Nested Sampling post-processing and plotting
https://anesthetic.readthedocs.io
MIT License
57 stars 16 forks source link

Emulated termination criteria #363

Open yallup opened 8 months ago

yallup commented 8 months ago

Description

Aim is to introduce a termination criteria, such that given a set of chains read in to anesthetic we can determine if the nested sampling runs satisfactorily terminated.

This is achieved by introducing a boolean is_terminated function, which calls the more informative critical_ratio. The aim of this is to return the ratio of something in the live points to the same something contained in the dead points. For first pass this is either the ratio of evidence or the ratio of KL divergences for these two point populations. The former then emulates the PC and MN termination criteria, so the default call of is_terminated is the exact polychord setup.

Checklist:

yallup commented 8 months ago

I would suggest black as a pre-commit hook, apologies I tangled some opinionated code formatting up in this, the code style not being enforced as a pre-commit seems vulnerable to such things!

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (80769c5) to head (5ef5cd9).

:exclamation: Current head 5ef5cd9 differs from pull request most recent head 27f5ddb. Consider uploading reports for the commit 27f5ddb to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #363 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 36 34 -2 Lines 3032 2990 -42 ========================================= - Hits 3032 2990 -42 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AdamOrmondroyd commented 8 months ago

I would suggest black as a pre-commit hook, apologies I tangled some opinionated code formatting up in this, the code style not being enforced as a pre-commit seems vulnerable to such things!

There is an (optional) hook to check the formatting, but there isn't a formatter. Personally I've grown to like having to do the anesthetic formatting by hand as it tends to look nicer and sometimes it forces me to think again about an ugly bit of code

yallup commented 8 months ago

Introduces a PC termination criteria to check on live points

Would be nice if we could make the description a bit more detailed. Currently this leaves me at a bit of a loss what this is exactly about.

Does this addition deserve an entry in our docs? https://anesthetic.readthedocs.io/en/latest/

Added a more complete description of the goal here to the original pr.

@williamjameshandley Reworked in a slightly unpleasant way to have the two functions declared only in scope, with some currently unnecessary code duplication as it stands, but I wanted to separate them in case the logX was added in a different way.

As this currently stand the tests are failing for the D_KL version as the run reports being terminated far too early (if I truncate at an early logL), I suspect my sums may be the wrong way round

yallup commented 7 months ago

OK, I've refactored this into something which now gets DKL correctly, and reorganises into something which impacts the samples class a little more gently. I've also implemented a few more criteria which shows why this layout is slightly more general.

The DKL was actually quite subtle, and after some thought it was not obvious how you could easily re-use the calculation from the evidence material.

Updates to documentation/feedback/tests welcome

Thanks Will, much cleaner in namespace when a separate module. My only request, and I am happy to make these changes is to be able to access the value as well as the criteria. As not all are expressed as a ratio my suggestion was to make all functions in terminate return a tuple of (Bool, float) representing the criteria and the underlying value

If that sounds reasonable I will make those changes to this PR