-
Notifications
You must be signed in to change notification settings - Fork 18
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
Adding facility for separation of the log likelihood and log prior via a Joint struct #75
Comments
I'm not particularly happy about enforcing a specific struct, it seems a bit restrictive. Also it seems difficult (impossible?) to evaluate both the prior and likelihood in a single execution of the model. What about the following design:
This would allow
|
Just an additional note: of course, this suggestion would still allow you to use a struct for implementing |
By this I presume you mean separately evaluating just the prior / likelihood? This is relatively simple as you can in the case of a model = DensityModel(Joint(lprior, llikelihood))
# then:
model.logdensity.lprior(z), model.logdensity.llikelihood(z)
# or to evaluate the joint
model.logdensity(z) Of course here we are not quite conforming to the intended usage pattern, as we must access the logdensity as a component of the model rather than the usual As for your proposal, I think it sounds good, and just to clarify, this conceptual splitting of the joint is only necessary in AMH / AHMC where The part I am not following as a result is, what would end-user usage look like with this approach? Presumably they'd define a logprior, a loglikelihood, but then there would still be a step where in AMH say we would need to define a new model constructor that then combines the two, or expect the user to do this themselves? Could you give an example of what this would look like as to me it seems that regardless we end up back to a model where the interior density function is the sum of the two components like you said, but these components cannot be accessed retrospectively as is required for tempering. |
No, I mean executing e.g. a Turing model and then accumulating log prior and log likelihood in one run (these quantities are evaluated by executing the model, there's usually no closed form expression or function). |
Ah I see, yeah it is a funny one because this whole approach is only required for anything external to |
With the suggestion, users/developers would just have to implement |
No, there would not necessarily be such a step as the DynamicPPL example shows. Basically, overloading struct Model{F}
execute::F
end
function logprior(model::Model, x)
return model.execute(PriorContext(), x)
end
function loglikelihood(model::Model, x)
return model.execute(LikelihoodContext(), x)
end (similar to what e.g. DynamicPPL does) and struct Model{P,L}
prior::P
likelihood::L
end
logprior(model::Model, x) = logpdf(model.prior, x)
loglikelihood(model::Model, x) = model.likelihood(x) (basically what EllipticalSliceSampling does: https://github.com/TuringLang/EllipticalSliceSampling.jl/blob/d54630e397b15efdae9c0ef25af839f62e8f35c2/src/model.jl#L3-L13). The |
Many of the packages that build upon AMCMC define model's with a log density function, in general this is the joint density consisting of the sum of the log prior and likelihood, for the
MCMCTempering
and some other situations (@yebai made me aware of these I cannot remember them off the top of my head but was assured I might have some luck in advocating for this as I believe it has a reasonably wide use case), I propose we add something like the following to AMCMC:The purpose is to allow a user to - if they desire - define the log prior and likelihood separately and pass this in to a logdensity model in AMH / as the log density function in AHMC etc. In most cases there is not much motivation to do this as you would simply sum your components in the density function, but having them separable is critical for MCMCTempering at least to work, and in general is a cheap and very simple interface to facilitate operation on the prior / likelihood.
I am open to a bit of discussion on the design of this, as it is a bit of a weird thing to expose to a user potentially, unless they are using tempering or something like that, so I would imagine it would just be documented and flagged up as an option to users rather than being explicitly encouraged, but perhaps other people have an idea for how the same functionality can be achieved in a nicer way in terms of design.
In tempering, we use the user-defined
Joint
in the model to then apply a temperature to result in the followingTemperedJoint
, hopefully this illustrates what I mean:Currently I need to add both the
Joint
andTemperedJoint
to any tempering implementations that depend on MCMCTempering (aside from Turing which works definitely as the user never defines the logdensity anyway), so ifJoint
could go in AMCMC, I could addTemperedJoint
toMCMCTempering
and the requirements for adding tempering to samplers becomes even more trivial.Woud like to hear thoughts, relatively simple PR if people are happy to add it.
The text was updated successfully, but these errors were encountered: