Closed marius311 closed 9 months ago
Of course, allocations would be minimized if MvNormal
just takes whatever types the user puts into the struct. But I assume then it becomes much more likely to run into type instability issues and you have to be much more careful when implementing methods operating with MvNormal
.
Regardless though, the unnecessary cholesky
decompositions are not caused by inefficiences or bugs in Distributions but rather inefficient and missing convert
definitions in PDMats. https://github.com/JuliaStats/PDMats.jl/pull/179 fixes your example.
In a typical Turing model even if you tried to pre-convert an MvNormal covariance to a PDMat, it'll still get cholseky-ed every gradient call if using ForwardDiff:
Imo the problem is the MvNormal constructor is a little too greedy promoting things when the eltypes don't match, and in the process recomputing cholesky (in this case the mean is Duals but the covariance is Floats).
A workaround is for the user to use the
MvNormal{T,Cov,Mean}(...)
constructor but this seems like an easy and potentially big performance footgun for users, even ones who were smart enough to try to manually do PDMat, and would be nice to fix (in some package, my sense is here, but maybe elsewhere).