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

Adding a generalised normal distribution #278

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jbrinchmann
Copy link

This pull request implements a generalized Gaussian distribution (https://en.wikipedia.org/wiki/Generalized_normal_distribution) - this is useful for situations where the distribution is close to Gaussian but with some kurtosis.

The distribution implemented is the symmetric version, but in keeping with the naming of the normal distribution I have not included this in the naming.

I have tried to follow the instructions for adding new distributions but for some reason when I do e.g.

In [1]: from pymc_experimental.distributions import GeneralizedNormal

In [2]: GeneralizedNormal.logcdf(1.5, mu=0.0, beta=1.3, alpha=0.5)
Out[2]: Log.0

which is an odd outcome since all intermediate steps are correct and the code works when embedded outside of pymc-experimental. As a consequence the tests do not appear to work.

…luded.

This follows broadly the Wikipedia page for notation - this differs slightly
from the implementation in scipy.stats, but the quantitative results are
identical as far as I can see.

This only implements the symmetric distribution - in accorandance with the
naming of normal distributions in pymc I have opted against including
Symmetric in the name. It would also be moderately easy to implement the
asymmetric distribution based on what is included here.
@ricardoV94
Copy link
Member

which is an odd outcome since all intermediate

This is normal! PyMC uses PyTensor which return symbolic expressions until actually evaluated.

You can call .eval() on the output to get the numerical value. More details in https://pytensor.readthedocs.io/en/latest/tutorial/adding.html


# moment here returns the mean
def moment(rv, size, mu, alpha, beta):
moment, _ = pt.broadcast_arrays(mu, alpha, beta)
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean this?

Suggested change
moment, _ = pt.broadcast_arrays(mu, alpha, beta)
moment, *_ = pt.broadcast_arrays(mu, alpha, beta)

Copy link
Member

Choose a reason for hiding this comment

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

Also missing the moment test

I had accidentally added an extra division by two in logp and a similar in logcdf.
The results from the codes have been double-checked  with scipy.stats.gennorm
@jbrinchmann
Copy link
Author

Thanks for the feedback - in trying to get things working I had introduce two minor bugs in logp and logcdf. These are now fixed and double-checked to agree with scipy.stats.gennorm and a moment test has been added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants