-
-
Notifications
You must be signed in to change notification settings - Fork 845
feat: add C implementation for math/base/special/gcd
#1703
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
Also, if we use double-precision floating-point numbers as input and the same as output we will have issues in syncing the javascript implementation with the C implementation as the right shift, and left shift operators used won't work for double-precision floating-point numbers. |
@stdlib/math/base/special/gcd
math/base/special/gcd
math/base/special/gcd
math/base/special/gcd
@aman-095 After looking at this again, I think we should do doubles in and doubles out in order to support a larger range of values, as done in JS. Re: bit shift. I suggest just casting the double to an integer of the appropriate type. Once cast, a bitwise implementation should be readily doable in C. |
@kgryte the test.js file contains a case with TWO_100. If we use typecast then the maximum we can do is typecast double to int64_t. But TWO_100 cannot be typecasted as it is out of the scope of int64_t. |
@aman-095 Yeah, that's my point. We should update the implementation to work on doubles, rather than |
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
lib/node_modules/@stdlib/math/base/special/gcd/include/stdlib/math/base/special/gcd.h
Outdated
Show resolved
Hide resolved
Signed-off-by: Athan <kgryte@gmail.com>
lib/node_modules/@stdlib/math/base/special/gcd/examples/c/example.c
Outdated
Show resolved
Hide resolved
lib/node_modules/@stdlib/math/base/special/gcd/benchmark/c/native/benchmark.c
Outdated
Show resolved
Hide resolved
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.
Thanks for the updates, @aman-095. Left another round of comments. Some small changes and I think we're getting there.
Signed-off-by: Athan <kgryte@gmail.com>
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.
LGTM. Thanks, @aman-095!
Resolves #1701.
Description
This pull request adds native C implementation for
@stdlib/math/base/special/gcd
.Related Issues
This pull request:
math/base/special/gcd
#1701Questions
The above implementation for gcd uses 64-bit signed integers as input and returns double-precision floating-point number as output. I have changed the test.native.js for the last case of test.js where we cannot have value >= 2^63 - 1, and removed the cases of infinity, NaN, as they won't work and hence are removed. Also, the case in test.js where when passed any non-integer should give NaN as output is removed as here when passed non-integer it would type cast it to int and answer then will not be NaN.
Other
No.
Checklist
@stdlib-js/reviewers