Skip to content
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 math/base/special/acotf #2117

Merged
merged 5 commits into from
Apr 4, 2024
Merged

feat: add math/base/special/acotf #2117

merged 5 commits into from
Apr 4, 2024

Conversation

gunjjoshi
Copy link
Member

@gunjjoshi gunjjoshi commented Apr 4, 2024

Resolves #2113.

Description

What is the purpose of this pull request?

This pull request:

float stdlib_base_acotf( const float x )

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.

  • Test Coverage report:
Screenshot 2024-04-04 at 09 43 02

  • Since we have used "inverse cotangent" in acot , I have followed the same, instead of using "arccotangent".

  • We do need to increase some tolerance, the maximum of which is 1.9 * EPS. But we get the same tolerance levels for both C and JavaScript versions.

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 Apr 4, 2024
Signed-off-by: GUNJ JOSHI <gunjjoshi8372@gmail.com>
Signed-off-by: GUNJ JOSHI <gunjjoshi8372@gmail.com>
Copy link
Member

@Pranavchiku Pranavchiku left a comment

Choose a reason for hiding this comment

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

Everything works well, this looks good, left a question above, once resolved this will be good to merge.

@Pranavchiku Pranavchiku added Feature Issue or pull request for adding a new feature. Native Addons Issue involves or relates to Node.js native add-ons. Base Issue or pull requests related to "low-level" functionality oriented toward library consumers. C Issue involves or relates to C. Ready To Merge A pull request which is ready to be merged. labels Apr 4, 2024
Signed-off-by: Athan <kgryte@gmail.com>
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.

LGTM. Thanks, @gunjjoshi and @Pranavchiku!

@kgryte
Copy link
Member

kgryte commented Apr 4, 2024

Re: tolerance levels. They are within reason, especially given that we are comparing against double-precision implementation. What matters equally is that C and JS tolerances are the same (i.e., the implementations are observationally equivalent), which they are.

Thank you, as well, for screenshotting the coverage, as a matter of practice.

@kgryte kgryte merged commit 01cbab9 into stdlib-js:develop Apr 4, 2024
8 checks passed
@gunjjoshi
Copy link
Member Author

Understood. Thanks, @kgryte @Pranavchiku !

@gunjjoshi gunjjoshi deleted the acotf branch April 4, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Base Issue or pull requests related to "low-level" functionality oriented toward library consumers. C Issue involves or relates to C. Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality. Native Addons Issue involves or relates to Node.js native add-ons. Ready To Merge A pull request which is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: add math/base/special/acotf
4 participants