-
Notifications
You must be signed in to change notification settings - Fork 228
Update logp in varinfo when external samplers are used #2616
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
Conversation
Turing.jl documentation for PR #2616 is available at: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2616 +/- ##
==========================================
- Coverage 85.60% 85.46% -0.15%
==========================================
Files 22 22
Lines 1459 1465 +6
==========================================
+ Hits 1249 1252 +3
- Misses 210 213 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 16291041266Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - 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.
Good stuff, just one trivial typo.
src/mcmc/external_sampler.jl
Outdated
|
||
There are a few more optional functions which you can implement to improve the integration with Turing.jl: | ||
|
||
- `Turing.Inference.isgibbscomponent(::MySampler)`: If you want your sampler to function as a component in Turing's Gibbs sampler, you should make this to `true`. |
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.
- `Turing.Inference.isgibbscomponent(::MySampler)`: If you want your sampler to function as a component in Turing's Gibbs sampler, you should make this to `true`. | |
- `Turing.Inference.isgibbscomponent(::MySampler)`: If you want your sampler to function as a component in Turing's Gibbs sampler, you should make this evaluate to `true`. |
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.
Changed, thanks!
Closes #2583, where the logp from
externalsampler
was not being correctly set invarinfo
and hence the chain.Plots shown below:
Before = logp was never updated, so logp is just a flat line.
After = logp changes with the parameters.
These were run using AdvancedMH. I've tested with AdvancedHMC and SliceSampling (cf. TuringLang/SliceSampling.jl#15) and similar improvements are observed.
Before
After