Emulated termination criteria

[yallup]

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:

• I have performed a self-review of my own code
• My code is PEP8 compliant (flake8 anesthetic tests)
• My code contains compliant docstrings (pydocstyle --convention=numpy anesthetic)
• New and existing unit tests pass locally with my changes (python -m pytest)
• I have added tests that prove my fix is effective or that my feature works
• I have appropriately incremented the semantic version number in both README.rst and anesthetic/_version.py

[yallup]

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]

Codecov Report

All modified and coverable lines are covered by tests ✅

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

❗ 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

@@            Coverage Diff            @@
##            master      #363   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           36        34    -2     
  Lines         3032      2990   -42     
=========================================
- Hits          3032      2990   -42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AdamOrmondroyd]

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

[williamjameshandley]

In the first instance, you need to revert the black adjustments for now -- the wholesale filechange makes it hard to review. Could you make an issue to discuss code formatters (e.g. switching to black).

[williamjameshandley]

Excellent. Thanks for taking charge of this @yallup -- I've wanted something like this in anesthetic for some time.

Apologies in advance -- this is going to be quite a picky review. I'd like to make sure that this will generalise to other stopping criteria, and I think the most straightforward way to get there is through a few adjacent iterations.

To make this obvious, we should implement one more stopping criteria. The simplest of these is one suggested to me by John Skilling last year, which is the same as the evidence-based one, but instead operating on the DKL (This is what John Skilling's in-house nested sampling algorithms tend to use). This can be thought of as measuring when the D_KL 'levels off'. John claims anecdotally that this is a more robust criterion.

With this in mind, I suggest a switch of 'Z' and 'D_KL' as strings to pick between criteria for now, with kwarg being conditionally popped depending on that first switch.

Other stopping criteria we could implement in future are 'logX', 'logLmax', 'ndead'.

[williamjameshandley]

In hindsight, do we need the dead points?
self.logX()[i_live[0]] + logZ_live vs self.logZ() would probably also do the trick.

[yallup]

Not sure I see this, if I am taking the ratio, self.logZ() is the union whereas I want the self.iloc[dead].logZ()? So I think I need dead idx? I will implement all the other suggestions and resend for comments

[lukashergt]

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/

[yallup]

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

[williamjameshandley]

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

[yallup]

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