Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalised stats #396

Merged
merged 6 commits into from
Sep 27, 2024
Merged

Normalised stats #396

merged 6 commits into from
Sep 27, 2024

Conversation

lukashergt
Copy link
Collaborator

@lukashergt lukashergt commented Sep 19, 2024

This PR introduces a norm keyword for the NestedSamples.stats method, that allows passing a previously computed NestedSamples.stats instance as a normalisation reference, producing additional columns of differences [Delta_logZ, ...].

Typical cosmology use-case would be when you have multiple models (e.g. LCDM, wCDM, w0waCDM) and you want to have evidence and KL-divergence values relative to one particular model (e.g. LCDM). I was getting annoyed having to type this up every time myself...

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

@lukashergt lukashergt added the enhancement New feature or request label Sep 19, 2024
@lukashergt lukashergt self-assigned this Sep 19, 2024
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (8c972f5) to head (fa919c1).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #396   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           36        36           
  Lines         3060      3069    +9     
=========================================
+ Hits          3060      3069    +9     

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

Copy link
Collaborator

@AdamOrmondroyd AdamOrmondroyd left a comment

Choose a reason for hiding this comment

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

I like the idea, comments inline

anesthetic/samples.py Show resolved Hide resolved
anesthetic/samples.py Show resolved Hide resolved
Copy link
Collaborator Author

@lukashergt lukashergt left a comment

Choose a reason for hiding this comment

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

Thanks @AdamOrmondroyd, all inline comments addressed.

anesthetic/samples.py Show resolved Hide resolved
anesthetic/samples.py Show resolved Hide resolved
AdamOrmondroyd
AdamOrmondroyd previously approved these changes Sep 27, 2024
for beta in [1., 0., 0.5]:
np.random.seed(42)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need a seed here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I managed to get failing tests in what follows, because of changes to the seed from the tests I added above... Nothing dramatic, just bad chance in combination with the tests checking against 3*std or similar. I could have fiddled around with tolerances, but just trying a new seed there was enough, so I did not dig deeper...

Copy link
Collaborator

@AdamOrmondroyd AdamOrmondroyd Sep 27, 2024

Choose a reason for hiding this comment

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

ok, I'll reapprove when you've done the version number ofc this is a minor bump

Copy link
Collaborator

@AdamOrmondroyd AdamOrmondroyd left a comment

Choose a reason for hiding this comment

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

lgtm

@lukashergt lukashergt merged commit a4c8521 into master Sep 27, 2024
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants