Fix deprecated Gibbs constructors. Add HISTORY entry.

Author: mhauru

HISTORY.md

@@ -1,3 +1,19 @@
+# Release 0.35.0
+
+## Breaking changes
+
+0.35.0 introduces a new Gibbs sampler. It's been included in several previous releases as `Turing.Experimental.Gibbs`, but now takes over the old Gibbs sampler, which gets removed completely.
+
+The new Gibbs sampler supports the same user-facing interface as the old one. However, given
+that the internals of it having been completely rewritten in a very different manner, there
+may be accidental breakage that we haven't anticipated. Please report any you find.
+
+`GibbsConditional` has also been removed. It was never very user-facing, but it was exported, so technically this is breaking.
+
+The old Gibbs constructor relied on being called with several subsamplers, and each of the constructors of the subsamplers would take as arguments the symbols for the variables that they are to sample, e.g. `Gibbs(HMC(:x), MH(:y))`. This constructor has been deprecated, and will be removed in the future. The new constructor works by assigning samplers to either symbols or `VarNames`, e.g. `Gibbs(; x=HMC(), y=MH())` or `Gibbs(@varname(x) => HMC(), @varname(y) => MH())`. This allows more granular specification of which sampler to use for which variable.
+
+Likewise, the old constructor for calling one subsampler more often than another, `Gibbs((HMC(:x), 2), (MH(:y), 1))` has been deprecated. The new way to achieve this effect is to list the same sampler multiple times, e.g. as `hmc = HMC(); mh = MH(); Gibbs(@varname(x) => hmc, @varname(x) => hmc, @varname(y) => mh)`.
+
 # Release 0.33.0
 
 ## Breaking changes

src/mcmc/gibbs.jl

@@ -279,24 +279,39 @@ function Gibbs(algs::Pair...)
     return Gibbs(map(first, algs), map(wrap_algorithm_maybe, map(last, algs)))
 end
 
-# The below constructor only serves to provide backwards compatibility with the constructor
-# of the old Gibbs sampler. It is deprecated and will be removed in the future.
+# The below two constructors only provide backwards compatibility with the constructor of
+# the old Gibbs sampler. They are deprecated and will be removed in the future.
 function Gibbs(algs::InferenceAlgorithm...)
-    alg_dict = Dict{Any,InferenceAlgorithm}()
-    for alg in algs
+    varnames = map(algs) do alg
         space = getspace(alg)
-        space_vns = if (space isa Symbol || space isa VarName)
+        if (space isa VarName)
             space
+        elseif (space isa Symbol)
+            VarName{space}()
         else
             tuple((s isa Symbol ? VarName{s}() : s for s in space)...)
         end
-        alg_dict[space_vns] = alg
     end
-    Base.depwarn(
-        "Specifying which sampler to use with which variable using syntax like `Gibbs(NUTS(:x), MH(:y))` is deprecated and will be removed in the future. Please use `Gibbs(; x=NUTS(), y=MH())` instead.",
-        :Gibbs,
+    msg = (
+        "Specifying which sampler to use with which variable using syntax like " *
+        "`Gibbs(NUTS(:x), MH(:y))` is deprecated and will be removed in the future. " *
+        "Please use `Gibbs(; x=NUTS(), y=MH())` instead. If you want different iteration " *
+        "counts for different subsamplers, use e.g. " *
+        "`Gibbs(@varname(x) => NUTS(), @varname(x) => NUTS(), @varname(y) => MH())`"
     )
-    return Gibbs(alg_dict)
+    Base.depwarn(msg, :Gibbs)
+    return Gibbs(varnames, map(wrap_algorithm_maybe, algs))
+end
+
+function Gibbs(algs_with_iters::Tuple{<:InferenceAlgorithm,Int}...)
+    algs = Iterators.map(first, algs_with_iters)
+    iters = Iterators.map(last, algs_with_iters)
+    algs_duplicated = Iterators.flatten((
+        Iterators.repeated(alg, iter) for (alg, iter) in zip(algs, iters)
+    ))
+    # This calls the other deprecated constructor from above, hence no need for a depwarn
+    # here.
+    return Gibbs(algs_duplicated...)
 end
 
 # TODO: Remove when no longer needed.

test/mcmc/gibbs.jl

@@ -52,7 +52,8 @@ has_dot_assume(::DynamicPPL.Model) = true
         @test_deprecated s4 = Gibbs(PG(3, :s), HMC(0.4, 8, :m; adtype=adbackend))
         @test_deprecated s5 = Gibbs(CSMC(3, :s), HMC(0.4, 8, :m; adtype=adbackend))
         @test_deprecated s6 = Gibbs(HMC(0.1, 5, :s; adtype=adbackend), ESS(:m))
-        for s in (s1, s2, s3, s4, s5, s6)
+        @test_deprecated s7 = Gibbs((HMC(0.1, 5, :s; adtype=adbackend), 2), (ESS(:m), 3))
+        for s in (s1, s2, s3, s4, s5, s6, s7)
             @test DynamicPPL.alg_str(Turing.Sampler(s, gdemo_default)) == "Gibbs"
         end
 
@@ -63,6 +64,7 @@ has_dot_assume(::DynamicPPL.Model) = true
         sample(gdemo_default, s4, N)
         sample(gdemo_default, s5, N)
         sample(gdemo_default, s6, N)
+        sample(gdemo_default, s7, N)
 
         g = Turing.Sampler(s3, gdemo_default)
         @test sample(gdemo_default, g, N) isa MCMCChains.Chains