wip: test pull request for iterating on GitHub workflows - #623
wip: test pull request for iterating on GitHub workflows#623Planeshifter wants to merge 22 commits into
Conversation
Planeshifter
changed the title
@stdlib/number/float32/base/signbit
|
Based off the Notes
|
Planeshifter
left a comment
Planeshifter
left a comment
There was a problem hiding this comment.
@kgryte Would appreciate a look at the scaffolded files to catch any further issues that need to be resolved with our bot.
| #include "stdlib/math/base/special/signbitf.h" | ||
| #include "stdlib/math/base/napi/unary.h" | ||
|
|
||
| STDLIB_MATH_BASE_NAPI_MODULE_D_B( stdlib_base_signbitf ) |
There was a problem hiding this comment.
@kgryte So STDLIB_MATH_BASE_NAPI_MODULE_D_B doesn't exist in stdlib/math/base/napi/unary.h. What should this file look like?
There was a problem hiding this comment.
There won't be this macro, as it is not common enough. See https://github.com/stdlib-js/stdlib/blob/develop/lib/node_modules/%40stdlib/number/float64/base/signbit/src/addon.c. Unless this pattern happens over and over, not likely to add. As mentioned elsewhere, cannot really predict what should go in addon.c as it will vary from package to package, even if for some types of packages, such as math/base/special/*, we predominantly use macros, this won't be true elsewhere (e.g., in number/base/* or blas/* or strided/*).
In short, in terms of scaffolding, I'd template the addon.c file with a TODO.
|
|
||
| #### stdlib_base_float32_signbit( x ) | ||
|
|
||
| Returns a `boolean` indicating if the sign bit for a [single-precision floating-point number][ieee754] is on (`true`) or off (`false`). |
There was a problem hiding this comment.
There was a problem hiding this comment.
We cannot rely on JS and C APIs being the same in terms of API signatures.
| #include <time.h> | ||
| #include <sys/time.h> | ||
|
|
||
| #define NAME "{{alias}}" |
| double elapsed; | ||
| bool out; | ||
| float x; | ||
| double t; |
| t = tic(); | ||
| for ( i = 0; i < ITERATIONS; i++ ) { | ||
| x = ( rand_double()*1000.0f ) - 500.0f; | ||
| out = x < 0.0f; |
There was a problem hiding this comment.
Not sure what happened here.
Personally, I'd just insert a TODO here, as benchmarks can be fairly bespoke.
| /** | ||
| * Returns a `boolean` indicating if the sign bit for a single-precision floating-point number is on (`true`) or off (`false`). | ||
| */ | ||
| bool stdlib_base_float32_signbit( const float x ); |
There was a problem hiding this comment.
Needs to be updated. And even if returning a bool, L22 would not be correct.
| * // returns true | ||
| */ | ||
| function signbitf( x ) { | ||
| return addon( x ); |
There was a problem hiding this comment.
As the native add-on will be return an int32, need to wrap with the Boolean constructor.
| "libraries": [], | ||
| "libpath": [], | ||
| "dependencies": [ | ||
| "@stdlib/math/base/special/signbitf", |
There was a problem hiding this comment.
This is incorrect and should not be included.
PR Commit MessagePlease review the above commit message and make any necessary adjustments. |
|
/stdlib help |
|
@Planeshifter, available slash commands include:
|
|
/stdlib merge |
1 similar comment
|
/stdlib merge |
|
/stdlib nuke |
|
@Planeshifter, slash command not recognized. Please use |
|
/stdlib merge |
|
/stdlib merge |
|
/stdlib update-copyright-years |
|
@Planeshifter I've updated the PR title to avoid confusion. |
|
/stdlib merge |
Description
This pull request:
@stdlib/number/float32/base/signbitRelated Issues
This pull request:
Questions
No.
Other
Opening this PR to test out and incrementally improve our AI workflow for scaffolding packages, which includes addition of C implementations. I expect to delete and re-run the respective scaffolding workflows multiple times as I continue improving the scaffolding outputs.
Checklist
@stdlib-js/reviewers