Adopting DensityInterface

[phipsgabler]

Implements #340

TODO:

• [x] Upgrade to AbstractPPL 0.4 as soon as released
• [x] Require Distributions 0.25.25
• [ ] Rename logdensity to logdensityof (deprecating the former), check interface compliance
• [x] Switch Distribution subtype check in compiler to use hasdensity trait
• [ ] Add IIDDensity (https://github.com/JuliaStats/Distributions.jl/pull/1416#issuecomment-964224841)
• [ ] Switch from loglikelihood to logdensityof with IIDDensity
• [ ] Wait for preparations in Bijectors/ChangesOfVariables (cf. https://github.com/TuringLang/Bijectors.jl/issues/210) and implement whatever changes happen to Bijectors.logpdf_with_trans

[phipsgabler]

Distributions 0.25.25 is the release where DensityInterface is introduced there. I guess we don't want to explicitely restrict on that; instead, should be able to just specialize distributions for now, and

The bigger "problem" is that logdensity and logpdf are nowhere used. Instead, it is Bijectors that hard-depends on distributions:

$ grep -rn 'logpdf' src 
src/submodel_macro.jl:42:julia> getlogp(vi) ≈ logpdf(Normal(), x) + logpdf(Uniform(0, 1 + abs(x)), 0.4)
src/submodel_macro.jl:107:julia> logprior = logpdf(Normal(), sub1_x) + logpdf(Normal(), sub2_x);
src/submodel_macro.jl:109:julia> loglikelihood = logpdf(Uniform(-1 - abs(sub1_x), 1 + abs(sub2_x)), 0.4);
src/context_implementations.jl:198:    return r, Bijectors.logpdf_with_trans(dist, r, istrans(vi, vn))
src/context_implementations.jl:226:    return r, Bijectors.logpdf_with_trans(dist, r, istrans(vi, vn))
src/context_implementations.jl:393:        return Bijectors.logpdf_with_trans(dist, ri, istrans(vi, vn))
src/context_implementations.jl:408:    lp = sum(Bijectors.logpdf_with_trans(dist, r, istrans(vi, vns[1])))
src/context_implementations.jl:425:    lp = sum(Bijectors.logpdf_with_trans.(dists, r, istrans(vi, vns[1])))
src/context_implementations.jl:439:    lp = sum(Bijectors.logpdf_with_trans.(dists, r, istrans(vi, vns[1])))
src/distribution_wrappers.jl:23:Distributions.logpdf(d::NoDist{<:Univariate}, ::Real) = 0
src/distribution_wrappers.jl:24:Distributions.logpdf(d::NoDist{<:Multivariate}, ::AbstractVector{<:Real}) = 0
src/distribution_wrappers.jl:25:function Distributions.logpdf(d::NoDist{<:Multivariate}, x::AbstractMatrix{<:Real})
src/distribution_wrappers.jl:28:Distributions.logpdf(d::NoDist{<:Matrixvariate}, ::AbstractMatrix{<:Real}) = 0
src/distribution_wrappers.jl:32:Bijectors.logpdf_with_trans(d::NoDist{<:Univariate}, ::Real) = 0
src/distribution_wrappers.jl:33:Bijectors.logpdf_with_trans(d::NoDist{<:Multivariate}, ::AbstractVector{<:Real}) = 0
src/distribution_wrappers.jl:34:function Bijectors.logpdf_with_trans(d::NoDist{<:Multivariate}, x::AbstractMatrix{<:Real})
src/distribution_wrappers.jl:37:Bijectors.logpdf_with_trans(d::NoDist{<:Matrixvariate}, ::AbstractMatrix{<:Real}) = 0

Technically this shifts the burden to implementors of DensityInterface to also implement logpdf_with_trans, then, and of course doesn't change anything wrt. Distributions. But the nomenclature gets inconsistent (pdf vs. densityof). Not sure what to do about that.

@torfjelde were there any big hurdles other than the bijetors when you tried to make DPPL work with Soss?

For comparison:

$ grep -rn 'loglikelihood(' src
src/model.jl:515:    loglikelihood(model::Model, varinfo::AbstractVarInfo)
src/model.jl:521:function Distributions.loglikelihood(model::Model, varinfo::AbstractVarInfo)
src/context_implementations.jl:233:    return Distributions.loglikelihood(right, left)
src/context_implementations.jl:604:    return Distributions.loglikelihood(dist, value)
src/context_implementations.jl:608:    return Distributions.loglikelihood(dists, value)
src/prob_macro.jl:29:        return loglikelihood(ex1, ex2, model, vi)
src/prob_macro.jl:188:function Distributions.loglikelihood(

[phipsgabler]

So @torfjelde should logpdf_with_trans just be kept for now, or what will the replacement in Bijecctors be?

[torfjelde]

So @torfjelde should logpdf_with_trans just be kept for now, or what will the replacement in Bijecctors be?

It'll be the same for now: https://github.com/TuringLang/Bijectors.jl/pull/212

[coveralls]

Pull Request Test Coverage Report for Build 8974831544

Details

• 4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 81.512%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/compiler.jl	4	5	80.0%
<!--	Total:	4	5

Totals
Change from base Build 8974235232:	0.01%
Covered Lines:	2879
Relevant Lines:	3532

---

💛 - Coveralls

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 76.38%. Comparing base (d5ae280) to head (0351101).

:exclamation: Current head 0351101 differs from pull request most recent head 3760e45. Consider uploading reports for the commit 3760e45 to get more accurate results

Files	Patch %	Lines
src/compiler.jl	80.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #342      +/-   ##
==========================================
- Coverage   76.64%   76.38%   -0.26%     
==========================================
  Files          30       21       -9     
  Lines        3288     2524     -764     
==========================================
- Hits         2520     1928     -592     
+ Misses        768      596     -172     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.