Road to DensityInterface?

[phipsgabler]

It turns out that most of DynamicPPL's dependency on Distributions is really through logpdf_with_trans, see TuringLang/DynamicPPL.jl#342.

Not knowing much about Bijectors: what needs to be done to make the change to things which are generic in DensityInterface? Is a logdensity_with_trans something feasible? Or must th "interface merge" happen at the level of the new ChangeOfVariables package?

CC @oschulz @torfjelde @devmotion

[oschulz]

@torfjelde correct me if I'm wrong: I don't think anything needs to be done in Bijectors.jl regarding DensityInterface.jl.

Instead, we need to support InverseFunctions.jl and ChangesOfVariables.jl in Bijectors: #199

[devmotion]

I agree, this was my impression as well. Most issues should be mentioned in the linked issue and the issue in ChangesOfVariables. Eg, Bijectors allows multiple samples implicitly whereas ChangesOfVariables makes application to multiple samples explicit with map or broadcast.

(BTW @oschulz I just merged and released the ChainRulesCore fix which blocked the CRTestutils PR which blocks the ChangesOfVariables PR - so hopefully the test errors are fixed soon 🙂)

It turns out that most of DynamicPPL's dependency on Distributions is really through logpdf_with_trans

Many samplers (such as eg AdvancedMH or EllipticalSliceSampling) have explicit dependencies on Distributions as well, so some additional work is needed even if DynamicPPL and Bijectors support other implementations of DensityInterface.

[oschulz]

Regarding AdvancedMH and EllipticalSliceSampling I guess the dependencies or more internal (proposal distributions and their tuning and so on) and not related to that target functions they'll accept?

I guess AdvancedMH could easily be extended to support anything that implements DensityInterface, in addition AdvancedMH.DensityModel?

AdvancedMH.DensityModel is actually pretty much exactly what DensityInterface.LogFuncDensity now provides, so AdvancedMH.DensityModel could just be deprecated/removed in future versions of AdvancedMH.

Maybe AbstractMCMC.AbstractModel could even go? Bit off topic-here, though.

[phipsgabler]

OK so that means it's basically allowed to assume that logpdf_with_trans is already compatible with non-Distribution density objects implementing DensityInterface, which additionally provide the necessary transformation routines?

If I got this right:

logpdf_with_trans(d, x, false) 
    == logdensityof(d, x)
    == with_logabdet_jacobian(d, x)[1]

logpdf_with_trans(d, x, true) 
    == logdensityof(transformed(d), x)
    == with_logabsdet_jacobian(transformed(d), x)[1]
    == logdensityof(d, x) - with_logabsdet_jacobian(bijector(d), x)[2]

? (Please check my undestanding here -- as I have said I'm new to this corner...) How will this look in practice -- can there be some defaulting? Must DPPL change from programming agains logpdf_with_trans to with_logabsdet_jacobian?

Many samplers (such as eg AdvancedMH or EllipticalSliceSampling) have explicit dependencies on Distributions as well, so some additional work is needed even if DynamicPPL and Bijectors support other implementations of DensityInterface.

Yeah, right, I was only referring to the interface part in DPPL.

[oschulz]

No, I think

logpdf_with_trans(d, x, false) 
    == logdensityof(d, x)

logpdf_with_trans(d, x, true) 
    == logdensityof(transformed(d), x)
    == logdensityof(d, x) - with_logabsdet_jacobian(bijector(d), x)[2]

would be correct, but the wouldn't be a with_logabdet_jacobian(d, x) or with_logabsdet_jacobian(transformed(d), x) in there. with_logabsdet_jacobian operates on transformation functions, not on distributions/densities/measures.

I'm not an expert on the Turing ecosystem at all, but the interface of logpdf_with_trans strikes me as suboptimal - it seems prone to type instability and AD-overhead due to the conditional statements necessary to handle the transform::Bool argument.

[oschulz]

Must DPPL change from programming agains logpdf_with_trans to with_logabsdet_jacobian

I expect Bijectors would continue to provide logpdf_with_trans without any changes. As for using it in DynamicPPL: I'm not an expert on the Turing ecosystem at all, but the interface of logpdf_with_trans strikes me as suboptimal - it seems prone to type instability and AD-overhead due to the conditional statements necessary to handle the transform::Bool argument.

I don't know DPPL well enough to understand how much it will needs to depend on Bijectors itself in the long run, now that we have ChangesOfVariables, InverseFunctions and DensityInterface. It would certainly be nice if the models created with DPPL themselves would support DensityInterface, of course.

[phipsgabler]

That'd be the plan, yes. I think @torfjelde has plans to change DPPL from "internal" automatic transformations to make them explicit and external, right?

[torfjelde]

I expect Bijectors would continue to provide logpdf_with_trans without any changes. As for using it in DynamicPPL: I'm not an expert on the Turing ecosystem at all, but the interface of logpdf_with_trans strikes me as suboptimal - it seems prone to type instability and AD-overhead due to the conditional statements necessary to handle the transform::Bool argument.

Yup. It's left-over from an older code-base that we haven't gotten to deprecating properly yet (exactly because it requires a lot of changes upstream).

That'd be the plan, yes. I think @torfjelde has plans to change DPPL from "internal" automatic transformations to make them explicit and external, right?

Not by default, but allowing this, yes:)

[phipsgabler]

OK, thanks everyone. For the sake of not mixing DPPL refactoring and Bijectors discussions too much:

• could someone list the concrete steps needing to be taken here or in ChangesOfVariables, and
• @torfjelde could you comment on the linked DPPL issue what changes are required there, especially what to use in the future instead of logpdf_with_trans if it's "almost deprecated", ensuring DensityInterface generality?

I'm also ready to get involved in PRs here if that speeds things up, someone just needs to tell me what to do.

[devmotion]

could someone list the concrete steps needing to be taken here or in ChangesOfVariables, and

I think this is discussed and possibly can be concretized in #199.

[phipsgabler]

Great, then I will close this one.