Adopt DensityInterface.jl #41
Conversation
Codecov Report
@@ Coverage Diff @@
## main #41 +/- ##
==========================================
- Coverage 77.77% 77.06% -0.72%
==========================================
Files 1 2 +1
Lines 108 109 +1
==========================================
Hits 84 84
- Misses 24 25 +1
Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build 1440131306Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
That should be fine for now. We don't have any support for likelihood-free inference yet. But even for likelihood-free inference, one often uses an approximate likelihood/loss function. So we can still use the density interface. |
|
I think this supports for objects supporting |
|
Yes, this is one of the main reasons for a unified interface. However, it will require some changes in DynamicPPL since we don't use |
|
That's why I am asking, in the current form of JuliaStats/Distributions.jl#1416 using |
|
Yes, that's something we have to address in the future in some way 🙂 Currently, Distributions is baked in in DynamicPPL and Turing very deeply, so it will take some time and effort to generalize it regardless of |
|
TBH, I had no idea we used julia> @model function foo(x)
m ~ Normal()
x ~ Normal(m)
end
foo (generic function with 2 methods)
julia> foo(1.2)()
1.2
julia> foo([1.2, 0.2])()
2-element Vector{Float64}:
1.2
0.2Is this really a good thing? It definitely makes the type checker in my brain complain. |
|
It is a good idea since before we had to maintain all kinds of custom dispatches for different distributions (but of course would miss any others) and always allocated arrays for the individual log densities which then were summed immediately. It is much more sane to let it implement whoever defines a Distribution type. Edit: I think it is also useful in your example since otherwise, if the distribution on the RHS depends on the number of samples, it seems you would have to artificially wrap the single observation in a vector or to introduce a type instability ( Generally, type-wise |
|
But I would suggest not polluting this PR with the seemingly unrelated discussion 🙂 |
|
"Polluting" perhaps don't use that loaded language. |
|
I'm sorry, I was in a rush and assume it was a bit strong. I only wanted to say that it seemed to distract from this PR and the discussion felt unrelated to this PR 🙂 |
|
No problem, I just wanted to signal that the statement touched a nerve with me on the first take. |
This is in the hope of finally having a shared logdensity function across "probabilistic packages". Distributions.jl is already pretty much on board, Soss.jl hopefully will join later as soon as questions about how to integrate this with properly done measure theory (implicit/explicit base measures) have been resolved.
The consensus was to let the name be
logdensityof(anddensityof) to avoid name clashes. The interface is intended for both types that "have a density" and "are a density".The other part of DensityInterface.jl is a trait,
hasdensity, which forAbstractProbabilisticModelI have defaulted totrue. I think likelihood-free inference is out of scope for the kind of models we target -- is that a reasonable assumption?See JuliaStats/Distributions.jl#1416, JuliaMath/DensityInterface.jl#1 and JuliaMath/DensityInterface.jl#3.