Skip to content

Conversation

@penelopeysm
Copy link
Member

@penelopeysm penelopeysm commented Oct 24, 2025

Note

This PR is pretty much ready to go, but requires TuringLang/AbstractMCMC.jl#182 before it can be properly merged, hence marking as draft.

This PR moves external sampler interface functions like getparams and getstats out of Turing. It's really a proper continuation of #2616, which I messed up quite badly by merging into a patch release and had to revert. This PR therefore targets breaking.

The twin PR above adds them to AbstractMCMC. Once this is done, anyone who defines an external sampler will only need to depend on AbstractMCMC (which they presumably already do, because it is a sampler) and LogDensityProblems (which is already a dep of AbstractMCMC). No need for a Turing extension.

For an example implementation, see the test mock in this PR (note no Turing dependencies required):

# Turing declares an interface for external samplers (see docstring for
# ExternalSampler). We should check that implementing this interface
# and only this interface allows us to use the sampler in Turing.
struct MyState{V<:AbstractVector}
params::V
end
AbstractMCMC.getparams(s::MyState) = s.params
AbstractMCMC.getstats(s::MyState) = (param_length=length(s.params),)
# externalsamplers must accept LogDensityModel inside their step function.
# By default Turing gives the externalsampler a LDF constructed with
# adtype=ForwardDiff, so we should expect that inside the sampler we can
# call both `logdensity` and `logdensity_and_gradient`.
#
# The behaviour of this sampler is to simply calculate logp and its
# gradient, and then return the same values.
#
# TODO: Do we also want to run ADTypeCheckContext to make sure that it is
# indeed using the adtype provided from Turing?
struct MySampler <: AbstractMCMC.AbstractSampler end
function AbstractMCMC.step(
rng::Random.AbstractRNG,
model::AbstractMCMC.LogDensityModel,
sampler::MySampler;
# This initial_params should be an AbstractVector because the model is just a
# LogDensityModel, not a DynamicPPL.Model
initial_params::AbstractVector,
kwargs...,
)
# Step 1
ldf = model.logdensity
lp = LogDensityProblems.logdensity(ldf, initial_params)
@test lp isa Real
lp, grad = LogDensityProblems.logdensity_and_gradient(ldf, initial_params)
@test lp isa Real
@test grad isa AbstractVector{<:Real}
return nothing, MyState(initial_params)
end
function AbstractMCMC.step(
rng::Random.AbstractRNG,
model::AbstractMCMC.LogDensityModel,
sampler::MySampler,
state::MyState;
kwargs...,
)
# Step >= 1
params = state.params
ldf = model.logdensity
lp = LogDensityProblems.logdensity(ldf, params)
@test lp isa Real
lp, grad = LogDensityProblems.logdensity_and_gradient(ldf, params)
@test lp isa Real
@test grad isa AbstractVector{<:Real}
return nothing, MyState(params)
end

@penelopeysm penelopeysm changed the base branch from main to breaking October 24, 2025 13:24
@penelopeysm penelopeysm marked this pull request as draft October 24, 2025 13:39
@github-actions
Copy link
Contributor

Turing.jl documentation for PR #2704 is available at:
https://TuringLang.github.io/Turing.jl/previews/PR2704/

@codecov
Copy link

codecov bot commented Oct 24, 2025

Codecov Report

❌ Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.52%. Comparing base (be007f3) to head (0ebf0e8).

Files with missing lines Patch % Lines
src/mcmc/gibbs.jl 0.00% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           breaking    #2704      +/-   ##
============================================
- Coverage     86.45%   85.52%   -0.94%     
============================================
  Files            21       21              
  Lines          1418     1409       -9     
============================================
- Hits           1226     1205      -21     
- Misses          192      204      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@@ -1,5 +1,5 @@
"""
ExternalSampler{S<:AbstractSampler,AD<:ADTypes.AbstractADType,Unconstrained}
ExternalSampler{Unconstrained,S<:AbstractSampler,AD<:ADTypes.AbstractADType}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this type parameter earlier so that we can dispatch on it more easily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant