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

Fix Rice distribution and add new parametrization (#3286) #3287

Closed
wants to merge 4 commits into from

Conversation

nbud
Copy link
Contributor

@nbud nbud commented Dec 4, 2018

Fix for #3286

I redefined the parametrization: nu is now the noncentrality parameter, b is the shape parameter. Before, nu was inconsistently the noncentrality or the shape parameter.

An extra test was added.

@nbud nbud mentioned this pull request Dec 4, 2018
@@ -3547,7 +3576,7 @@ def random(self, point=None, size=None):
"""
nu, sd = draw_values([self.nu, self.sd],
point=point, size=size)
return generate_samples(stats.rice.rvs, b=nu, scale=sd, loc=0,
return generate_samples(stats.rice.rvs, b=nu/sd, scale=sd, loc=0,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return generate_samples(stats.rice.rvs, b=nu/sd, scale=sd, loc=0,
return generate_samples(stats.rice.rvs, b=nu / sd, scale=sd, loc=0,

@twiecki
Copy link
Member

twiecki commented Dec 5, 2018

Thanks @nbud. Can you add this to the release-notes?

@nbud
Copy link
Contributor Author

nbud commented Dec 5, 2018

@twiecki Done and rebased

@twiecki
Copy link
Member

twiecki commented Dec 5, 2018

Great, can merge when tests pass. Thanks!

@nbud
Copy link
Contributor Author

nbud commented Dec 5, 2018

Looks like Travis failed due to a network error!

@ColCarroll
Copy link
Member

Restarted it!

@twiecki
Copy link
Member

twiecki commented Dec 6, 2018

Oops, I guess you wanted the order the other way around, can your ebase this one instead?

@nbud
Copy link
Contributor Author

nbud commented Dec 6, 2018

@twiecki Rebase where? Merging #3289 already committed the whole content of this PR to master. I reckon we should close this PR, feel free to reopen if you prefer!

@nbud nbud closed this Dec 6, 2018
@twiecki
Copy link
Member

twiecki commented Dec 6, 2018

Oh OK then!

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.

3 participants