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

Rice distribution is misdefined #3286

Closed
nbud opened this issue Dec 4, 2018 · 2 comments
Closed

Rice distribution is misdefined #3286

nbud opened this issue Dec 4, 2018 · 2 comments

Comments

@nbud
Copy link
Contributor

nbud commented Dec 4, 2018

Description of your problem

The Rice distribution Rice(nu, sigma) is the distribution of sqrt(X^2+Y^2) where X~N(nu, sigma^2) and Y~N(nu, sigma^2), with X and Y independent. This parametrisation is the one notably used in the Wikipedia article on Rice distribution.

scipy.stats.rice used a different distribution: shape parameter b=nu/sigma and scale sigma.

The current Rice distribution in pymc inconsistently mixes the two parametrisations: nu is sometimes the location parameter of the normal distributions, sometimes the shape parameter.

import pymc3 as pm
from scipy import stats
import matplotlib.pyplot as plt
import numpy as np

sigma = 5
nu = 3

u = pm.distributions.Rice.dist(nu, sigma)
v = stats.rice(b=nu/sigma, scale=sigma)

np.random.seed(123)
mean_u = u.mean.eval()

n = 10000
samples_u = u.random(size=n)
samples_v = v.rvs(size=n)

plt.figure()
plt.hist(samples_u, density=True, bins=50, label="pymc3.Rice")
plt.hist(samples_v, density=True, bins=50, label="scipy Rice dist")
plt.axvline(mean_u, label="pymc3.Rice.mean", color="C2")
plt.axvline(np.mean(samples_u), label="empirical mean of pymc3.Rice", color="C3")
plt.legend()

assert np.isclose(mean_u, v.mean())  # pass

figure_1

I'm writing a PR to fix this.

@twiecki
Copy link
Member

twiecki commented Dec 4, 2018

Good catch!

twiecki pushed a commit that referenced this issue Dec 6, 2018
* Fix Rice distribution and add new parametrization (#3286)

* fix math error in the docstring introduced by prev commit

* code format

* Update RELEASE-NOTES.md

* Add i1e and i0e.grad

* elemwise i0e and i1e, does not work

* elemwise i0e and i1e

* Rice now accepts tensor parameters

* update RELEASE-NOTES.md
@nbud
Copy link
Contributor Author

nbud commented Dec 6, 2018

Closed by #3289, see also #3287

@nbud nbud closed this as completed Dec 6, 2018
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

No branches or pull requests

2 participants