Skip to content

Support DPPL 0.37 #2550

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Support DPPL 0.37 #2550

wants to merge 1 commit into from

Conversation

mhauru
Copy link
Member

@mhauru mhauru commented May 15, 2025

Currently in a very unfinished state.

github-actions[bot]
github-actions bot reviewed May 15, 2025
View reviewed changes
ext/TuringOptimExt.jl
@@ -104,8 +105,8 @@ function Optim.optimize(
options::Optim.Options=Optim.Options();
kwargs...,
)
ctx = Optimisation.OptimizationContext(DynamicPPL.DefaultContext())
f = Optimisation.OptimLogDensity(model, ctx)
vi = DynamicPPL.setaccs!!(VarInfo(model), (LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator(),))
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
vi = DynamicPPL.setaccs!!(VarInfo(model), (LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator(),))
vi = DynamicPPL.setaccs!!(
VarInfo(model),
(LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator()),
)

ext/TuringOptimExt.jl
@@ -127,8 +128,8 @@
options::Optim.Options=Optim.Options();
kwargs...,
)
ctx = Optimisation.OptimizationContext(DynamicPPL.DefaultContext())
f = Optimisation.OptimLogDensity(model, ctx)
vi = DynamicPPL.setaccs!!(VarInfo(model), (LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator(),))
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
vi = DynamicPPL.setaccs!!(VarInfo(model), (LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator(),))
vi = DynamicPPL.setaccs!!(
VarInfo(model),
(LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator()),
)

ext/TuringOptimExt.jl
@@ -144,9 +145,11 @@
end

function _map_optimize(model::DynamicPPL.Model, args...; kwargs...)
ctx = Optimisation.OptimizationContext(DynamicPPL.DefaultContext())
return _optimize(Optimisation.OptimLogDensity(model, ctx), args...; kwargs...)
vi = DynamicPPL.setaccs!!(VarInfo(model), (LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator(),))
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
vi = DynamicPPL.setaccs!!(VarInfo(model), (LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator(),))
vi = DynamicPPL.setaccs!!(
VarInfo(model),
(LogPriorWithoutJacobianAccumulator(), DynamicPPL.LogLikelihoodAccumulator()),
)

ext/TuringOptimExt.jl
logdensity_optimum = Optimisation.OptimLogDensity(
f.ldf.model, vi_optimum, f.ldf.context
)
logdensity_optimum = Optimisation.OptimLogDensity(f.ldf.model, vi_optimum; adtype=f.ldf.adtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
logdensity_optimum = Optimisation.OptimLogDensity(f.ldf.model, vi_optimum; adtype=f.ldf.adtype)
logdensity_optimum = Optimisation.OptimLogDensity(
f.ldf.model, vi_optimum; adtype=f.ldf.adtype
)

src/mcmc/Inference.jl
VarInfo(),
SamplingContext(rng, DynamicPPL.SampleFromPrior(), DynamicPPL.PriorContext()),
),
DynamicPPL.evaluate!!(model, vi, SamplingContext(rng, DynamicPPL.SampleFromPrior())),
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
DynamicPPL.evaluate!!(model, vi, SamplingContext(rng, DynamicPPL.SampleFromPrior())),
DynamicPPL.evaluate!!(model, vi, SamplingContext(rng, DynamicPPL.SampleFromPrior()))

src/optimisation/Optimisation.jl
Comment on lines 149 to 151
M<:DynamicPPL.Model,
V<:DynamicPPL.VarInfo,
C<:OptimizationContext,
V<:DynamicPPL.AbstractVarInfo,
AD<:ADTypes.AbstractADType,
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
M<:DynamicPPL.Model,
V<:DynamicPPL.VarInfo,
C<:OptimizationContext,
V<:DynamicPPL.AbstractVarInfo,
AD<:ADTypes.AbstractADType,
M<:DynamicPPL.Model,V<:DynamicPPL.AbstractVarInfo,AD<:ADTypes.AbstractADType

