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

Allow saving additional information returned by the likelihood function #395

Merged
merged 26 commits into from
Oct 3, 2022

Conversation

segasai
Copy link
Collaborator

@segasai segasai commented Sep 21, 2022

Starting implementation of #368
The function can now return an additional blob
The tests should pass, but the additional info is not saved yet.
early feedback welcome

@coveralls
Copy link

coveralls commented Sep 21, 2022

Pull Request Test Coverage Report for Build 3108342513

  • 82 of 85 (96.47%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 90.576%

Changes Missing Coverage Covered Lines Changed/Added Lines %
py/dynesty/dynamicsampler.py 33 34 97.06%
py/dynesty/utils.py 23 25 92.0%
Totals Coverage Status
Change from base Build 3088827015: 0.06%
Covered Lines: 3950
Relevant Lines: 4361

💛 - Coveralls

@segasai
Copy link
Collaborator Author

segasai commented Sep 21, 2022

This is now a workign version that allows the likelihood to return an arbitrary 1d numpy array (optionally) if blob=True option is specified in the sampler. Those are stored in results.blob attribute
the example is in the test/test_blob.py
the docs are absent
It is unclear whether the name for the 'blob' is the best one,
the tests need to be expanded to include pools and resuming.

@segasai
Copy link
Collaborator Author

segasai commented Sep 22, 2022

I think this is ready for review.
It'd be great to have a second look on this.
After excluding docs/tests the patch is surprisingly small and not too difficult.
The performance effects are probably non-zero given the overhead of objects etc (at least the test suite on my machine seemed to slow down by ~ 2.5%, but that's probably small enough to not worry about it)
Also I am thinking of making the next version 2.0 given this change and the checkpointing..

@segasai segasai merged commit 6c4779b into master Oct 3, 2022
@segasai segasai deleted the extra_info branch October 4, 2022 09:30
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.

2 participants