Globally rename sample!! -> evaluate_and_sample!!, add changelog warning

Author: penelopeysm

HISTORY.md

@@ -35,8 +35,9 @@ To aid with this process, `contextualize` is now exported from DynamicPPL.
 
 The main situation where one _did_ want to specify an additional evaluation context was when that context was a `SamplingContext`.
 Doing this would allow you to run the model and sample fresh values, instead of just using the values that existed in the VarInfo object.
-Thus, this release also introduces the unexported function `sample!!`.
-Essentially, `sample!!(rng, model, varinfo, sampler)` is a drop-in replacement for `evaluate!!(model, varinfo, SamplingContext(rng, sampler))`.
+Thus, this release also introduces the **unexported** function `evaluate_and_sample!!`.
+Essentially, `evaluate_and_sample!!(rng, model, varinfo, sampler)` is a drop-in replacement for `evaluate!!(model, varinfo, SamplingContext(rng, sampler))`.
+**Do note that this is an internal method**, and its name or semantics are liable to change in the future without warning.
 
 There are many methods that no longer take a context argument, and listing them all would be too much.
 However, here are the more user-facing ones:
@@ -50,7 +51,7 @@ However, here are the more user-facing ones:
 And a couple of more internal changes:
 
   - `evaluate!!`, `evaluate_threadsafe!!`, and `evaluate_threadunsafe!!` no longer accept context arguments
-  - `evaluate!!` no longer takes rng and sampler (if you used this, you should use `sample!!` instead, or construct your own `SamplingContext`)
+  - `evaluate!!` no longer takes rng and sampler (if you used this, you should use `evaluate_and_sample!!` instead, or construct your own `SamplingContext`)
   - The model evaluation function, `model.f` for some `model::Model`, no longer takes a context as an argument
 
 ## 0.36.12

docs/src/api.md

@@ -455,7 +455,7 @@ By default, it does not perform any actual sampling: it only evaluates the model
 To perform sampling, you can either wrap `model.context` in a `SamplingContext`, or use this convenience method:
 
 ```@docs
-DynamicPPL.sample!!
+DynamicPPL.evaluate_and_sample!!
 ```
 
 The behaviour of a model execution can be changed with evaluation contexts, which are a field of the model.

ext/DynamicPPLMCMCChainsExt.jl

