Skip to content

Commit ff5f2cb

Browse files
penelopeysmyebaimhauru
authored
Fix condition and fix in submodels (#892)
* Fix conditioning in submodels * Simplify contextual_isassumption * Add documentation * Fix some tests * Add tests; fix a bunch of nested submodel issues * Fix fix as well * Fix doctests * Add unit tests for new functions * Add changelog entry * Update changelog Co-authored-by: Hong Ge <[email protected]> * Finish docs * Add a test for conditioning submodel via arguments * Clean new tests up a bit * Fix for VarNames with non-identity lenses * Apply suggestions from code review Co-authored-by: Markus Hauru <[email protected]> * Apply suggestions from code review * Make PrefixContext contain a varname rather than symbol (#896) --------- Co-authored-by: Hong Ge <[email protected]> Co-authored-by: Markus Hauru <[email protected]>
1 parent be27636 commit ff5f2cb

14 files changed

+1109
-234
lines changed

‎HISTORY.md

+69-29
Original file line numberDiff line numberDiff line change
@@ -4,38 +4,25 @@
44

55
**Breaking changes**
66

7-
### AD testing utilities
8-
9-
`DynamicPPL.TestUtils.AD.run_ad` now links the VarInfo by default.
10-
To disable this, pass the `linked=false` keyword argument.
11-
If the calculated value or gradient is incorrect, it also throws a `DynamicPPL.TestUtils.AD.ADIncorrectException` rather than a test failure.
12-
This exception contains the actual and expected gradient so you can inspect it if needed; see the documentation for more information.
13-
From a practical perspective, this means that if you need to add this to a test suite, you need to use `@test run_ad(...) isa Any` rather than just `run_ad(...)`.
7+
### Submodels: conditioning
148

15-
### SimpleVarInfo linking / invlinking
16-
17-
Linking a linked SimpleVarInfo, or invlinking an unlinked SimpleVarInfo, now displays a warning instead of an error.
9+
Variables in a submodel can now be conditioned and fixed in a correct way.
10+
See https://github.com/TuringLang/DynamicPPL.jl/issues/857 for a full illustration, but essentially it means you can now do this:
1811

19-
### VarInfo constructors
20-
21-
`VarInfo(vi::VarInfo, values)` has been removed. You can replace this directly with `unflatten(vi, values)` instead.
22-
23-
The `metadata` argument to `VarInfo([rng, ]model[, sampler, context, metadata])` has been removed.
24-
If you were not using this argument (most likely), then there is no change needed.
25-
If you were using the `metadata` argument to specify a blank `VarNamedVector`, then you should replace calls to `VarInfo` with `DynamicPPL.typed_vector_varinfo` instead (see 'Other changes' below).
26-
27-
The `UntypedVarInfo` constructor and type is no longer exported.
28-
If you needed to construct one, you should now use `DynamicPPL.untyped_varinfo` instead.
29-
30-
The `TypedVarInfo` constructor and type is no longer exported.
31-
The _type_ has been replaced with `DynamicPPL.NTVarInfo`.
32-
The _constructor_ has been replaced with `DynamicPPL.typed_varinfo`.
12+
```julia
13+
@model function inner()
14+
x ~ Normal()
15+
return y ~ Normal()
16+
end
17+
@model function outer()
18+
return a ~ to_submodel(inner() | (x=1.0,))
19+
end
20+
```
3321

34-
Note that the exact kind of VarInfo returned by `VarInfo(rng, model, ...)` is an implementation detail.
35-
Previously, it was guaranteed that this would always be a VarInfo whose metadata was a `NamedTuple` containing `Metadata` structs.
36-
Going forward, this is no longer the case, and you should only assume that the returned object obeys the `AbstractVarInfo` interface.
22+
and the `a.x` variable will be correctly conditioned.
23+
(Previously, you would have to condition `inner()` with the variable `a.x`, meaning that you would need to know what prefix to use before you had actually prefixed it.)
3724

38-
### VarName prefixing behaviour
25+
### Submodel prefixing
3926

4027
The way in which VarNames in submodels are prefixed has been changed.
4128
This is best explained through an example.
@@ -77,9 +64,62 @@ outer() | (@varname(var"a.x") => 1.0,)
7764
outer() | (a.x=1.0,)
7865
```
7966

80-
If you are sampling from a model with submodels, this doesn't affect the way you interact with the `MCMCChains.Chains` object, because VarNames are converted into Symbols when stored in the chain.
67+
In a similar way, if the variable on the left-hand side of your tilde statement is not just a single identifier, any fields or indices it accesses are now properly respected.
68+
Consider the following setup:
69+
70+
```julia
71+
using DynamicPPL, Distributions
72+
@model inner() = x ~ Normal()
73+
@model function outer()
74+
a = Vector{Float64}(undef, 1)
75+
a[1] ~ to_submodel(inner())
76+
return a
77+
end
78+
```
79+
80+
In this case, the variable sampled is actually the `x` field of the first element of `a`:
81+
82+
```julia
83+
julia> only(keys(VarInfo(outer()))) == @varname(a[1].x)
84+
true
85+
```
86+
87+
Before this version, it used to be a single variable called `var"a[1].x"`.
88+
89+
Note that if you are sampling from a model with submodels, this doesn't affect the way you interact with the `MCMCChains.Chains` object, because VarNames are converted into Symbols when stored in the chain.
8190
(This behaviour will likely be changed in the future, in that Chains should be indexable by VarNames and not just Symbols, but that has not been implemented yet.)
8291

92+
### AD testing utilities
93+
94+
`DynamicPPL.TestUtils.AD.run_ad` now links the VarInfo by default.
95+
To disable this, pass the `linked=false` keyword argument.
96+
If the calculated value or gradient is incorrect, it also throws a `DynamicPPL.TestUtils.AD.ADIncorrectException` rather than a test failure.
97+
This exception contains the actual and expected gradient so you can inspect it if needed; see the documentation for more information.
98+
From a practical perspective, this means that if you need to add this to a test suite, you need to use `@test run_ad(...) isa Any` rather than just `run_ad(...)`.
99+
100+
### SimpleVarInfo linking / invlinking
101+
102+
Linking a linked SimpleVarInfo, or invlinking an unlinked SimpleVarInfo, now displays a warning instead of an error.
103+
104+
### VarInfo constructors
105+
106+
`VarInfo(vi::VarInfo, values)` has been removed. You can replace this directly with `unflatten(vi, values)` instead.
107+
108+
The `metadata` argument to `VarInfo([rng, ]model[, sampler, context, metadata])` has been removed.
109+
If you were not using this argument (most likely), then there is no change needed.
110+
If you were using the `metadata` argument to specify a blank `VarNamedVector`, then you should replace calls to `VarInfo` with `DynamicPPL.typed_vector_varinfo` instead (see 'Other changes' below).
111+
112+
The `UntypedVarInfo` constructor and type is no longer exported.
113+
If you needed to construct one, you should now use `DynamicPPL.untyped_varinfo` instead.
114+
115+
The `TypedVarInfo` constructor and type is no longer exported.
116+
The _type_ has been replaced with `DynamicPPL.NTVarInfo`.
117+
The _constructor_ has been replaced with `DynamicPPL.typed_varinfo`.
118+
119+
Note that the exact kind of VarInfo returned by `VarInfo(rng, model, ...)` is an implementation detail.
120+
Previously, it was guaranteed that this would always be a VarInfo whose metadata was a `NamedTuple` containing `Metadata` structs.
121+
Going forward, this is no longer the case, and you should only assume that the returned object obeys the `AbstractVarInfo` interface.
122+
83123
**Other changes**
84124

85125
While these are technically breaking, they are only internal changes and do not affect the public API.

‎docs/Project.toml

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
[deps]
2+
AbstractPPL = "7a57a42e-76ec-4ea3-a279-07e840d6d9cf"
23
Accessors = "7d9f7c33-5ae7-4f3b-8dc6-eff91059b697"
34
DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"
45
Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"

‎docs/make.jl

+3-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ makedocs(;
2424
format=Documenter.HTML(; size_threshold=2^10 * 400),
2525
modules=[DynamicPPL, Base.get_extension(DynamicPPL, :DynamicPPLMCMCChainsExt)],
2626
pages=[
27-
"Home" => "index.md", "API" => "api.md", "Internals" => ["internals/varinfo.md"]
27+
"Home" => "index.md",
28+
"API" => "api.md",
29+
"Internals" => ["internals/varinfo.md", "internals/submodel_condition.md"],
2830
],
2931
checkdocs=:exports,
3032
doctest=false,

‎docs/src/api.md

+7-7
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ decondition
7878

7979
## Fixing and unfixing
8080

81-
We can also _fix_ a collection of variables in a [`Model`](@ref) to certain using [`fix`](@ref).
81+
We can also _fix_ a collection of variables in a [`Model`](@ref) to certain values using [`DynamicPPL.fix`](@ref).
8282

83-
This might seem quite similar to the aforementioned [`condition`](@ref) and its siblings,
83+
This is quite similar to the aforementioned [`condition`](@ref) and its siblings,
8484
but they are indeed different operations:
8585

8686
- `condition`ed variables are considered to be _observations_, and are thus
@@ -89,19 +89,19 @@ but they are indeed different operations:
8989
- `fix`ed variables are considered to be _constant_, and are thus not included
9090
in any log-probability computations.
9191

92-
The differences are more clearly spelled out in the docstring of [`fix`](@ref) below.
92+
The differences are more clearly spelled out in the docstring of [`DynamicPPL.fix`](@ref) below.
9393

9494
```@docs
95-
fix
95+
DynamicPPL.fix
9696
DynamicPPL.fixed
9797
```
9898

99-
The difference between [`fix`](@ref) and [`condition`](@ref) is described in the docstring of [`fix`](@ref) above.
99+
The difference between [`DynamicPPL.fix`](@ref) and [`DynamicPPL.condition`](@ref) is described in the docstring of [`DynamicPPL.fix`](@ref) above.
100100

101-
Similarly, we can [`unfix`](@ref) variables, i.e. return them to their original meaning:
101+
Similarly, we can revert this with [`DynamicPPL.unfix`](@ref), i.e. return the variables to their original meaning:
102102

103103
```@docs
104-
unfix
104+
DynamicPPL.unfix
105105
```
106106

107107
## Predicting

0 commit comments

Comments
 (0)