-
Notifications
You must be signed in to change notification settings - Fork 421
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
Inverse / normal-inverse gaussian distributions #954
Inverse / normal-inverse gaussian distributions #954
Conversation
assert!(InverseGaussian::new(-1.0, -1.0).is_err()); | ||
assert!(InverseGaussian::new(1.0, -1.0).is_err()); | ||
assert!(InverseGaussian::new(1.0, 1.0).is_ok()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have a test for value stability, but I guess that can also be added later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a value stability test by trying to reproduce what was done for the other distributions but I'm not sure of what is to be tested
assert!(NormalInverseGaussian::new(-1.0, -1.0).is_err()); | ||
assert!(NormalInverseGaussian::new(1.0, 2.0).is_err()); | ||
assert!(NormalInverseGaussian::new(2.0, 1.0).is_ok()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, a value stability test could be added.
I did not verify whether the sampling is correct, but the code looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like carefully written code, and I was able to check at least that the Inverse Gaussian sampling algorithm matches that on Wikipedia.
@vks I checked the sampling of the normal-inverse Gaussian distribution manually by generating a bunch of points, fitted them using scipy's norminvgauss function and checked that the parameters were the same. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks a lot!
Discussed in #952 .