MarginalLogDensities extension #1036
Conversation
See: TuringLang/Turing.jl#2664 Co-authored-by: Sam Urmy <[email protected]>
Benchmark Report for Commit aaca138Computer InformationBenchmark Results |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1036 +/- ##
==========================================
+ Coverage 82.34% 82.39% +0.04%
==========================================
Files 38 39 +1
Lines 3949 3960 +11
==========================================
+ Hits 3252 3263 +11
Misses 697 697 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 17500096701Details
💛 - Coveralls |
There was a problem hiding this comment.
@ElOceanografo Here's the updated PR. I got rid of the double negative sign, and added some documentation. If you'd like to have push access to this let me know and I can add you!
| # Determine the indices for the variables to marginalise out. | ||
| varinfo = DynamicPPL.typed_varinfo(model) | ||
| vns = map(_to_varname, varnames) | ||
| varindices = reduce(vcat, DynamicPPL.vector_getranges(varinfo, vns)) |
There was a problem hiding this comment.
It seems to me that in principle MLD should be able to handle this case fine:
using DynamicPPL, Distributions, MarginalLogDensities, LinearAlgebra
@model function f()
x ~ MvNormal([0.0, 1.0, 2.0], I)
end
marginalize(f(), [@varname(x[1])])Unfortunately, attempting to do this now falls over:
ERROR: KeyError: key VarName{:x, Accessors.IndexLens{Tuple{Int64}}}[x[1]] not found
Stacktrace:
[1] vector_getranges(varinfo::VarInfo{@NamedTuple{…}, DynamicPPL.AccumulatorTuple{…}}, vns::Vector{VarName{…}})
@ DynamicPPL ~/ppl/dppl/src/varinfo.jl:732This is definitely a limitation of using the internal vector_getranges function. I wonder if maybe we should explicitly allow the user to specify the indices that they want to remove?
| # Use linked `varinfo` to that we're working in unconstrained space | ||
| varinfo_linked = DynamicPPL.link(varinfo, model) |
There was a problem hiding this comment.
Does MLD require a linked VarInfo, or would an unlinked VarInfo be fine?
The reason I'm thinking about this is because if the VarInfo is linked, then all the parameters that are later supplied must be in linked space, which is potentially a bit confusing (though nothing that can't be fixed by documentation). Example:
julia> using DynamicPPL, Distributions, Bijectors, MarginalLogDensities
julia> @model function f()
x ~ Normal()
y ~ Beta(2, 2)
end
f (generic function with 2 methods)
julia> m = marginalize(f(), [@varname(x)]);
julia> m([0.5]) # this 0.5 is in linked space
0.3436055008678415
julia> logpdf(Beta(2, 2), 0.5) # this 0.5 is unlinked, so logp is wrong
0.4054651081081644
julia> inverse(Bijectors.bijector(Beta(2, 2)))(0.5) # this is the unlinked value corresponding to 0.5
0.6224593312018546
julia> logpdf(Beta(2, 2), 0.6224593312018546) # now logp matches
0.3436055008678416If an unlinked VarInfo is acceptable, then the choice of varinfo should probably also be added as an argument to marginalize.
|
DynamicPPL.jl documentation for PR #1036 is available at: |
Transferred from TuringLang/Turing.jl#2664.