Open eigenfoo opened 5 years ago
LKJ in tfp is the same as LKJCorr.
Edited!
Update: #46 merges in some more random variables (those that can be created via tfp's bijectors and transformed variables). There are still quite a few random variables outstanding, though.
@eigenfoo your PR gives me a template to work off! I'll see how fast I can get setup and running for development at work.
FYI: LKJ in tfp now has Cholesky parameterization https://github.com/tensorflow/probability/commit/9b007d8e52074fe973cf06290cf5411274636834
I haven't contributed yet but would be interested to have a go with helping with some of these. Looking at the code base it seems a lot of these still are not implemented, so it is still open, correct?
@tblazina yes, still open! I recently learned a new pattern at this year's PyCon, which helps with tracking progress. Was wondering what the other devs think of this - when putting in a PR, put in also an issue that we can close, and reference it in the branch name (e.g. distribution-flat-#93
)? I think there's github magic that we can take advantage of :smile:
@ericmjl, ok great, thanks for quick response! I'll try to get dev environment all set up and see if I can get plugging away at some of this. I think your idea about tracking progress sounds to me and was along the lines of my next question, is that for a big issue like this with lots of different parts, is the best practice in this project to submit a PR for each distribution that we implement and simply reference this issue in the PR?
is the best practice in this project to submit a PR for each distribution that we implement and simply reference this issue in the PR?
That is how I would have done it before this year's PyCon. This kind of issue is like an over-arching one, which multiple other smaller issues can reference. Hence, I think it's more appropriate to reference the smaller issues, so that you can take advantage of the auto-closing keywords (e.g. "resolves #391")
Ok, well for now (until there is a bit of consensus), I proceed as was done before, referencing the issue in the PR. Don't want to step on any toes if I'm just starting to contribute :smile:, but I do like the idea of opening smaller issues and referencing those in the PR.
Filing this as an issue so we don't forget: https://github.com/pymc-devs/pymc4/pull/41#issuecomment-453775302
I think we can achieve most of the important distributions using
tfp.distributions.Mixture
andtfp.distributions.Bijector
(e.g. the zero-inflated distributions are just mixtures with the zero distribution).@ColCarroll I'm assuming there will be a problem with having a distribution/RV named
Deterministic
? 😄 That's what tfp calls the thing that PyMC calls theConstant
distribution. If so, I can write something to remap names.Continuous:
Discrete:
Multivariate:
Timeseries: