-
-
Notifications
You must be signed in to change notification settings - Fork 794
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
feat: add C implementation for math/base/special/gammaln
#2636
Conversation
Yes, I think that would be good. |
I've made changes in both |
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.
Let's land this PR.
I investigated and found that Julia is way closer to our implementation (8.5 * EPS
to get tests to pass), so that the large tolerances are due to R using a different algorithm. We should add Julia test fixtures and use them in the unit tests, but that can be done in a follow-up PR.
Thanks for checking this @Planeshifter! I'll make the changes in a follow-up PR. |
@gunjjoshi Thank you! It may also be useful to add Julia fixtures to |
PR-URL: stdlib-js#2636 Ref: stdlib-js#649 Reviewed-by: Philipp Burckhardt <[email protected]> Reviewed-by: Athan Reines <[email protected]>
Resolves #1988.
Description
This pull request:
math/base/special/gammaln
.Related Issues
This pull request:
Questions
Shall I change both
test.js
andtest.native.js
to use tolerance of the form1.0 * EPS
here? We have a high amount of tolerance required here as well, just likemath/base/special/gamma
.Other
No.
Checklist
@stdlib-js/reviewers