-
Notifications
You must be signed in to change notification settings - Fork 19
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
Added support for models using the LogDensityProblems.jl interface #70
Conversation
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.
Excellent, thank you!
I'll merge this in after tests pass |
Tests are failing because we're still testing on Julia 1.0, let me fix that. |
🥳 Thank you, @torfjelde! |
@@ -68,6 +68,24 @@ Quantiles | |||
|
|||
``` | |||
|
|||
### Usage with [`LogDensityProblems.jl`](https://github.com/tpapp/LogDensityProblems.jl) | |||
|
|||
It can also be used with models defining the [`LogDensityProblems.jl`](https://github.com/tpapp/LogDensityProblems.jl) interface by wrapping it in `AbstractMCMC.LogDensityModel` before passing it to `sample`: |
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.
Is it really necessary to wrap the log density? Couldn't we wrap anything in a LogDensityModel that's not one of our specialized model types? We could even check LogDensityProblems.capabilities !== nothing
to ensure that we do not wrap anything accidentally that does not implement the interface.
That means this PR should have bumped Julia compat to 1.6, shouldn't it? |
Oops, thanks for catching that, @devmotion |
With this change it's possible to use models implementing the LogDensityProblems.jl interface. This is now easy to support after TuringLang/AbstractMCMC.jl#110.
Similar PR merged for AHMC.jl: TuringLang/AdvancedHMC.jl#301