Merge remote-tracking branch 'origin/breaking' into mhauru/varnamedvector-as-default

Author: mhauru

.github/workflows/Benchmarking.yml

@@ -59,14 +59,14 @@ jobs:
           echo "DPPL_COMMIT_URL=$COMMIT_URL" >> $GITHUB_ENV
 
       - name: Find Existing Comment
-        uses: peter-evans/find-comment@v3
+        uses: peter-evans/find-comment@v4
         id: find_comment
         with:
           issue-number: ${{ github.event.pull_request.number }}
           comment-author: github-actions[bot]
 
       - name: Post Benchmark Results as PR Comment
-        uses: peter-evans/create-or-update-comment@v4
+        uses: peter-evans/create-or-update-comment@v5
         with:
           issue-number: ${{ github.event.pull_request.number }}
           body: |

HISTORY.md

@@ -4,6 +4,18 @@
 
 Removed the method `returned(::Model, values, keys)`; please use `returned(::Model, ::AbstractDict{<:VarName})` instead.
 
+## 0.38.7
+
+Made a small tweak to DynamicPPL's compiler output to avoid potential undefined variables when resuming model functions midway through (e.g. with Libtask in Turing's SMC/PG samplers).
+
+## 0.38.6
+
+Renamed keyword argument `only_ddpl` to `only_dppl` for `Experimental.is_suitable_varinfo`.
+
+## 0.38.5
+
+Improve performance of VarNamedVector, mostly by changing how it handles contiguification.
+
 ## 0.38.4
 
 Improve performance of VarNamedVector. It should now be very nearly on par with Metadata for all models we've benchmarked on.
@@ -19,8 +31,6 @@ The generic method `returned(::Model, values, keys)` is deprecated and will be r
 
 Added a compatibility entry for [email protected].
 
-> > > > > > > main
-
 ## 0.38.1
 
 Added `from_linked_vec_transform` and `from_vec_transform` methods for `ProductNamedTupleDistribution`.

ext/DynamicPPLJETExt.jl

@@ -4,12 +4,12 @@ using DynamicPPL: DynamicPPL
 using JET: JET
 
 function DynamicPPL.Experimental.is_suitable_varinfo(
-    model::DynamicPPL.Model, varinfo::DynamicPPL.AbstractVarInfo; only_ddpl::Bool=true
+    model::DynamicPPL.Model, varinfo::DynamicPPL.AbstractVarInfo; only_dppl::Bool=true
 )
     f, argtypes = DynamicPPL.DebugUtils.gen_evaluator_call_with_types(model, varinfo)
     # If specified, we only check errors originating somewhere in the DynamicPPL.jl.
     # This way we don't just fall back to untyped if the user's code is the issue.
-    result = if only_ddpl
+    result = if only_dppl
         JET.report_call(f, argtypes; target_modules=(JET.AnyFrameModule(DynamicPPL),))
     else
         JET.report_call(f, argtypes)
