Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

to_submodel does not correctly handle manually specified prefixes when sampling #785

Closed
bgroenks96 opened this issue Jan 24, 2025 · 3 comments · Fixed by #787
Closed

to_submodel does not correctly handle manually specified prefixes when sampling #785

bgroenks96 opened this issue Jan 24, 2025 · 3 comments · Fixed by #787
Labels
bug Something isn't working high priority

Comments

@bgroenks96
Copy link
Collaborator

The new to_submodel implementation does not work correctly with prefixes specified via prefix.

Here is an MWE using Turing:

using Turing

@model function submodel()
  x ~ Normal(0,1)
  return x
end

@model function parentmodel()
  x1 ~ to_submodel(Turing.prefix(submodel(), :a), false)
  x2 ~ to_submodel(Turing.prefix(submodel(), :b), false)
  return x1 + x2
end

m = parentmodel()
println(rand(m))

prior_chain = sample(m, Prior(), 1000)
@show prior_chain

The parameter named tuple returned by rand(m) is correct:

(var"a.x" = -0.2742944683229543, var"b.x" = 0.09103999696852481)

But the call to sample throws an error:

ERROR: LoadError: varname x used multiple times in model
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:35
  [2] record_varname!(context::DynamicPPL.DebugUtils.DebugContext{…}, varname::AbstractPPL.VarName{…}, dist::Normal{…})
    @ DynamicPPL.DebugUtils ~/.julia/packages/DynamicPPL/uiOqX/src/debug_utils.jl:244
  [3] record_pre_tilde_assume!
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/debug_utils.jl:311 [inlined]
  [4] tilde_assume
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/debug_utils.jl:330 [inlined]
  [5] tilde_assume!!(context::DynamicPPL.DebugUtils.DebugContext{…}, right::Normal{…}, vn::AbstractPPL.VarName{…}, vi::DynamicPPL.UntypedVarInfo{…})
    @ DynamicPPL ~/.julia/packages/DynamicPPL/uiOqX/src/context_implementations.jl:114
  [6] submodel
    @ ~/workspace/scratch/mwe/turing/turing_submodel_bug.jl:4 [inlined]
  [7] _evaluate!!
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/model.jl:936 [inlined]
  [8] rand_like!!
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/model.jl:1318 [inlined]
  [9] rand_like!!
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/model.jl:1298 [inlined]
 [10] tilde_assume!!
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/context_implementations.jl:108 [inlined]
 [11] parentmodel(__model__::DynamicPPL.Model{…}, __varinfo__::DynamicPPL.UntypedVarInfo{…}, __context__::DynamicPPL.DebugUtils.DebugContext{…})
    @ Main ~/workspace/scratch/mwe/turing/turing_submodel_bug.jl:10
 [12] _evaluate!!
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/model.jl:936 [inlined]
 [13] evaluate_threadunsafe!!
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/model.jl:909 [inlined]
 [14] check_model_and_trace(rng::Random.TaskLocalRNG, model::DynamicPPL.Model{…}; varinfo::DynamicPPL.UntypedVarInfo{…}, context::DynamicPPL.SamplingContext{…}, error_on_failure::Bool, kwargs::@Kwargs{})
    @ DynamicPPL.DebugUtils ~/.julia/packages/DynamicPPL/uiOqX/src/debug_utils.jl:601
 [15] check_model_and_trace
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/debug_utils.jl:584 [inlined]
 [16] #check_model_and_trace#8
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/debug_utils.jl:582 [inlined]
 [17] check_model_and_trace
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/debug_utils.jl:581 [inlined]
 [18] check_model
    @ ~/.julia/packages/DynamicPPL/uiOqX/src/debug_utils.jl:627 [inlined]
 [19] _check_model
    @ ~/.julia/packages/Turing/oFGEb/src/mcmc/Inference.jl:296 [inlined]
 [20] _check_model
    @ ~/.julia/packages/Turing/oFGEb/src/mcmc/Inference.jl:299 [inlined]
 [21] #sample#6
    @ ~/.julia/packages/Turing/oFGEb/src/mcmc/Inference.jl:320 [inlined]
 [22] sample
    @ ~/.julia/packages/Turing/oFGEb/src/mcmc/Inference.jl:312 [inlined]
 [23] #sample#5
    @ ~/.julia/packages/Turing/oFGEb/src/mcmc/Inference.jl:309 [inlined]
 [24] sample(model::DynamicPPL.Model{typeof(parentmodel), (), (), (), Tuple{}, Tuple{}, DynamicPPL.DefaultContext}, alg::Prior, N::Int64)
    @ Turing.Inference ~/.julia/packages/Turing/oFGEb/src/mcmc/Inference.jl:306
 [25] top-level scope
    @ ~/workspace/scratch/mwe/turing/turing_submodel_bug.jl:17
 [26] include(fname::String)
    @ Base.MainInclude ./client.jl:494
 [27] top-level scope
    @ REPL[3]:1

This does not occur if autoprefixing is enabled. However, in that case, the manual prefix seems to be ignored in the resulting chain:

Summary Statistics
  parameters      mean       std      mcse    ess_bulk    ess_tail      rhat   ess_per_sec 
      Symbol   Float64   Float64   Float64     Float64     Float64   Float64       Float64 

        x1.x    0.0001    1.0165    0.0322   1002.2252   1025.2091    0.9998      323.9254
        x2.x    0.0004    0.9857    0.0301   1074.3013    994.5895    0.9999      347.2208

Quantiles
  parameters      2.5%     25.0%     50.0%     75.0%     97.5% 
      Symbol   Float64   Float64   Float64   Float64   Float64 

        x1.x   -1.9258   -0.6871   -0.0096    0.7225    1.9681
        x2.x   -2.0095   -0.6769    0.0350    0.6898    1.9609

Maybe this is actually a bug in Turing...? Feel free to move this issue if so.

@penelopeysm
Copy link
Member

penelopeysm commented Jan 25, 2025

Hi, @bgroenks96, thanks for reporting:)
There are at least two bugs here, unfortunately, and maybe a third one if you consider the fact that prefix isn't exported to be a bug. We'll push fixes for these next week, hopefully.
As for where you report it, please don't fret about it, it's trivial for us to transfer the issues to the right repository. As it happens the error message that you get is a DynamicPPL problem but there's also a broader Turing/AbstractMCMC problem about the prefixes in the chain (it turns out it's actually in DynamicPPL; I've now put that here #788).

@bgroenks96
Copy link
Collaborator Author

Thank you for looking into it so quickly!

@mhauru mhauru added the bug Something isn't working label Jan 27, 2025
@penelopeysm
Copy link
Member

This should be fixed in DynamicPPL 0.34.2, please feel free to reopen an issue if it's still broken!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants