Conversation
|
Yes, we'd like to merge the AdvancedPS code to Turing.jl. Libtask was holding us back; now, it is ready. |
penelopeysm
force-pushed
the
py/kwargs
branch
from
ab20d4d to
fd8fa3f
Compare
|
AdvancedPS.jl documentation for PR #118 is available at: |
penelopeysm
force-pushed
the
py/kwargs
branch
2 times, most recently
from
cf3da3f to
28c9e2e
Compare
penelopeysm
force-pushed
the
py/kwargs
branch
from
28c9e2e to
051012c
Compare
|
Hi @yebai, sorry just tidying this up before I go away :) I am happy to try to work on upstreaming the necessary parts to Turing next year, but that might take more time, and this PR will unblock keyword arguments on SMC/PG on Turing (TuringLang/Turing.jl#2660). So I think it makes sense to merge this first. I tested locally and it all works as intended, so happy for it to be reviewed now (cc also @mhauru). |
penelopeysm
changed the title
AbstractTuringLibtaskModel...)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
=======================================
Coverage ? 95.41%
=======================================
Files ? 8
Lines ? 436
Branches ? 0
=======================================
Hits ? 416
Misses ? 20
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sunxd3
left a comment
sunxd3
left a comment
There was a problem hiding this comment.
The logic looks good.
I am a bit worrying about introducing a struct with "Turing" in the naming (AbstractTuringLibtaskModel) into the namespace. This is purely aesthetic, of course. That said, not a blocker at all since we agree to move these into Turing pretty soon.
Thanks for the work, Penny!
> [!NOTE] > ~~This PR requires some changes to AdvancedPS. TuringLang/AdvancedPS.jl#118 This is merged > > ~~It also needs the following Libtask patch: TuringLang/Libtask.jl#198 This is merged > > ~~This PR also lacks tests; some should be added.~~ Tests added. This PR allows models with keyword arguments to be run with SMC / PG. Example: ```julia julia> using Turing julia> @model function m(y; n=0) x ~ Normal(n) y ~ Normal(x) end m (generic function with 2 methods) julia> mean(sample(m(5.0), PG(20), 1000)) [...] ERROR: Models with keyword arguments need special treatment to be used with particle methods. Please run: using Libtask; Libtask.@might_produce(m) before sampling from this model with particle methods. Stacktrace: [...] julia> using Libtask; Libtask.@might_produce(m) julia> mean(sample(m(5.0), PG(20), 1000)) Sampling 100%|███████████████████████████████████████████████████████████████████| Time: 0:00:05 Mean parameters mean Symbol Float64 x 2.7182 julia> mean(sample(m(5.0; n=10.0), PG(20), 1000)) Sampling 100%|███████████████████████████████████████████████████████████████████| Time: 0:00:04 Mean parameters mean Symbol Float64 x 7.4854 ``` Closes #2007.
3 participants
This PR modifies LibtaskExt to allow sampling from Turing models with keyword arguments with SMC or PG. See TuringLang/Turing.jl#2660 for usage.
It's obviously quite ugly, the problem is that even though Turing is the only library that subtypes the abstract type, the abstract type has to be defined in AdvancedPS so that the LibtaskExt can dispatch on it. Ideally, that abstract type + any relevant methods inside LibtaskExt should just be shifted into Turing proper. That's for another time.