Type piracy breaks Turing/AbstractMCMC sampling in external code

[bgroenks96]

Hi,

It seems that the MarkovChainMonteCarlo module currently commits type piracy in AbstractMCMC here by overwriting the dispatch:

AdvancedMH.transition(sampler::AdvancedMH.MHSampler, model::AdvancedMH.DensityModel, params, log_density::Real)

which then has the effect of breaking basic MH sampling on an unrelated model after loading the CalibrateEmulateSample module:

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

sample(testmodel(), MH(), 10)

Error:

ERROR: type MCMCState has no field lp
Stacktrace:
  [1] getproperty(x::CalibrateEmulateSample.MarkovChainMonteCarlo.MCMCState{NamedTuple{(:x,), Tuple{Float64}}, Float64}, f::Symbol)
    @ Base .\Base.jl:37
  [2] propose!!(rng::Random.TaskLocalRNG, vi::DynamicPPL.TypedVarInfo{NamedTuple{(:x,), Tuple{DynamicPPL.Metadata{Dict{AbstractPPL.VarName{:x, Setfield.IdentityLens}, Int64}, Vector{Normal{Float64}}, Vector{AbstractPPL.VarName{:x, Setfield.IdentityLens}}, Vector{Float64}, Vector{Set{DynamicPPL.Selector}}}}}, Float64}, model::DynamicPPL.Model{typeof(testmodel), (), (), (), Tuple{}, Tuple{}, DynamicPPL.DefaultContext}, spl::DynamicPPL.Sampler{MH{(), NamedTuple{(), Tuple{}}}}, proposal::NamedTuple{(), Tuple{}})
    @ Turing.Inference C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\mh.jl:392
  [3] #step#62
    @ C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\mh.jl:451 [inlined]
  [4] step(rng::Random.TaskLocalRNG, model::DynamicPPL.Model{typeof(testmodel), (), (), (), Tuple{}, Tuple{}, DynamicPPL.DefaultContext}, spl::DynamicPPL.Sampler{MH{(), NamedTuple{(), Tuple{}}}}, vi::DynamicPPL.TypedVarInfo{NamedTuple{(:x,), Tuple{DynamicPPL.Metadata{Dict{AbstractPPL.VarName{:x, Setfield.IdentityLens}, Int64}, Vector{Normal{Float64}}, Vector{AbstractPPL.VarName{:x, Setfield.IdentityLens}}, Vector{Float64}, Vector{Set{DynamicPPL.Selector}}}}}, Float64})
    @ Turing.Inference C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\mh.jl:441
  [5] macro expansion
    @ C:\Users\bgroenke\.julia\packages\AbstractMCMC\fWWW0\src\sample.jl:168 [inlined]
  [6] macro expansion
    @ C:\Users\bgroenke\.julia\packages\ProgressLogging\6KXlp\src\ProgressLogging.jl:328 [inlined]
  [7] macro expansion
    @ C:\Users\bgroenke\.julia\packages\AbstractMCMC\fWWW0\src\logging.jl:9 [inlined]
  [8] mcmcsample(rng::Random.TaskLocalRNG, model::DynamicPPL.Model{typeof(testmodel), (), (), (), Tuple{}, Tuple{}, DynamicPPL.DefaultContext}, sampler::DynamicPPL.Sampler{MH{(), NamedTuple{(), Tuple{}}}}, N::Int64; progress::Bool, progressname::String, callback::Nothing, discard_initial::Int64, thinning::Int64, 
chain_type::Type, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ AbstractMCMC C:\Users\bgroenke\.julia\packages\AbstractMCMC\fWWW0\src\sample.jl:116
  [9] sample(rng::Random.TaskLocalRNG, model::DynamicPPL.Model{typeof(testmodel), (), (), (), Tuple{}, Tuple{}, DynamicPPL.DefaultContext}, sampler::DynamicPPL.Sampler{MH{(), NamedTuple{(), Tuple{}}}}, N::Int64; chain_type::Type, resume_from::Nothing, progress::Bool, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Turing.Inference C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\Inference.jl:160
 [10] sample
    @ C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\Inference.jl:149 [inlined]
 [11] #sample#2
    @ C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\Inference.jl:146 [inlined]
 [12] sample
    @ C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\Inference.jl:139 [inlined]
 [13] #sample#1
    @ C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\Inference.jl:136 [inlined]
 [14] sample(model::DynamicPPL.Model{typeof(testmodel), (), (), (), Tuple{}, Tuple{}, DynamicPPL.DefaultContext}, alg::MH{(), NamedTuple{(), Tuple{}}}, N::Int64)
    @ Turing.Inference C:\Users\bgroenke\.julia\packages\Turing\HwTAU\src\inference\Inference.jl:130
 [15] top-level scope
    @ c:\Users\bgroenke\dmawi\data\sparc\personal_accounts\03_PhD\Brian\explore\modeling\sciml_experiments\seb\run_seb_eks.jl:253

Is there a quick fix for this? Maybe a way to make this dispatch more specific?

[odunbar]

Thanks for raising this issue! I'll take a look, I agree it should be fixable by making changing the sampler dispatch

[odunbar]

Hi @bgroenks96 let me know if this issue is resolved on main for you! I will get round to releasing a new version soonish. Thanks again for raising the issue, please let us know how you get on with your application, and reopen if there are continued problems.

I'd also be pleased to hear about anything else that annoys you - all issues greatly welcome!

[bgroenks96]

Hi @odunbar thanks for the quick fix. Seems to be ok for me on main.

I think my main other issue with the package at the moment is the hard dependency on python. I would rather this be an opt-in soft dependency, but from #18 it sounds like this would break existing code and is not likely to happen.