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

DOC: clarify parameters of norminvgauss in scipy.stats #14592

Merged
merged 2 commits into from
Aug 25, 2021

Conversation

chrisb83
Copy link
Member

Reference issue

Closes gh-14568

What does this implement/fix?

Explain alternative parametrization of norminvgauss

Additional information

I updated the tutorial and documentation of norminvgauss

@chrisb83 chrisb83 added scipy.stats Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org labels Aug 17, 2021
@chrisb83 chrisb83 added this to the 1.8.0 milestone Aug 17, 2021
@josef-pkt
Copy link
Member

LGTM

That's helpful
I didn't check the math.

@mdhaber
Copy link
Contributor

mdhaber commented Aug 17, 2021

The math looks right. We could spend a lot of time trying to get the appearance of code and math right and make the correspondence (e.g. between α and alpha) explicit, but it's really hard to satisfy everyone, so I'll ignore it.

@Nishan-Pradhan can you confirm that this resolves the issue? You mentioned that you did some tests; can you paste some of that here - an example of TF code, equivalent SciPy code according to the note in this PR, and results that show they're equivalent? If so, I'll go ahead and merge.

Thank you!

@Nishan-Pradhan
Copy link

Hi @mdhaber, I was unable to compare the SciPy implementation to TF as it seems the TF NormalInverseGaussian is not yet implemented fully, as confirmed here: tensorflow/probability#1398

I have however managed to match up the results of SciPy transformation to an Excel based implementation of the NIG distribution using the mu and delta parameters which leads me to believe that the transformation is correct.

@mdhaber mdhaber merged commit 2ec41b5 into scipy:master Aug 25, 2021
@mdhaber
Copy link
Contributor

mdhaber commented Aug 25, 2021

Thanks @chrisb83 @josef-pkt @Nishan-Pradhan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org scipy.stats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stats.norminvgauss incorrect implementation?
4 participants