Skip to content

CDF and inverse-CDFs for (log-)priors#1033

Merged
martinjrobins merged 16 commits intomasterfrom
i1032-prior-cdf-icdf
Feb 14, 2020
Merged

CDF and inverse-CDFs for (log-)priors#1033
martinjrobins merged 16 commits intomasterfrom
i1032-prior-cdf-icdf

Conversation

@ben18785
Copy link
Collaborator

To do #282, it is necessary to have cumulative distribution functions and their inverses for the priors.

@codecov
Copy link

codecov bot commented Feb 10, 2020

Codecov Report

Merging #1033 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #1033    +/-   ##
=======================================
  Coverage     100%    100%            
=======================================
  Files          64      64            
  Lines        6577    6756   +179     
=======================================
+ Hits         6577    6756   +179
Impacted Files Coverage Δ
pints/_log_priors.py 100% <100%> (ø) ⬆️
pints/_log_pdfs.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94fdae4...b73ec94. Read the comment docs.

need to adapt example notebook; also need to see what people think we should name the cdf methods here (since they're not really cdfs!)
@ben18785
Copy link
Collaborator Author

@martinjrobins Could you have a look at this? My main issue concerns the multivariate normal -- in this case inverse cdfs are not available, so I created a pseudo-inverse-cdf function that can be used to convert samples uniformly distributed within the unit cube to those in the same space as the prior (this is necessary for multinest). Given that the cdf and icdf functions I use for this distribution are not actually what they say, I'm wondering about endowing each prior with functions "convert_to_unit_cube" and "convert_from_unit_cube". For all distributions but the mv-normal, the convert_to_unit_cube function would call the cdf and convert_from_unit_cube would call icdf. Do you think this sounds reasonable? This would avoid the mv-normal having functions that don't correspond to what they're named, at least.

@ben18785
Copy link
Collaborator Author

Actually, have gone ahead and think this is ready for review.

Copy link
Member

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

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

looks great @ben18785 . I'm happy with the "convert_from..." functions. Just one comment below about whether we can support multiple samples at once, and in either case I think the docstrings should be updated to make it clear what the dimensions of u or x can be.

Copy link
Member

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

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

great, thanks @ben18785!

@ben18785
Copy link
Collaborator Author

@martinjrobins -- prod, if happy, can you merge please?

@martinjrobins martinjrobins merged commit 5caa0ff into master Feb 14, 2020
@martinjrobins martinjrobins deleted the i1032-prior-cdf-icdf branch February 14, 2020 15:55
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