src/optimisation/Optimisation.jl
return OptimLogDensity(
DynamicPPL.LogDensityFunction(model, DynamicPPL.VarInfo(model), ctx; adtype=adtype)
)
function OptimLogDensity(model::DynamicPPL.Model, vi::DynamicPPL.AbstractVarInfo=DynamicPPL.VarInfo(model); adtype=AutoForwardDiff())
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
function OptimLogDensity(model::DynamicPPL.Model, vi::DynamicPPL.AbstractVarInfo=DynamicPPL.VarInfo(model); adtype=AutoForwardDiff())
function OptimLogDensity(
model::DynamicPPL.Model,
vi::DynamicPPL.AbstractVarInfo=DynamicPPL.VarInfo(model);
adtype=AutoForwardDiff(),
)

test/optimisation/Optimisation.jl
Comment on lines +62 to +64
vi_joint = DynamicPPL.setaccs!!(deepcopy(vi), (LogPriorWithoutJacobianAccumulator(), LogLikelihoodAccumulator()))
vi_prior = DynamicPPL.setaccs!!(deepcopy(vi), (LogPriorWithoutJacobianAccumulator(),))
vi_likelihood = DynamicPPL.setaccs!!(deepcopy(vi), (LogLikelihoodAccumulator(),))
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
vi_joint = DynamicPPL.setaccs!!(deepcopy(vi), (LogPriorWithoutJacobianAccumulator(), LogLikelihoodAccumulator()))
vi_prior = DynamicPPL.setaccs!!(deepcopy(vi), (LogPriorWithoutJacobianAccumulator(),))
vi_likelihood = DynamicPPL.setaccs!!(deepcopy(vi), (LogLikelihoodAccumulator(),))
vi_joint = DynamicPPL.setaccs!!(
deepcopy(vi),
(LogPriorWithoutJacobianAccumulator(), LogLikelihoodAccumulator()),
)
vi_prior = DynamicPPL.setaccs!!(
deepcopy(vi), (LogPriorWithoutJacobianAccumulator(),)
)
vi_likelihood = DynamicPPL.setaccs!!(
deepcopy(vi), (LogLikelihoodAccumulator(),)
)

test/test_utils/ad_utils.jl
Comment on lines +150 to 152
value, vi = DynamicPPL.tilde_assume(
DynamicPPL.childcontext(context), right, vn, vi
)
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
value, vi = DynamicPPL.tilde_assume(
DynamicPPL.childcontext(context), right, vn, vi
)
value, vi = DynamicPPL.tilde_assume(DynamicPPL.childcontext(context), right, vn, vi)

test/test_utils/ad_utils.jl
function DynamicPPL.tilde_observe(context::ADTypeCheckContext, right, left, vi)
logp, vi = DynamicPPL.tilde_observe(DynamicPPL.childcontext(context), right, left, vi)
function DynamicPPL.tilde_observe!!(context::ADTypeCheckContext, right, left, vn, vi)
left, vi = DynamicPPL.tilde_observe(DynamicPPL.childcontext(context), right, left, vn, vi)
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter v1.0.62] reported by reviewdog 🐶

Suggested change
left, vi = DynamicPPL.tilde_observe(DynamicPPL.childcontext(context), right, left, vn, vi)
left, vi = DynamicPPL.tilde_observe(
DynamicPPL.childcontext(context), right, left, vn, vi
)

@yebai
Copy link
Member

yebai commented May 18, 2025

Lots of interface code changes here are unnecessary and are planned to be removed in #2413

Perhaps we should address #2413 first.

@mhauru
Copy link
Member Author

mhauru commented May 20, 2025

That is tempting, to not waste time fixing code that's on its way out. I worry though that removing the samplers will take a while still, and in the meanwhile all the accumulator stuff, and other DPPL changes that build on it, would be held back from Turing.jl. For instance, introducing ValuesAsInModelAccumulator would cut our inference time in #2542 by half.

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

@github-actions github-actions[bot] github-actions[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@mhauru mhauru

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mhauru @yebai