Support proposals of varying dimensions in vector-of-proposals (#120)#121
Support proposals of varying dimensions in vector-of-proposals (#120)#121
Conversation
|
AdvancedMH.jl documentation for PR #121 is available at: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
+ Coverage 90.03% 90.88% +0.85%
==========================================
Files 9 9
Lines 311 340 +29
==========================================
+ Hits 280 309 +29
Misses 31 31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 21801718299Details
💛 - Coveralls |
| mixed-dimension proposal vectors. | ||
| """ | ||
| proposal_dim(p::Proposal{<:UnivariateDistribution}) = 1 | ||
| proposal_dim(::Proposal{<:Function}) = 1 |
There was a problem hiding this comment.
The code in the PR all looks good, and I know I didn't say this in the original issue, but there are still quite a few more blockers to using this upstream in Turing. For example in this line which handles conditional proposals, are we implicitly assuming that the proposal is necessarily conditional on a single element?
Consider e.g.
@model f() = x ~ product_distribution([uni, uni])where uni is some univariate distribution, and we want to propose new values of x based on the value at the previous step. Then we would need something like
StaticProposal(slice -> MvNormal(slice, I))Or even something like a matrix distribution, which we would flatten inside an LDF. I don't immediately know how we can represent this in AdvancedMH where everything has to be flattened. I suppose that we would have to use something like a StaticProposal(vec(matrix_dist)).
Again, I don't think this has to be handled in this PR (if anything I actively think it shouldn't be). To be completely honest, I also don't know how to handle this, short of adding another field to StaticProposal that specifies the length. Just wanted to check that I'm understanding correctly
There was a problem hiding this comment.
Basically the problem is, if I have a high-level construct like
spl = MH(
@varname(x) => xprop,
@varname(y) => yprop,
)how do we translate that into AdvancedMH. From an LDF we can identify the range of indices that each varname belongs to, so as long as we can translate the individual prop's into AdvancedMH proposals, this PR will let us then stack them together, which is great.
But we still need to be able to translate the props and that can depend quite a bit on what prop is, what x's distribution in the model is, etc. For example if x ~ LKJCholesky() and we want to say 'sample x from the prior', that is quite easy to do in Turing because you have access to the distribution. But in AdvancedMH we would need to translate this into 'sample a vectorised form of x from a vectorised form of LKJCholesky'. Then it gets more complex if you have a conditional distribution etc.
There was a problem hiding this comment.
these are tricky interface questions, we'll deal with them when they emerge
on the first comment, my cop out thought is that, if the user is serious about using this type of proposal, they can overload proposal_dim. this is not quite nice, but I am going to leave it there. that said, I added an error message
There was a problem hiding this comment.
fwiw I'm happy to merge and release this!
3 participants
Fix #120
parameters are split according to each proposal's dimension before stepping, and the resulting draws are repacked into a flat vector afterward