@@ -18,15 +18,15 @@ function DynamicPPL.Experimental.is_suitable_varinfo(
 end
 
 function DynamicPPL.Experimental._determine_varinfo_jet(
-    model::DynamicPPL.Model; only_ddpl::Bool=true
+    model::DynamicPPL.Model; only_dppl::Bool=true
 )
     # Generate a typed varinfo to test model type stability with
     varinfo = DynamicPPL.typed_varinfo(model)
 
     # Check type stability of evaluation (i.e. DefaultContext)
     model = DynamicPPL.setleafcontext(model, DynamicPPL.DefaultContext())
     eval_issuccess, eval_result = DynamicPPL.Experimental.is_suitable_varinfo(
-        model, varinfo; only_ddpl
+        model, varinfo; only_dppl
     )
     if !eval_issuccess
         @debug "Evaluation with typed varinfo failed with the following issues:"
@@ -36,7 +36,7 @@ function DynamicPPL.Experimental._determine_varinfo_jet(
     # Check type stability of initialisation (i.e. InitContext)
     model = DynamicPPL.setleafcontext(model, DynamicPPL.InitContext())
     init_issuccess, init_result = DynamicPPL.Experimental.is_suitable_varinfo(
-        model, varinfo; only_ddpl
+        model, varinfo; only_dppl
     )
     if !init_issuccess
         @debug "Initialisation with typed varinfo failed with the following issues:"

src/compiler.jl

@@ -461,9 +461,16 @@ function generate_tilde(left, right)
         elseif $isassumption
             $(generate_tilde_assume(left, dist, vn))
         else
-            # If `vn` is not in `argnames`, we need to make sure that the variable is defined.
-            if !$(DynamicPPL.inargnames)($vn, __model__)
-                $left = $(DynamicPPL.getconditioned_nested)(
+            # If `vn` is not in `argnames`, then it's definitely been conditioned on (if
+            # it's not in `argnames` and wasn't conditioned on, then `isassumption` would
+            # be true).
+            $left = if $(DynamicPPL.inargnames)($vn, __model__)
+                # This is a no-op and looks redundant, but defining the compiler output this
+                # way ensures that the variable `$left` is always defined. See
+                # https://github.com/TuringLang/DynamicPPL.jl/pull/1110.
+                $left
+            else
+                $(DynamicPPL.getconditioned_nested)(
                     __model__.context, $(DynamicPPL.prefix)(__model__.context, $vn)
                 )
             end

src/experimental.jl

@@ -16,7 +16,7 @@ Check if the `model` supports evaluation using the provided `varinfo`.
 - `varinfo`: The varinfo to verify the support for.
 
 # Keyword Arguments
-- `only_ddpl`: If `true`, only consider error reports occuring in the tilde pipeline. Default: `true`.
+- `only_dppl`: If `true`, only consider error reports occuring in the tilde pipeline. Default: `true`.
 
 # Returns
 - `issuccess`: `true` if the model supports the varinfo, otherwise `false`.
@@ -28,7 +28,7 @@ function is_suitable_varinfo end
 function _determine_varinfo_jet end
 
 """
-    determine_suitable_varinfo(model; only_ddpl::Bool=true)
+    determine_suitable_varinfo(model; only_dppl::Bool=true)
 
 Return a suitable varinfo for the given `model`.
 
@@ -42,7 +42,7 @@ See also: [`DynamicPPL.Experimental.is_suitable_varinfo`](@ref).
 - `model`: The model for which to determine the varinfo.
 
 # Keyword Arguments
-- `only_ddpl`: If `true`, only consider error reports within DynamicPPL.jl.
+- `only_dppl`: If `true`, only consider error reports within DynamicPPL.jl.
 
 # Examples
 
@@ -83,10 +83,10 @@ julia> vi isa typeof(DynamicPPL.typed_varinfo(model_with_static_support()))
 true
 ```
 """
-function determine_suitable_varinfo(model::DynamicPPL.Model; only_ddpl::Bool=true)
+function determine_suitable_varinfo(model::DynamicPPL.Model; only_dppl::Bool=true)
     # If JET.jl has been loaded, and thus `determine_varinfo` has been defined, we use that.
     return if Base.get_extension(DynamicPPL, :DynamicPPLJETExt) !== nothing
-        _determine_varinfo_jet(model; only_ddpl)
+        _determine_varinfo_jet(model; only_dppl)
     else
         # Warn the user.
         @warn "JET.jl is not loaded. Assumes the model is compatible with typed varinfo."

test/accumulators.jl

@@ -117,7 +117,7 @@ using DynamicPPL:
             @test at_all64[:LogLikelihood] == ll_f64
 
             @test haskey(AccumulatorTuple(lp_f64), Val(:LogPrior))
-            @test ~haskey(AccumulatorTuple(lp_f64), Val(:LogLikelihood))
+            @test !haskey(AccumulatorTuple(lp_f64), Val(:LogLikelihood))
             @test length(AccumulatorTuple(lp_f64, ll_f64)) == 2
             @test keys(at_all64) == (:LogPrior, :LogLikelihood)
             @test collect(at_all64) == [lp_f64, ll_f64]

test/ext/DynamicPPLJETExt.jl

@@ -56,7 +56,7 @@
             DynamicPPL.NTVarInfo
         # Should fail if we're including errors in the model body.
         @test DynamicPPL.Experimental.determine_suitable_varinfo(
-            demo5(); only_ddpl=false
+            demo5(); only_dppl=false
         ) isa DynamicPPL.UntypedVarInfo
     end
 

test/varinfo.jl

@@ -73,10 +73,10 @@ end
             r = rand(dist)
 
             @test isempty(vi)
-            @test ~haskey(vi, vn)
+            @test !haskey(vi, vn)
             @test !(vn in keys(vi))
             vi = push!!(vi, vn, r, dist)
-            @test ~isempty(vi)
+            @test !isempty(vi)
             @test haskey(vi, vn)
             @test vn in keys(vi)
 
@@ -97,7 +97,7 @@ end
             vi = empty!!(vi)
             @test isempty(vi)
             vi = push!!(vi, vn, r, dist)
-            @test ~isempty(vi)
+            @test !isempty(vi)
         end
 
         test_base(VarInfo())

test/varnamedvector.jl

@@ -621,14 +621,14 @@ end
         vnv = DynamicPPL.VarNamedVector()
         vnv = setindex!!(vnv, 1.0, vn)
         vnv = setindex!!(vnv, 2, @varname(b))
-        @test ~DynamicPPL.is_tightly_typed(vnv)
+        @test !DynamicPPL.is_tightly_typed(vnv)
         test_tightenability(vnv)
         @inferred DynamicPPL.loosen_types!!(vnv, VarName, Any, Any)
         # Likewise when first mixed types are pushed, but then deleted.
         vnv = DynamicPPL.VarNamedVector()
         vnv = setindex!!(vnv, 1.0, vn)
         vnv = setindex!!(vnv, 2, @varname(b))
-        @test ~DynamicPPL.is_tightly_typed(vnv)
+        @test !DynamicPPL.is_tightly_typed(vnv)
         vnv = delete!!(vnv, vn)
         @test DynamicPPL.is_tightly_typed(vnv)
         test_tightenability(vnv)
@@ -646,17 +646,17 @@ end
         t = eltype(vnv.transforms)
         # Loosen key type.
         vnv = @inferred DynamicPPL.loosen_types!!(vnv, VarName, e, t)
-        @test ~DynamicPPL.is_tightly_typed(vnv)
+        @test !DynamicPPL.is_tightly_typed(vnv)
         vnv = DynamicPPL.tighten_types!!(vnv)
         @test DynamicPPL.is_tightly_typed(vnv)
         # Loosen element type
         vnv = @inferred DynamicPPL.loosen_types!!(vnv, k, Real, t)
-        @test ~DynamicPPL.is_tightly_typed(vnv)
+        @test !DynamicPPL.is_tightly_typed(vnv)
         vnv = DynamicPPL.tighten_types!!(vnv)
         @test DynamicPPL.is_tightly_typed(vnv)
         # Loosen transformation type
         vnv = @inferred DynamicPPL.loosen_types!!(vnv, k, e, Function)
-        @test ~DynamicPPL.is_tightly_typed(vnv)
+        @test !DynamicPPL.is_tightly_typed(vnv)
         vnv = DynamicPPL.tighten_types!!(vnv)
         @test DynamicPPL.is_tightly_typed(vnv)
         # Loosening to the same types as currently should do nothing.