Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement generalized gamma distribution #1059

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

kylejcaron
Copy link
Contributor

@kylejcaron kylejcaron commented Jul 18, 2022

Thank you for opening a PR!

This PR implements an Op for a Generalized Gamma Random Variable with the parameterization GG(alpha, p, lambd). This corresponds to scipy in the following mapping: scipy.stats.gengamma(a=alpha/p, c=p, scale=lambd).

This goes along with the following PyMC PR to implement the Generalized Gamma distribution

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

  • There is an informative high-level description of the changes.
  • The description and/or commit message(s) references the relevant GitHub issue(s).
  • pre-commit is installed and set up.
  • The commit messages follow these guidelines.
  • The commits correspond to relevant logical changes, and there are no commits that fix changes introduced by other commits in the same branch/BR.
  • There are tests covering the changes introduced in the PR.

Don't worry, your PR doesn't need to be in perfect order to submit it. As development progresses and/or reviewers request changes, you can always rewrite the history of your feature/PR branches.

If your PR is an ongoing effort and you would like to involve us in the process, simply make it a draft PR.

@kylejcaron kylejcaron force-pushed the implement_gen_gamma branch from ade3d5a to 052447b Compare July 18, 2022 15:01
@kylejcaron kylejcaron marked this pull request as ready for review July 18, 2022 15:03
@codecov
Copy link

codecov bot commented Jul 18, 2022

Codecov Report

Merging #1059 (7fa673c) into main (8794f48) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 7fa673c differs from pull request most recent head d9ede7d. Consider uploading reports for the commit d9ede7d to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1059   +/-   ##
=======================================
  Coverage   79.25%   79.25%           
=======================================
  Files         152      152           
  Lines       47875    47887   +12     
  Branches    10906    10907    +1     
=======================================
+ Hits        37942    37954   +12     
  Misses       7436     7436           
  Partials     2497     2497           
Impacted Files Coverage Δ
aesara/tensor/random/basic.py 98.92% <100.00%> (+0.02%) ⬆️

@brandonwillard brandonwillard added random variables Involves random variables and/or sampling Op implementation Involves the implementation of an Op enhancement New feature or request labels Jul 18, 2022
Copy link
Member

@brandonwillard brandonwillard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, although the commits need to be squashed/rebased before merging (because there's a merge commit in this branch).

@kylejcaron kylejcaron force-pushed the implement_gen_gamma branch from 7fa673c to d9ede7d Compare July 18, 2022 16:34
@kylejcaron
Copy link
Contributor Author

Looks good, although the commits need to be squashed/rebased before merging (because there's a merge commit in this branch).

just rebased and squashed! Any advice on how to coordinate this PR with the corresponding PyMC PR?

@brandonwillard
Copy link
Member

Any advice on how to coordinate this PR with the corresponding PyMC PR?

You can increment the required Aesara version in your PyMC PR; that will at least guarantee that your changes will only apply to a version of Aesara with these changes.

@brandonwillard brandonwillard merged commit 9e0434b into aesara-devs:main Jul 19, 2022
@kylejcaron kylejcaron deleted the implement_gen_gamma branch July 19, 2022 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Op implementation Involves the implementation of an Op random variables Involves random variables and/or sampling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants