Skip to content

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

Merged
kgryte merged 8 commits intostdlib-js:developfrom
gunjjoshi:spence
Jun 7, 2024
Merged

feat: add C implementation for math/base/special/spence#2298
kgryte merged 8 commits intostdlib-js:developfrom
gunjjoshi:spence

Conversation

@gunjjoshi
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread lib/node_modules/@stdlib/math/base/special/spence/src/main.c Outdated
Signed-off-by: Philipp Burckhardt <pburckhardt@outlook.com>
Copy link
Copy Markdown
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.)

Comment thread lib/node_modules/@stdlib/math/base/special/spence/lib/native.js Outdated
Signed-off-by: Athan <kgryte@gmail.com>
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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