-
Notifications
You must be signed in to change notification settings - Fork 3
Compat for Turing v0.41 #44
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
| vi = Turing.DynamicPPL.VarInfo(rng, model, Turing.SampleFromUniform()) | ||
| vi_spl = last(Turing.DynamicPPL.evaluate_and_sample!!(rng, model, vi, Turing.SampleFromUniform())) | ||
| θ = vi_spl[:] | ||
| ℓp = LogDensityProblems.logdensity(ℓ, θ) |
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.
calling logdensity leads to an extra model evaluation, we can skip this by using the logjoint which is already in the varinfo
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #44 +/- ##
==========================================
- Coverage 96.30% 96.29% -0.02%
==========================================
Files 10 10
Lines 352 351 -1
==========================================
- Hits 339 338 -1
Misses 13 13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
SliceSampling.jl documentation for PR #44 is available at: |
| uses: TuringLang/actions/DocsDocumenter@main | ||
| with: | ||
| julia-version: 1.11 |
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.
the docs example uses PG, which doesn't work on 1.12 so this has to be pinned for now
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.
Hi @penelopeysm , thanks for the PR. Everything looks good. One quick thing though, could you update the docstring here, which seems outdated now that we have a more explicit way to initialize the parameters?
|
Thanks @Red-Portal! I just deleted it because initial params are now always in unlinked space so there's no possibility of the params being different from what the user specified. I think. Does slice sampling require linked VarInfo / unconstrained parameters? |
|
@penelopeysm thanks for the update. All looks good to me now.
Except for bounded supports, in general, yes. |
|
Neat, okay. There might be some upcoming changes to the external sampler interface, but it will be a while. I'll check back again when those land! |
No description provided.