Came across this issue in TFP: https://github.com/tensorflow/probability/issues/400 and found out that our transformation of LKJ is just a interval transformation - that's incorrect as it will produce invalid correlation matrix right?
https://github.com/pymc-devs/pymc3/blob/master/pymc3/distributions/multivariate.py#L1158-L1159
Our LKJCorr will return a logp of -inf in some cases, yes. The proper transformation was hard to implement in theano, that's why I wrote LKJCholescyCov.
Unless we implement it however, we should probably deprecate pm.LKJCorr.
That makes sense, should we revisit the transformation for correlation matrix now since there is a version in TFP? It would need some theano.scan magic but it should be doable.
Proposal:
LKJCorrCholesky (or add a Cholesky parameterization to LKJCorr and make that the default)CorrCholesky transformationLKJCorrRegarding the transformation, we can opt to use the simplier version in TFP. Some related discussion see:
https://github.com/tensorflow/probability/issues/694#issuecomment-599192616
Most helpful comment
Proposal:
LKJCorrCholesky(or add a Cholesky parameterization toLKJCorrand make that the default)CorrCholeskytransformationLKJCorrRegarding the transformation, we can opt to use the simplier version in TFP. Some related discussion see:
https://github.com/tensorflow/probability/issues/694#issuecomment-599192616