You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is a proof-of-concept for #1038; it's not mergeable yet. I would want a few things to be changed for this, but the basic concept is there:
A unified API in PosteriorStats
Moving it to a PosteriorStats extension (PS brings in a lot of deps)
Implementing it for the chains methods
Tests
Docs
cc @sethaxen: Especially regarding PosteriorStats being a relatively heavy dep, is there any reasonable chance that this functionality could be extracted into a separate package that depends only on Distributions + basic stats stuff?
I wonder in general if turning PosteriorStats into a monorepo-like structure would quite useful for encouraging extensions? You could have a number of small repos that define units of functionality, and PosteriorStats could just be an aggregator that re-exports all of them. That would make it easy for other packages to hook into bits of it as needed, without having to pick up the deps from the other bits that they don't need.
The thing is that this is quite neat and I'd really like to have it in the main DynamicPPL package rather than an extension, since extensions have much poorer discoverability!
julia>using DynamicPPL, Distributions, LinearAlgebra
julia>@modelfunctionf(y)
x ~MvNormal(zeros(2), I)
y ~MvNormal(zeros(2), I)
end
f (generic function with 2 methods)
julia> model =f([1.0, 1.0])
Model{typeof(f), (:y,), (), (), Tuple{Vector{Float64}}, Tuple{}, DefaultContext, false}(f, (y = [1.0, 1.0],), NamedTuple(), DefaultContext())
julia> vi =VarInfo(model)
VarInfo
├─ transform_strategy:UnlinkAll()
├─ values
│ VarNamedTuple
│ └─ x =>VectorValue{Vector{Float64}, Bijectors.VectorBijectors.TypedIdentity}([0.3793074864362537, -1.0132794015923987], Bijectors.VectorBijectors.TypedIdentity())
└─ accs
AccumulatorTuple with 3 accumulators
├─ LogPrior =>LogPriorAccumulator(-2.423181723888365)
├─ LogJacobian =>LogJacobianAccumulator(0.0)
└─ LogLikelihood =>LogLikelihoodAccumulator(-2.8378770664093453)
julia>pointwise_logdensities(model, vi)
VarNamedTuple
├─ x =>-2.423181723888365
└─ y =>-2.8378770664093453
julia>pointwise_logdensities(model, vi; factorize=true)
VarNamedTuple
├─ x => [-0.9908756178379672, -1.4323061060503974]
└─ y => [-1.4189385332046727, -1.4189385332046727]
❌ Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.38%. Comparing base (34b8230) to head (aa2e6e4).
cc @sethaxen: Especially regarding PosteriorStats being a relatively heavy dep, is there any reasonable chance that this functionality could be extracted into a separate package that depends only on Distributions + basic stats stuff?
Yeah, I think we can do that. Even in testing the functionality, I needed to implement much of the machinery needed to compute arbitrary conditional/marginal distributions. There have been longstanding issues open for each of these features in Distributions, and they probably ultimately belong there, but since that's not likely to happen soon, I think it could make sense to have a PartitionedDistributions.jl package that implements at least marginal, conditional, and pointwise_conditional_logpdfs (AKA pointwise_conditional_loglikelihoods). I could start by just putting the latter in such a package and register it and then add the rest of the features later.
Once sethaxen/PartitionedDistributions.jl#5 is merged and there are some basic docs, I'll make a release and register it. The easier marginal/conditional implementations are already merged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a proof-of-concept for #1038; it's not mergeable yet. I would want a few things to be changed for this, but the basic concept is there:
cc @sethaxen: Especially regarding PosteriorStats being a relatively heavy dep, is there any reasonable chance that this functionality could be extracted into a separate package that depends only on Distributions + basic stats stuff?
I wonder in general if turning PosteriorStats into a monorepo-like structure would quite useful for encouraging extensions? You could have a number of small repos that define units of functionality, and PosteriorStats could just be an aggregator that re-exports all of them. That would make it easy for other packages to hook into bits of it as needed, without having to pick up the deps from the other bits that they don't need.
The thing is that this is quite neat and I'd really like to have it in the main DynamicPPL package rather than an extension, since extensions have much poorer discoverability!