aesara-devs / aesara

Aesara is a Python library for defining, optimizing, and efficiently evaluating mathematical expressions involving multi-dimensional arrays.
https://aesara.readthedocs.io
Other
1.17k stars 156 forks source link

Add JAX implementation for `InvGammaRV` #1480

Closed PaulScemama closed 1 year ago

PaulScemama commented 1 year ago

This PR is a draft to close https://github.com/aesara-devs/aesara/issues/1368. Furthermore, I had to change the .pre-commit-config.yaml slightly to pass the mypy check. This bug has been discussed in https://github.com/aesara-devs/aesara/discussions/1474#discussioncomment-5362160.

@brandonwillard @rlouf let me know what you both think regarding the types-setuptools issue. If it hasn't affected anyone else then I would assume you wouldn't want the change to the .pre-commit-config.yaml.

Here are a few important guidelines and requirements to check before your PR can be merged:

EDIT:

Something that I didn't realize in the beginning...from wolfram:

"the inverse gamma distribution with shape parameter $\alpha$ and scale parameter $\beta$ is the distribution followed by the inverse of a gamma distribution with shape parameter $\alpha$ and scale parameter $\color{red} 1 / \beta$."

Could be useful to say something like:

"the InvGamma(shape, scale) is equivalent to taking the reciprocal of samples from a Gamma(shape, 1 / scale) distribution" in the docs.

codecov[bot] commented 1 year ago

Codecov Report

Merging #1480 (0b66d2a) into main (4a687c0) will increase coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head 0b66d2a differs from pull request most recent head eda2f50. Consider uploading reports for the commit eda2f50 to get more accurate results

Additional details and impacted files [![Impacted file tree graph](https://codecov.io/gh/aesara-devs/aesara/pull/1480/graphs/tree.svg?width=650&height=150&src=pr&token=2HNzVWyxrA&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs)](https://codecov.io/gh/aesara-devs/aesara/pull/1480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs) ```diff @@ Coverage Diff @@ ## main #1480 +/- ## ======================================= Coverage 75.06% 75.06% ======================================= Files 194 194 Lines 50091 50101 +10 Branches 12096 12097 +1 ======================================= + Hits 37600 37610 +10 Misses 10170 10170 Partials 2321 2321 ``` | [Impacted Files](https://codecov.io/gh/aesara-devs/aesara/pull/1480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs) | Coverage Δ | | |---|---|---| | [aesara/link/jax/dispatch/random.py](https://codecov.io/gh/aesara-devs/aesara/pull/1480?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs#diff-YWVzYXJhL2xpbmsvamF4L2Rpc3BhdGNoL3JhbmRvbS5weQ==) | `100.00% <100.00%> (ø)` | |
brandonwillard commented 1 year ago

"the InvGamma(shape, scale) is equivalent to taking the reciprocal of samples from a Gamma(shape, 1 / scale) distribution" in the docs.

Yes, definitely.

brandonwillard commented 1 year ago

This PR is a draft to close #1368. Furthermore, I had to change the .pre-commit-config.yaml slightly to pass the mypy check. This bug has been discussed in #1474 (reply in thread).

Is that still an issue now that https://github.com/aesara-devs/aesara/pull/1482 is merged?

PaulScemama commented 1 year ago

This PR is a draft to close #1368. Furthermore, I had to change the .pre-commit-config.yaml slightly to pass the mypy check. This bug has been discussed in #1474 (reply in thread).

Is that still an issue now that #1482 is merged?

It is not an issue anymore.

brandonwillard commented 1 year ago

Thanks a lot, @PaulScemama!

PaulScemama commented 1 year ago

Thank you! @brandonwillard I learned a lot.