-
Notifications
You must be signed in to change notification settings - Fork 219
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
Reenable PriorContext for Optimization #2165
Conversation
AFAICT support for |
I see. It was working before though. Do you think it makes sense to support it for some of the user use cases? |
IMO if we 're allowing both |
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 had one minor style change to suggest.
I also got some extra peace of mind by, in the test, checking that
true_prior_logpdf = -Distributions.logpdf(Uniform(0, 2), a[1])
@test Turing.OptimLogDensity(m1, prictx)(a) ≈ true_prior_logpdf
Could incorporate a more explicit check like that.
Both of these are optional, I'd be okay with the code as is. I'll leave approving for @torfjelde since there might be context (pun unintended) to this that I lack.
@mhauru I have incorporated your suggestions, thank you! |
Would be happy to include this:) But we need to fix the merge conflicts 😕 |
Co-authored-by: Markus Hauru <[email protected]>
Pull Request Test Coverage Report for Build 9384233247Details
💛 - Coveralls |
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.
Looks good to me
Thanks @alyst! |
Lovely work @alyst :) |
This PR reenables the support for PriorContext in optimization, which was disabled by #2022.
While it is not directly supported by the current optim_problem() interface (only MLE/MAP are supported, although adding MaxPrior is possible), I think it is useful to support PriorContext for Turing.OptimLogDensity().
In my code I use it for multi-objective Pareto optimization (prior + llh).