-
-
Notifications
You must be signed in to change notification settings - Fork 834
feat: add C implementation for math/base/special/gammaln
#2636
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
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 <pburckhardt@outlook.com> Reviewed-by: Athan Reines <kgryte@gmail.com>
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