Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add C implementation for math/base/special/spence #2298

Merged
merged 8 commits into from
Jun 7, 2024
Merged

feat: add C implementation for math/base/special/spence #2298

merged 8 commits into from
Jun 7, 2024

Conversation

gunjjoshi
Copy link
Member

@gunjjoshi gunjjoshi commented Jun 3, 2024

Resolves #894.

Description

What is the purpose of this pull request?

This pull request:

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@stdlib-bot stdlib-bot added the Math Issue or pull request specific to math functionality. label Jun 3, 2024
@gunjjoshi
Copy link
Member Author

While implementing this, a bit higher tolerance was required in the tests. I checked the implementation with that of cephes, but it was the same.

Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
Copy link
Member

@Planeshifter Planeshifter left a 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, will merge after CI checks have passed (updated @returns to @return in C function documentation.)

Signed-off-by: Athan <kgryte@gmail.com>
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

Overall, this is good; however, we should investigate the test tolerance discrepancy before merging.

@kgryte kgryte added the C Issue involves or relates to C. label Jun 3, 2024
@kgryte
Copy link
Member

kgryte commented Jun 3, 2024

This PR also supersedes #968. Thus, once this PR is merged, the other PR should be closed.

Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for working on this @gunjjoshi. I'll merge once CI passes.

@kgryte kgryte merged commit e382574 into stdlib-js:develop Jun 7, 2024
8 checks passed
@gunjjoshi gunjjoshi deleted the spence branch June 11, 2024 03:12
aman-095 pushed a commit to aman-095/stdlib that referenced this pull request Jun 13, 2024
PR-URL: stdlib-js#2298
Closes: stdlib-js#894
Closes: stdlib-js#968
Ref: stdlib-js#649
Co-authored-by: Athan Reines <kgryte@gmail.com>
Co-authored-by: Philipp Burckhardt <pburckhardt@outlook.com>
Reviewed-by: Athan Reines <kgryte@gmail.com>
Reviewed-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
Signed-off-by: Athan Reines <kgryte@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C Issue involves or relates to C. Math Issue or pull request specific to math functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: Add C implementation for @stdlib/math/base/special/spence
4 participants