-
Notifications
You must be signed in to change notification settings - Fork 0
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
Bijectors #16
Bijectors #16
Conversation
@ThomasGesseyJones and @kilian1103 may be interested in this. |
Hi @williamjameshandley, this is very cool! Since your |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #16 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 6 +1
Lines 361 416 +55
=========================================
+ Hits 361 416 +55 ☔ View full report in Codecov by Sentry. |
|
||
Parameters | ||
---------- | ||
x : array_like, shape (..., d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this want/benefit stricter shape checking? I note for example if I have a 100D data likelihood I can
model.likelihood(theta).bijector(np.random.rand(1)[...,None])
i.e. I'm passing something of shape (1,1), rather than (100,) or (1,100) and it returns a valid data draw, but I'm not sure what this actually is!
"""Bijector between U([0, 1])^d and the distribution. | ||
|
||
- x in [0, 1]^d is the hypercube space. | ||
- theta in R^d is the physical space. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this docstring could clarify as this method is valid on likelihood or posterior/prior what physical space is. It feels most natural to define this for parameter space (theta doubly suggesting this) transformations or some comment on it's dual usage (if it is intended/makes sense to use on data distributions too)
y = x[..., i] | ||
a = (m - 10 * np.sqrt(c)).min(axis=-1) | ||
b = (m + 10 * np.sqrt(c)).max(axis=-1) | ||
theta[..., i] = bisect(f, a, b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would benefit from some inline comments/ more expanded docstring explaining what is going on here as an additional moving part.
Description
Adds bijective functions to lsbi.stats.multivariate_normal and lsbi.stats.mixture_multivariate normal to/from the unit hypercube from/to physical space.
The first of these is a relatively trivial helper function, the second implements this for mixture models. The inverse model is efficient, whilst the forward requires a bisection search. This bisection search is vectorised, and so is efficient for large numbers of points, but relatively slow for a single point.
Checklist:
flake8 lsbi tests
)pydocstyle --convention=numpy lsbi
)python -m pytest
)