Skip to content

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
@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