From 9df4914b9df7278fc97b106fb74c8c0fa748f26b Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Fri, 28 Nov 2025 17:14:44 +0000 Subject: [PATCH 1/2] disable fallback for returned and pointwise_logdensities --- HISTORY.md | 8 ++++++++ Project.toml | 2 +- ext/DynamicPPLMCMCChainsExt.jl | 15 ++++++--------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 24c0df3d0..fda2109e7 100644 --- a/HISTORY.md +++ b/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. diff --git a/Project.toml b/Project.toml index 23f5eec5b..2ce3a679f 100644 --- a/Project.toml +++ b/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" diff --git a/ext/DynamicPPLMCMCChainsExt.jl b/ext/DynamicPPLMCMCChainsExt.jl index d8c343917..9236ae25e 100644 --- a/ext/DynamicPPLMCMCChainsExt.jl +++ b/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 From 916b8d521236335d3905c9979c54214ddf495cd9 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Mon, 1 Dec 2025 10:54:08 +0000 Subject: [PATCH 2/2] Add tests --- test/ext/DynamicPPLMCMCChainsExt.jl | 49 +++++++++++++++++++++-------- test/pointwise_logdensities.jl | 25 +++++++++++++++ 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/test/ext/DynamicPPLMCMCChainsExt.jl b/test/ext/DynamicPPLMCMCChainsExt.jl index f537415d5..6091492df 100644 --- a/test/ext/DynamicPPLMCMCChainsExt.jl +++ b/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` diff --git a/test/pointwise_logdensities.jl b/test/pointwise_logdensities.jl index be5f20010..780d45b46 100644 --- a/test/pointwise_logdensities.jl +++ b/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