Remove constraint on AbstractPDMat
Closes #1219 Tests it works by using BlockDiagonals.jl
Basic philosophy:
- Anything that worked with PDMats before should be just as fast
- Anything that didn't work before should be correct, but might be slower
- (e.g. we might recompute the cholesky everytime
randis called)
- (e.g. we might recompute the cholesky everytime
TODO
- [x]
logpdf - [x]
rand - [x]
sqmahal(d::MvNormal, x::AbstractVector) - [x]
sqmahal(d::MvNormal, x::AbstractMatrix) - [x]
MvNormalCanon - [x] https://github.com/JuliaStats/PDMats.jl/pull/162 needs to be merged and tagged
I think it would be great if it would be possible to use different matrices and types as covariance matrices.
I'm not happy about the added complexity in Distributions though and think there should be a better way to handle this. In particular, I think that similar generalizations are also desirable in other scenarios where currently AbstractPDMats are required.
Maybe we could instead add these fallbacks of invquad, invquad!, unwhiten! etc. for AbstractMatrix to PDMats instead? And then only loosen the restrictions in the types of e.g. MvNormal in Distributions?
I think that would make sense. It also would make it easier for other matrix packages to overload them.
One issue is I have no interest in implementing all of them, just like these 3, which is all we need for MvNormal.
@andreasnoack what do you think. Would such a PR be welcomed to PDMats.jl?
I would still want to add these test types here.
One thing I have shown here right now is the code changes are not hard or deep. Just a few operations. This is much easier than creating the work arounds.
This should now be good to go.
I started the registration process for PDMats 0.11.11: https://github.com/JuliaRegistries/General/pull/61420 Let's rerun tests when it's available.