[Merged by Bors] - Condition with Dict as underlying storage
#419
Conversation
these to simplify conditioning code too
…ifferent behvaior on different julia versions
src/model.jl
Outdated
| julia> deconditioned_model(rng) # (×) `m[1]` is still conditioned | ||
| 2-element Vector{Float64}: | ||
| 1.0 | ||
| 2.0 |
There was a problem hiding this comment.
Note that we could actually address this by introducing a DeconditionContext (can we maybe unify these?) that we'd fall back to if we can't locate the corresponding variable in existing ConditionContext
Co-authored-by: David Widmann <[email protected]>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: David Widmann <[email protected]>
Co-authored-by: David Widmann <[email protected]>
|
So I made an attempt at implementing the Gibbs sampler using We can address this, and I think there's an argument to be made for doing this anyways just to make sure that we see the same varnames everywhere. We have to make Lines 451 to 453 in 0ba86e2 instead use the "unwrapped" varnames that are passed to the tilde-pipeline Lines 483 to 485 in 0ba86e2 We should still have the same zero runtime overhead because the resulting varnames should be inferred correctly to The only issue (but this is an issue nonetheless): |
|
@devmotion would you be able to have another look at this now, or you still on vacation? |
|
bors try |
yebai
left a comment
yebai
left a comment
There was a problem hiding this comment.
Thanks, @torfjelde, @devmotion - nice generalisation of ConditionContext!
|
|
||
| return canview(child, value) | ||
| end | ||
| Base.haskey(vi::SimpleVarInfo, vn::VarName) = nested_haskey(vi.values, vn) |
| 2.0 | ||
| julia> # (✓) this works though | ||
| deconditioned_model_2 = deconditioned_model | (@varname(m[1]) => missing); |
There was a problem hiding this comment.
Clarification question: would model_missing = model | (@varname(m[1]) => missing) work similiarly (i.e. m = [missing, 2.0])?
| Return `true` if `vn` is found in `context`. | ||
| """ | ||
| hasvalue(context, vn) = false |
There was a problem hiding this comment.
I feel that hasvalue should be unified with haskey. But perhaps this can be fixed in a future PR.
|
bors r+ |
This PR allows usage of `Dict` as the underlying storage in addition to the currently supported `NamedTuple`. Similarly to `SimpleVarInfo`, this gives us two approaches: one with somewhat limited support, as outlined in the docstring, but with (usually) zero runtime overhead (`NamedTuple`), and one with full support but with runtime overead (`Dict`).
It sounds good to me. We can probably open a PR to discuss this further. |
|
Pull request successfully merged into master. Build succeeded: |
bors
bot
changed the title
Dict as underlying storageDict as underlying storage
This PR allows usage of
Dictas the underlying storage in addition to the currently supportedNamedTuple.Similarly to
SimpleVarInfo, this gives us two approaches: one with somewhat limited support, as outlined in the docstring, but with (usually) zero runtime overhead (NamedTuple), and one with full support but with runtime overead (Dict).