From 1a0982c0448f8f4796a36b06652dddf9325eb8f9 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Thu, 28 Nov 2024 14:49:48 +0000 Subject: [PATCH 1/6] Fix wrong function being called --- src/test_utils/contexts.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test_utils/contexts.jl b/src/test_utils/contexts.jl index ec663f5cc..4748a221c 100644 --- a/src/test_utils/contexts.jl +++ b/src/test_utils/contexts.jl @@ -92,8 +92,8 @@ function test_context(context::DynamicPPL.AbstractContext, model::DynamicPPL.Mod # Setting the child context to a leaf should now change the leafcontext accordingly. context_with_new_leaf = DynamicPPL.setchildcontext(context, leafcontext_new) - @test DynamicPPL.setchildcontext(context_with_new_leaf) === - DynamicPPL.setleafcontext(context_with_new_leaf) === + @test DynamicPPL.childcontext(context_with_new_leaf) === + DynamicPPL.leafcontext(context_with_new_leaf) === leafcontext_new # Make sure that the we can evaluate the model with the context (i.e. that none of the tilde-functions are incorrectly overloaded). From 527b389e17bd4bfd0d81ba484ecbc9d3bb342382 Mon Sep 17 00:00:00 2001 From: Penelope Yong Date: Thu, 28 Nov 2024 14:56:13 +0000 Subject: [PATCH 2/6] Don't test setchildcontext on leaf contexts --- src/test_utils/contexts.jl | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/test_utils/contexts.jl b/src/test_utils/contexts.jl index 4748a221c..4de08bf62 100644 --- a/src/test_utils/contexts.jl +++ b/src/test_utils/contexts.jl @@ -71,6 +71,15 @@ function test_context(context::DynamicPPL.AbstractContext, model::DynamicPPL.Mod node_trait isa Union{DynamicPPL.IsLeaf,DynamicPPL.IsParent} || throw(ValueError("Invalid NodeTrait: $node_trait")) + # To see change, let's make sure we're using a different leaf context than the current. + leafcontext_new = if DynamicPPL.leafcontext(context) isa DefaultContext + PriorContext() + else + DefaultContext() + end + @test DynamicPPL.leafcontext(DynamicPPL.setleafcontext(context, leafcontext_new)) == + leafcontext_new + # The interface methods. if node_trait isa DynamicPPL.IsParent # `childcontext` and `setchildcontext` @@ -79,22 +88,14 @@ function test_context(context::DynamicPPL.AbstractContext, model::DynamicPPL.Mod @test DynamicPPL.childcontext( DynamicPPL.setchildcontext(context, childcontext_new) ) == childcontext_new + # Setting the child context to a leaf should now change the leafcontext + # accordingly. + context_with_new_leaf = DynamicPPL.setchildcontext(context, leafcontext_new) + @test DynamicPPL.childcontext(context_with_new_leaf) === + DynamicPPL.leafcontext(context_with_new_leaf) === + leafcontext_new end - # To see change, let's make sure we're using a different leaf context than the current. - leafcontext_new = if DynamicPPL.leafcontext(context) isa DefaultContext - PriorContext() - else - DefaultContext() - end - @test DynamicPPL.leafcontext(DynamicPPL.setleafcontext(context, leafcontext_new)) == - leafcontext_new - - # Setting the child context to a leaf should now change the leafcontext accordingly. - context_with_new_leaf = DynamicPPL.setchildcontext(context, leafcontext_new) - @test DynamicPPL.childcontext(context_with_new_leaf) === - DynamicPPL.leafcontext(context_with_new_leaf) === - leafcontext_new # Make sure that the we can evaluate the model with the context (i.e. that none of the tilde-functions are incorrectly overloaded). # The tilde-pipeline contains two different paths: with `SamplingContext` as a parent, and without it. From ec0448ea224697f7574fca21a3e352ef616160e2 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Thu, 28 Nov 2024 16:18:48 +0100 Subject: [PATCH 3/6] Update src/test_utils/contexts.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- src/test_utils/contexts.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test_utils/contexts.jl b/src/test_utils/contexts.jl index 4de08bf62..93bb02d3b 100644 --- a/src/test_utils/contexts.jl +++ b/src/test_utils/contexts.jl @@ -96,7 +96,6 @@ function test_context(context::DynamicPPL.AbstractContext, model::DynamicPPL.Mod leafcontext_new end - # Make sure that the we can evaluate the model with the context (i.e. that none of the tilde-functions are incorrectly overloaded). # The tilde-pipeline contains two different paths: with `SamplingContext` as a parent, and without it. # NOTE(torfjelde): Need to sample with the untyped varinfo _using_ the context, since the From b389ce47010cbf3e0a7a809c0b53b9fb319331da Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Thu, 28 Nov 2024 19:07:52 +0100 Subject: [PATCH 4/6] fixed method ambiguities for DebugContext --- src/debug_utils.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/debug_utils.jl b/src/debug_utils.jl index dcd3fcc37..f9ae2428a 100644 --- a/src/debug_utils.jl +++ b/src/debug_utils.jl @@ -332,7 +332,7 @@ function DynamicPPL.tilde_assume(context::DebugContext, right, vn, vi) record_post_tilde_assume!(context, vn, right, value, logp, vi) return value, logp, vi end -function DynamicPPL.tilde_assume(rng, context::DebugContext, sampler, right, vn, vi) +function DynamicPPL.tilde_assume(rng::Random.AbstractRNG, context::DebugContext, sampler, right, vn, vi) record_pre_tilde_assume!(context, vn, right, vi) value, logp, vi = DynamicPPL.tilde_assume( rng, childcontext(context), sampler, right, vn, vi @@ -425,7 +425,7 @@ function DynamicPPL.dot_tilde_assume(context::DebugContext, right, left, vn, vi) end function DynamicPPL.dot_tilde_assume( - rng, context::DebugContext, sampler, right, left, vn, vi + rng::Random.AbstractRNG, context::DebugContext, sampler, right, left, vn, vi ) record_pre_dot_tilde_assume!(context, vn, left, right, vi) value, logp, vi = DynamicPPL.dot_tilde_assume( From 52db39db49cfaa409adf90186208a59ad4444571 Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Thu, 28 Nov 2024 19:10:40 +0100 Subject: [PATCH 5/6] test the context interface for DebugContext on multiple models --- test/debug_utils.jl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/debug_utils.jl b/test/debug_utils.jl index a7a9f7b71..d0ef27348 100644 --- a/test/debug_utils.jl +++ b/test/debug_utils.jl @@ -1,9 +1,9 @@ @testset "check_model" begin @testset "context interface" begin - # HACK: Require a model to instantiate it, so let's just grab one. - model = first(DynamicPPL.TestUtils.DEMO_MODELS) - context = DynamicPPL.DebugUtils.DebugContext(model) - DynamicPPL.TestUtils.test_context(context, model) + @testset "$(model.f)" for model in DynamicPPL.TestUtils.DEMO_MODELS + context = DynamicPPL.DebugUtils.DebugContext(model) + DynamicPPL.TestUtils.test_context(context, model) + end end @testset "$(model.f)" for model in DynamicPPL.TestUtils.DEMO_MODELS @@ -14,7 +14,7 @@ # Check that the trace contains all the variables in the model. varnames_in_trace = DynamicPPL.DebugUtils.varnames_in_trace(trace) for vn in DynamicPPL.TestUtils.varnames(model) - @test vn in varnames_in_trace + @test vn in varnames_in_traces end # Quick checks for `show` of trace. From 88472c60976d604483168af7b521c7ab69df1d9b Mon Sep 17 00:00:00 2001 From: Tor Erlend Fjelde Date: Thu, 28 Nov 2024 19:11:16 +0100 Subject: [PATCH 6/6] Update src/debug_utils.jl Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- src/debug_utils.jl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/debug_utils.jl b/src/debug_utils.jl index f9ae2428a..f486482a9 100644 --- a/src/debug_utils.jl +++ b/src/debug_utils.jl @@ -332,7 +332,9 @@ function DynamicPPL.tilde_assume(context::DebugContext, right, vn, vi) record_post_tilde_assume!(context, vn, right, value, logp, vi) return value, logp, vi end -function DynamicPPL.tilde_assume(rng::Random.AbstractRNG, context::DebugContext, sampler, right, vn, vi) +function DynamicPPL.tilde_assume( + rng::Random.AbstractRNG, context::DebugContext, sampler, right, vn, vi +) record_pre_tilde_assume!(context, vn, right, vi) value, logp, vi = DynamicPPL.tilde_assume( rng, childcontext(context), sampler, right, vn, vi