@@ -115,7 +115,7 @@ function DynamicPPL.predict(
     iters = Iterators.product(1:size(chain, 1), 1:size(chain, 3))
     predictive_samples = map(iters) do (sample_idx, chain_idx)
         DynamicPPL.setval_and_resample!(varinfo, parameter_only_chain, sample_idx, chain_idx)
-        varinfo = last(DynamicPPL.sample!!(rng, model, varinfo))
+        varinfo = last(DynamicPPL.evaluate_and_sample!!(rng, model, varinfo))
 
         vals = DynamicPPL.values_as_in_model(model, false, varinfo)
         varname_vals = mapreduce(

src/extract_priors.jl

@@ -116,7 +116,7 @@ function extract_priors(rng::Random.AbstractRNG, model::Model)
     # workaround for the fact that `order` is still hardcoded in VarInfo, and hence you
     # can't push new variables without knowing the num_produce. Remove this when possible.
     varinfo = setaccs!!(varinfo, (PriorDistributionAccumulator(), NumProduceAccumulator()))
-    varinfo = last(sample!!(rng, model, varinfo))
+    varinfo = last(evaluate_and_sample!!(rng, model, varinfo))
     return getacc(varinfo, Val(:PriorDistributionAccumulator)).priors
 end
 

src/model.jl

@@ -815,7 +815,7 @@ end
 # ^ Weird Documenter.jl bug means that we have to write the two above separately
 # as it can only detect the `function`-less syntax.
 function (model::Model)(rng::Random.AbstractRNG, varinfo::AbstractVarInfo=VarInfo())
-    return first(sample!!(rng, model, varinfo))
+    return first(evaluate_and_sample!!(rng, model, varinfo))
 end
 
 """
@@ -829,7 +829,7 @@ function use_threadsafe_eval(context::AbstractContext, varinfo::AbstractVarInfo)
 end
 
 """
-    sample!!([rng::Random.AbstractRNG, ]model::Model, varinfo[, sampler])
+    evaluate_and_sample!!([rng::Random.AbstractRNG, ]model::Model, varinfo[, sampler])
 
 Evaluate the `model` with the given `varinfo`, but perform sampling during the
 evaluation using the given `sampler` by wrapping the model's context in a
@@ -839,7 +839,7 @@ If `sampler` is not provided, defaults to [`SampleFromPrior`](@ref).
 
 Returns a tuple of the model's return value, plus the updated `varinfo` object.
 """
-function sample!!(
+function evaluate_and_sample!!(
     rng::Random.AbstractRNG,
     model::Model,
     varinfo::AbstractVarInfo,
@@ -848,10 +848,10 @@ function sample!!(
     sampling_model = contextualize(model, SamplingContext(rng, sampler, model.context))
     return evaluate!!(sampling_model, varinfo)
 end
-function sample!!(
+function evaluate_and_sample!!(
     model::Model, varinfo::AbstractVarInfo, sampler::AbstractSampler=SampleFromPrior()
 )
-    return sample!!(Random.default_rng(), model, varinfo, sampler)
+    return evaluate_and_sample!!(Random.default_rng(), model, varinfo, sampler)
 end
 
 """
@@ -1038,7 +1038,7 @@ Base.nameof(model::Model{<:Function}) = nameof(model.f)
 Generate a sample of type `T` from the prior distribution of the `model`.
 """
 function Base.rand(rng::Random.AbstractRNG, ::Type{T}, model::Model) where {T}
-    x = last(sample!!(rng, model, SimpleVarInfo{Float64}(OrderedDict())))
+    x = last(evaluate_and_sample!!(rng, model, SimpleVarInfo{Float64}(OrderedDict())))
     return values_as(x, T)
 end
 

src/sampler.jl

@@ -58,7 +58,7 @@ function AbstractMCMC.step(
     kwargs...,
 )
     vi = VarInfo()
-    DynamicPPL.sample!!(rng, model, vi, sampler)
+    DynamicPPL.evaluate_and_sample!!(rng, model, vi, sampler)
     return vi, nothing
 end
 

src/simple_varinfo.jl

@@ -39,7 +39,7 @@ julia> rng = StableRNG(42);
 julia> # In the `NamedTuple` version we need to provide the place-holder values for
        # the variables which are using "containers", e.g. `Array`.
        # In this case, this means that we need to specify `x` but not `m`.
-       _, vi = DynamicPPL.sample!!(rng, m, SimpleVarInfo((x = ones(2), )));
+       _, vi = DynamicPPL.evaluate_and_sample!!(rng, m, SimpleVarInfo((x = ones(2), )));
 
 julia> # (✓) Vroom, vroom! FAST!!!
        vi[@varname(x[1])]
@@ -57,12 +57,12 @@ julia> vi[@varname(x[1:2])]
  1.3736306979834252
 
 julia> # (×) If we don't provide the container...
-       _, vi = DynamicPPL.sample!!(rng, m, SimpleVarInfo()); vi
+       _, vi = DynamicPPL.evaluate_and_sample!!(rng, m, SimpleVarInfo()); vi
 ERROR: type NamedTuple has no field x
 [...]
 
 julia> # If one does not know the varnames, we can use a `OrderedDict` instead.
-       _, vi = DynamicPPL.sample!!(rng, m, SimpleVarInfo{Float64}(OrderedDict()));
+       _, vi = DynamicPPL.evaluate_and_sample!!(rng, m, SimpleVarInfo{Float64}(OrderedDict()));
 
 julia> # (✓) Sort of fast, but only possible at runtime.
        vi[@varname(x[1])]
@@ -91,28 +91,28 @@ demo_constrained (generic function with 2 methods)
 
 julia> m = demo_constrained();
 
-julia> _, vi = DynamicPPL.sample!!(rng, m, SimpleVarInfo());
+julia> _, vi = DynamicPPL.evaluate_and_sample!!(rng, m, SimpleVarInfo());
 
 julia> vi[@varname(x)] # (✓) 0 ≤ x < ∞
 1.8632965762164932
 
-julia> _, vi = DynamicPPL.sample!!(rng, m, DynamicPPL.settrans!!(SimpleVarInfo(), true));
+julia> _, vi = DynamicPPL.evaluate_and_sample!!(rng, m, DynamicPPL.settrans!!(SimpleVarInfo(), true));
 
 julia> vi[@varname(x)] # (✓) -∞ < x < ∞
 -0.21080155351918753
 
-julia> xs = [last(DynamicPPL.sample!!(rng, m, DynamicPPL.settrans!!(SimpleVarInfo(), true)))[@varname(x)] for i = 1:10];
+julia> xs = [last(DynamicPPL.evaluate_and_sample!!(rng, m, DynamicPPL.settrans!!(SimpleVarInfo(), true)))[@varname(x)] for i = 1:10];
 
 julia> any(xs .< 0)  # (✓) Positive probability mass on negative numbers!
 true
 
 julia> # And with `OrderedDict` of course!
-       _, vi = DynamicPPL.sample!!(rng, m, DynamicPPL.settrans!!(SimpleVarInfo(OrderedDict()), true));
+       _, vi = DynamicPPL.evaluate_and_sample!!(rng, m, DynamicPPL.settrans!!(SimpleVarInfo(OrderedDict()), true));
 
 julia> vi[@varname(x)] # (✓) -∞ < x < ∞
 0.6225185067787314
 
-julia> xs = [last(DynamicPPL.sample!!(rng, m, DynamicPPL.settrans!!(SimpleVarInfo(), true)))[@varname(x)] for i = 1:10];
+julia> xs = [last(DynamicPPL.evaluate_and_sample!!(rng, m, DynamicPPL.settrans!!(SimpleVarInfo(), true)))[@varname(x)] for i = 1:10];
 
 julia> any(xs .< 0) # (✓) Positive probability mass on negative numbers!
 true
@@ -259,12 +259,12 @@ end
 
 function untyped_simple_varinfo(model::Model)
     varinfo = SimpleVarInfo(OrderedDict())
-    return last(sample!!(model, varinfo))
+    return last(evaluate_and_sample!!(model, varinfo))
 end
 
 function typed_simple_varinfo(model::Model)
     varinfo = SimpleVarInfo{Float64}()
-    return last(sample!!(model, varinfo))
+    return last(evaluate_and_sample!!(model, varinfo))
 end
 
 function unflatten(svi::SimpleVarInfo, x::AbstractVector)

src/test_utils/model_interface.jl

@@ -92,7 +92,9 @@ Even though it is recommended to implement this by hand for a particular `Model`
 a default implementation using [`SimpleVarInfo{<:Dict}`](@ref) is provided.
 """
 function varnames(model::Model)
-    return collect(keys(last(DynamicPPL.sample!!(model, SimpleVarInfo(Dict())))))
+    return collect(
+        keys(last(DynamicPPL.evaluate_and_sample!!(model, SimpleVarInfo(Dict()))))
+    )
 end
 
 """

src/varinfo.jl

@@ -197,7 +197,7 @@ Construct a VarInfo object for the given `model`, which has just a single
 function untyped_varinfo(
     rng::Random.AbstractRNG, model::Model, sampler::AbstractSampler=SampleFromPrior()
 )
-    return last(sample!!(rng, model, VarInfo(Metadata()), sampler))
+    return last(evaluate_and_sample!!(rng, model, VarInfo(Metadata()), sampler))
 end
 function untyped_varinfo(model::Model, sampler::AbstractSampler=SampleFromPrior())
     return untyped_varinfo(Random.default_rng(), model, sampler)

test/compiler.jl

@@ -598,13 +598,13 @@ module Issue537 end
         # an attempt at a `NamedTuple` of the form `(x = 1, __varinfo__)`.
         @model empty_model() = return x = 1
         empty_vi = VarInfo()
-        retval_and_vi = DynamicPPL.sample!!(empty_model(), empty_vi)
+        retval_and_vi = DynamicPPL.evaluate_and_sample!!(empty_model(), empty_vi)
         @test retval_and_vi isa Tuple{Int,typeof(empty_vi)}
 
         # Even if the return-value is `AbstractVarInfo`, we should return
         # a `Tuple` with `AbstractVarInfo` in the second component too.
         @model demo() = return __varinfo__
-        retval, svi = DynamicPPL.sample!!(demo(), SimpleVarInfo())
+        retval, svi = DynamicPPL.evaluate_and_sample!!(demo(), SimpleVarInfo())
         @test svi == SimpleVarInfo()
         if Threads.nthreads() > 1
             @test retval isa DynamicPPL.ThreadSafeVarInfo{<:SimpleVarInfo}
@@ -620,11 +620,11 @@ module Issue537 end
             f(x) = return x^2
             return f(1.0)
         end
-        retval, svi = DynamicPPL.sample!!(demo(), SimpleVarInfo())
+        retval, svi = DynamicPPL.evaluate_and_sample!!(demo(), SimpleVarInfo())
         @test retval isa Float64
 
         @model demo() = x ~ Normal()
-        retval, svi = DynamicPPL.sample!!(demo(), SimpleVarInfo())
+        retval, svi = DynamicPPL.evaluate_and_sample!!(demo(), SimpleVarInfo())
 
         # Return-value when using `to_submodel`
         @model inner() = x ~ Normal()

test/model.jl

@@ -162,12 +162,12 @@ const GDEMO_DEFAULT = DynamicPPL.TestUtils.demo_assume_observe_literal()
             for i in 1:10
                 Random.seed!(100 + i)
                 vi = VarInfo()
-                DynamicPPL.sample!!(Random.default_rng(), model, vi, sampler)
+                DynamicPPL.evaluate_and_sample!!(Random.default_rng(), model, vi, sampler)
                 vals = vi[:]
 
                 Random.seed!(100 + i)
                 vi = VarInfo()
-                DynamicPPL.sample!!(Random.default_rng(), model, vi, sampler)
+                DynamicPPL.evaluate_and_sample!!(Random.default_rng(), model, vi, sampler)
                 @test vi[:] == vals
             end
         end
@@ -332,7 +332,7 @@ const GDEMO_DEFAULT = DynamicPPL.TestUtils.demo_assume_observe_literal()
             @test logjoint(model, x) !=
                 DynamicPPL.TestUtils.logjoint_true_with_logabsdet_jacobian(model, x...)
             # Ensure `varnames` is implemented.
-            vi = last(DynamicPPL.sample!!(model, SimpleVarInfo(OrderedDict())))
+            vi = last(DynamicPPL.evaluate_and_sample!!(model, SimpleVarInfo(OrderedDict())))
             @test all(collect(keys(vi)) .== DynamicPPL.TestUtils.varnames(model))
             # Ensure `posterior_mean` is implemented.
             @test DynamicPPL.TestUtils.posterior_mean(model) isa typeof(x)
@@ -591,7 +591,10 @@ const GDEMO_DEFAULT = DynamicPPL.TestUtils.demo_assume_observe_literal()
             xs_train = 1:0.1:10
             ys_train = ground_truth_β .* xs_train + rand(Normal(0, 0.1), length(xs_train))
             m_lin_reg = linear_reg(xs_train, ys_train)
-            chain = [last(DynamicPPL.sample!!(m_lin_reg, VarInfo())) for _ in 1:10000]
+            chain = [
+                last(DynamicPPL.evaluate_and_sample!!(m_lin_reg, VarInfo())) for
+                _ in 1:10000
+            ]
 
             # chain is generated from the prior
             @test mean([chain[i][@varname(β)] for i in eachindex(chain)]) ≈ 1.0 atol = 0.1

test/simple_varinfo.jl

@@ -158,7 +158,7 @@
 
             ### Sampling ###
             # Sample a new varinfo!
-            _, svi_new = DynamicPPL.sample!!(model, svi)
+            _, svi_new = DynamicPPL.evaluate_and_sample!!(model, svi)
 
             # Realization for `m` should be different wp. 1.
             for vn in DynamicPPL.TestUtils.varnames(model)
@@ -226,9 +226,9 @@
 
         # Initialize.
         svi_nt = DynamicPPL.settrans!!(SimpleVarInfo(), true)
-        svi_nt = last(DynamicPPL.sample!!(model, svi_nt))
+        svi_nt = last(DynamicPPL.evaluate_and_sample!!(model, svi_nt))
         svi_vnv = DynamicPPL.settrans!!(SimpleVarInfo(DynamicPPL.VarNamedVector()), true)
-        svi_vnv = last(DynamicPPL.sample!!(model, svi_vnv))
+        svi_vnv = last(DynamicPPL.evaluate_and_sample!!(model, svi_vnv))
 
         for svi in (svi_nt, svi_vnv)
             # Sample with large variations in unconstrained space.
@@ -273,7 +273,7 @@
             )
 
             # Resulting varinfo should no longer be transformed.
-            vi_result = last(DynamicPPL.sample!!(model, deepcopy(vi)))
+            vi_result = last(DynamicPPL.evaluate_and_sample!!(model, deepcopy(vi)))
             @test !DynamicPPL.istrans(vi_result)
 
             # Set the values to something that is out of domain if we're in constrained space.

test/varinfo.jl

@@ -498,7 +498,7 @@ end
         # without having to change the VarInfo object.
         vi = VarInfo()
         meta = vi.metadata
-        _, vi = DynamicPPL.sample!!(model, vi, SampleFromUniform())
+        _, vi = DynamicPPL.evaluate_and_sample!!(model, vi, SampleFromUniform())
         @test all(x -> !istrans(vi, x), meta.vns)
 
         # Check that linking and invlinking set the `trans` flag accordingly
@@ -572,7 +572,7 @@ end
         vi = DynamicPPL.untyped_varinfo(model)
         vi = DynamicPPL.settrans!!(vi, true, vn)
         # Sample in unconstrained space.
-        vi = last(DynamicPPL.sample!!(model, vi))
+        vi = last(DynamicPPL.evaluate_and_sample!!(model, vi))
         f = DynamicPPL.from_linked_internal_transform(vi, vn, dist)
         x = f(DynamicPPL.getindex_internal(vi, vn))
         @test getlogjoint(vi) ≈ Bijectors.logpdf_with_trans(dist, x, true)
@@ -581,7 +581,7 @@ end
         vi = DynamicPPL.typed_varinfo(model)
         vi = DynamicPPL.settrans!!(vi, true, vn)
         # Sample in unconstrained space.
-        vi = last(DynamicPPL.sample!!(model, vi))
+        vi = last(DynamicPPL.evaluate_and_sample!!(model, vi))
         f = DynamicPPL.from_linked_internal_transform(vi, vn, dist)
         x = f(DynamicPPL.getindex_internal(vi, vn))
         @test getlogjoint(vi) ≈ Bijectors.logpdf_with_trans(dist, x, true)
@@ -590,7 +590,7 @@ end
         vi = DynamicPPL.typed_varinfo(model)
         vi = DynamicPPL.settrans!!(vi, true, vn)
         # Sample in unconstrained space.
-        vi = last(DynamicPPL.sample!!(model, vi))
+        vi = last(DynamicPPL.evaluate_and_sample!!(model, vi))
         f = DynamicPPL.from_linked_internal_transform(vi, vn, dist)
         x = f(DynamicPPL.getindex_internal(vi, vn))
         @test getlogjoint(vi) ≈ Bijectors.logpdf_with_trans(dist, x, true)
@@ -599,23 +599,23 @@ end
         ## `SimpleVarInfo{<:NamedTuple}`
         vi = DynamicPPL.settrans!!(SimpleVarInfo(), true)
         # Sample in unconstrained space.
-        vi = last(DynamicPPL.sample!!(model, vi))
+        vi = last(DynamicPPL.evaluate_and_sample!!(model, vi))
         f = DynamicPPL.from_linked_internal_transform(vi, vn, dist)
         x = f(DynamicPPL.getindex_internal(vi, vn))
         @test getlogjoint(vi) ≈ Bijectors.logpdf_with_trans(dist, x, true)
 
         ## `SimpleVarInfo{<:Dict}`
         vi = DynamicPPL.settrans!!(SimpleVarInfo(Dict()), true)
         # Sample in unconstrained space.
-        vi = last(DynamicPPL.sample!!(model, vi))
+        vi = last(DynamicPPL.evaluate_and_sample!!(model, vi))
         f = DynamicPPL.from_linked_internal_transform(vi, vn, dist)
         x = f(DynamicPPL.getindex_internal(vi, vn))
         @test getlogjoint(vi) ≈ Bijectors.logpdf_with_trans(dist, x, true)
 
         ## `SimpleVarInfo{<:VarNamedVector}`
         vi = DynamicPPL.settrans!!(SimpleVarInfo(DynamicPPL.VarNamedVector()), true)
         # Sample in unconstrained space.
-        vi = last(DynamicPPL.sample!!(model, vi))
+        vi = last(DynamicPPL.evaluate_and_sample!!(model, vi))
         f = DynamicPPL.from_linked_internal_transform(vi, vn, dist)
         x = f(DynamicPPL.getindex_internal(vi, vn))
         @test getlogjoint(vi) ≈ Bijectors.logpdf_with_trans(dist, x, true)
@@ -1000,7 +1000,7 @@ end
         # Sampling from `model2` should hit the `istrans(vi) == true` branches
         # because all the existing variables are linked.
         model2 = demo(2)
-        varinfo2 = last(DynamicPPL.sample!!(model2, deepcopy(varinfo1)))
+        varinfo2 = last(DynamicPPL.evaluate_and_sample!!(model2, deepcopy(varinfo1)))
         for vn in [@varname(x[1]), @varname(x[2])]
             @test DynamicPPL.istrans(varinfo2, vn)
         end
@@ -1019,7 +1019,7 @@ end
         # Sampling from `model2` should hit the `istrans(vi) == true` branches
         # because all the existing variables are linked.
         model2 = demo_dot(2)
-        varinfo2 = last(DynamicPPL.sample!!(model2, deepcopy(varinfo1)))
+        varinfo2 = last(DynamicPPL.evaluate_and_sample!!(model2, deepcopy(varinfo1)))
         for vn in [@varname(x), @varname(y[1])]
             @test DynamicPPL.istrans(varinfo2, vn)
         end

test/varnamedvector.jl

@@ -610,7 +610,9 @@ end
             DynamicPPL.TestUtils.test_values(varinfo_eval, value_true, vns)
 
             # Is sampling correct?
-            varinfo_sample = last(DynamicPPL.sample!!(model, deepcopy(varinfo)))
+            varinfo_sample = last(
+                DynamicPPL.evaluate_and_sample!!(model, deepcopy(varinfo))
+            )
             # Log density should be different.
             @test getlogjoint(varinfo_sample) != getlogjoint(varinfo)
             # Values should be different.