disable fallback for returned and pointwise_logdensities (#1159)

* disable fallback for returned and pointwise_logdensities

* Add tests

Author: penelopeysm

HISTORY.md

@@ -1,5 +1,13 @@
 # DynamicPPL Changelog
 
+## 0.38.10
+
+`returned(model, chain)` and `pointwise_logdensities(model, chain)` will now error if a value for a random variable cannot be found in the chain.
+(Previously, they would instead resample such variables, which could lead to silent mistakes.)
+
+If you encounter this error and it is accompanied by a warning about `hasvalue` not being implemented, you should be able to fix this by [using FlexiChains instead of MCMCChains](https://github.com/penelopeysm/FlexiChains.jl).
+(Alternatively, implementations of `hasvalue` for unsupported distributions are more than welcome; these must be provided in the Distributions extension of AbstractPPL.jl.)
+
 ## 0.38.9
 
 Remove warning when using Enzyme as the AD backend.

Project.toml

@@ -1,6 +1,6 @@
 name = "DynamicPPL"
 uuid = "366bfd00-2699-11ea-058f-f148b4cae6d8"
-version = "0.38.9"
+version = "0.38.10"
 
 [deps]
 ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b"

ext/DynamicPPLMCMCChainsExt.jl

@@ -226,7 +226,10 @@ function DynamicPPL.predict(
     )
     predictions = map(params_and_stats) do ps
         _, varinfo = DynamicPPL.init!!(
-            rng, model, varinfo, DynamicPPL.InitFromParams(ps.params)
+            rng,
+            model,
+            varinfo,
+            DynamicPPL.InitFromParams(ps.params, DynamicPPL.InitFromPrior()),
         )
         DynamicPPL.ParamsWithStats(varinfo)
     end
@@ -316,11 +319,7 @@ function DynamicPPL.returned(model::DynamicPPL.Model, chain_full::MCMCChains.Cha
     params_with_stats = AbstractMCMC.to_samples(DynamicPPL.ParamsWithStats, chain)
     return map(params_with_stats) do ps
         first(
-            DynamicPPL.init!!(
-                model,
-                varinfo,
-                DynamicPPL.InitFromParams(ps.params, DynamicPPL.InitFromPrior()),
-            ),
+            DynamicPPL.init!!(model, varinfo, DynamicPPL.InitFromParams(ps.params, nothing))
         )
     end
 end
@@ -426,9 +425,7 @@ function DynamicPPL.pointwise_logdensities(
         values_dict = chain_sample_to_varname_dict(parameter_only_chain, sample_idx, chain_idx)
         # Re-evaluate the model
         _, vi = DynamicPPL.init!!(
-            model,
-            vi,
-            DynamicPPL.InitFromParams(values_dict, DynamicPPL.InitFromPrior()),
+            model, vi, DynamicPPL.InitFromParams(values_dict, nothing)
         )
         DynamicPPL.getacc(vi, Val(accname)).logps
     end

test/ext/DynamicPPLMCMCChainsExt.jl

@@ -3,19 +3,6 @@ module DynamicPPLMCMCChainsExtTests
 using DynamicPPL, Distributions, MCMCChains, Test, AbstractMCMC
 
 @testset "DynamicPPLMCMCChainsExt" begin
-    @model demo() = x ~ Normal()
-    model = demo()
-
-    chain = MCMCChains.Chains(
-        randn(1000, 2, 1),
-        [:x, :y],
-        Dict(:internals => [:y]);
-        info=(; varname_to_symbol=Dict(@varname(x) => :x)),
-    )
-    chain_generated = @test_nowarn returned(model, chain)
-    @test size(chain_generated) == (1000, 1)
-    @test mean(chain_generated) ≈ 0 atol = 0.1
-
     @testset "from_samples" begin
         @model function f(z)
             x ~ Normal()
@@ -61,6 +48,42 @@ using DynamicPPL, Distributions, MCMCChains, Test, AbstractMCMC
             @test new_p.stats == p.stats
         end
     end
+
+    @testset "returned (basic)" begin
+        @model demo() = x ~ Normal()
+        model = demo()
+
+        chain = MCMCChains.Chains(
+            randn(1000, 2, 1),
+            [:x, :y],
+            Dict(:internals => [:y]);
+            info=(; varname_to_symbol=Dict(@varname(x) => :x)),
+        )
+        chain_generated = @test_nowarn returned(model, chain)
+        @test size(chain_generated) == (1000, 1)
+        @test mean(chain_generated) ≈ 0 atol = 0.1
+    end
+
+    @testset "returned: errors on missing variable" begin
+        # Create a chain that only has `m`.
+        @model function m_only()
+            return m ~ Normal()
+        end
+        model_m_only = m_only()
+        chain_m_only = AbstractMCMC.from_samples(
+            MCMCChains.Chains,
+            hcat([ParamsWithStats(VarInfo(model_m_only), model_m_only) for _ in 1:50]),
+        )
+
+        # Define a model that needs both `m` and `s`.
+        @model function f()
+            m ~ Normal()
+            s ~ Exponential()
+            return y ~ Normal(m, s)
+        end
+        model = f() | (; y=1.0)
+        @test_throws "No value was provided" returned(model, chain_m_only)
+    end
 end
 
 # test for `predict` is in `test/model.jl`

test/pointwise_logdensities.jl

@@ -94,4 +94,29 @@ end
         @test logprior ≈ logprior_true
         @test loglikelihood ≈ loglikelihood_true
     end
+
+    @testset "errors when variables are missing" begin
+        # Create a chain that only has `m`.
+        @model function m_only()
+            return m ~ Normal()
+        end
+        model_m_only = m_only()
+        chain_m_only = AbstractMCMC.from_samples(
+            MCMCChains.Chains,
+            hcat([ParamsWithStats(VarInfo(model_m_only), model_m_only) for _ in 1:50]),
+        )
+
+        # Define a model that needs both `m` and `s`.
+        @model function f()
+            m ~ Normal()
+            s ~ Exponential()
+            return y ~ Normal(m, s)
+        end
+        model = f() | (; y=1.0)
+        @test_throws "No value was provided" pointwise_logdensities(model, chain_m_only)
+        @test_throws "No value was provided" pointwise_loglikelihoods(model, chain_m_only)
+        @test_throws "No value was provided" pointwise_prior_logdensities(
+            model, chain_m_only
+        )
+    end
 end