-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add Capability to Marginalize Likelihood #539
base: dev
Are you sure you want to change the base?
Conversation
That was fast. |
Codecov Report@@ Coverage Diff @@
## dev #539 +/- ##
=========================================
+ Coverage 56% 56.02% +0.01%
=========================================
Files 186 186
Lines 18035 18050 +15
=========================================
+ Hits 10101 10112 +11
- Misses 7934 7938 +4
Continue to review full report at Codecov.
|
9d150d9
to
8a71e99
Compare
We want to factor out the model evaluation from the actual computation of the log-likelihood for forthcoming additions of "marginalized parameters". So, we add lnValue() to LikelihoodBase, where it calls evaluateModel() first, then we pass the model evaluation to lnLikelihood(). Now, subclasses implement lnLikelihood with the modelOutput as an input parameter to the function. We don't pass modelOutput as const so that it can be overwritten to avoid copying the modelOutput, preserving the behavior that was there before. I'm not sure I like this behavior, but it's also a protected function that's working a vector created internal to the class hierarchy.
This is laying the groundwork for having the likelihood do marginalization for the user. This add the neccessary data and constructor to LikelihoodBase. In particular, we'll need the joint pdf of the marginal parameter space and the user-constructed integration rule for integrating over that space.
The user will need to override this method for doing marginalization in the likelihood. In particular, this method will also pass the current value of the marginal parameters in addition to the inference parameters. This will be the user function that gets called if a marginal parameter space and integration rule have been provided.
If we detect the user has supplied a marinal parameter pdf, then we instead will use the user-supplied integration rule to integrate over the parameter space, calling the user-overrideen evaluateModel() method that accepts marginal parameters as well as inference parameters.
8a71e99
to
1e2dade
Compare
OK, I have two tests now. This is ready as far as I'm concerned. Feedback welcome @dmcdougall, in particular is the documentation clear? I'm a little worried the I should note that I intend to open an issue after this is merged because one thing I'd like to do is generalize this to parallelize the quadrature loop. But that's a longer discussion. As is, this is just evaluating one quadrature point at a time in the marginalization step, which is fine if your model is cheap. |
This is testing the case where we have a uniform RV for the marginal parameters. The likelihood is just f(m) (data =0, effectively uniform pdf) for ease of analytical integration for the test.
This one is now testing where we treat the marginal parameter pdf as the weighting function in the quadrature rule. Here, we use a zero mean/unit variance Gaussian for the 2D marginal parameter space and then use Gauss-Hermite quadrature.
1e2dade
to
e339a52
Compare
Had forgotten to implement the new constructor for the Gaussian likelihood subclasses. Pushed in a commit that does so. |
Labeling do not merge again. Even though my tests are passing my application that is using this capability is not behaving as expected and I suspect one or several bugs still lingers here. Will update when its sorted, one way or another. |
This follows up on #528 to allow the marginalization of a "marginal parameter space". That is, if instead of your model looking like f(m), it looks like f(m,q) and you want to marginalize over q, you'll now be able to do that.
The code is written, but it will be a couple of more days before I can have time to write tests. That said, i wanted to open this up now to get feedback on the interface and on the clarity of the documentation (since the integration over the marginal space can be done in a couple of ways). As such, I'm labeling as do not merge until I push some tests (and squash in any fixes for bugs identified by the testing).