feat: add math/base/special/factorialf#10032
feat: add math/base/special/factorialf#10032DivyanshuVortex wants to merge 14 commits intostdlib-js:developfrom
math/base/special/factorialf#10032Conversation
|
/stdlib update-copyright-years |
Coverage Report
The above coverage report was generated for the changes in this PR. |
| return (double)now.tv_sec + (double)now.tv_usec/1.0e6; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Can you update this to use random_uniform instead? You can refer to tanf for this
| */ | ||
| static double benchmark( void ) { | ||
| double elapsed; | ||
| int32_t x; |
There was a problem hiding this comment.
Variables x and y should be float here.
|
|
||
| t = tic(); | ||
| for ( i = 0; i < ITERATIONS; i++ ) { | ||
| x = (int32_t)floor( 33.0 * rand_double() ); |
There was a problem hiding this comment.
should be outside the loop. you can refer to tanf
| #include <time.h> | ||
| #include <sys/time.h> | ||
|
|
||
| #define NAME "factorial" |
There was a problem hiding this comment.
| #define NAME "factorial" | |
| #define NAME "factorialf" |
|
|
||
| // MAIN // | ||
|
|
||
| bench( pkg+'::integers', function benchmark( b ) { |
There was a problem hiding this comment.
we have moved away from string concatenation to string interpolation. The formatting change is described in #8647. Applies here and below.
|
|
||
| var resolve = require( 'path' ).resolve; | ||
| var bench = require( '@stdlib/bench' ); | ||
| var isnanf = require( '@stdlib/math/base/assert/is-nan' ); |
There was a problem hiding this comment.
| var isnanf = require( '@stdlib/math/base/assert/is-nan' ); | |
| var isnanf = require( '@stdlib/math/base/assert/is-nanf' ); |
| continue; | ||
| } | ||
| v = factorialf( x[ i ] ); | ||
| if ( v === float64ToFloat32( expected[ i ] ) ) { |
There was a problem hiding this comment.
Can you refactor this to use @stdlib/number/float32/base/assert/is-almost-same-value instead?
There was a problem hiding this comment.
Same comment applies below and for test.native.js
|
|
||
| <section class="related"> | ||
|
|
||
| * * * |
There was a problem hiding this comment.
You can remove this section as it is auto generated.
|
|
||
| <!-- <related-links> --> | ||
|
|
||
| [@stdlib/math/base/special/factorial]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/math/base/special/factorial |
There was a problem hiding this comment.
related links section is also auto-generated
| } | ||
| return STDLIB_CONSTANT_FLOAT32_PINF; | ||
| } | ||
| return (float)stdlib_base_gamma( (double)x + 1.0 ); |
There was a problem hiding this comment.
I believe you should wait for gammaf to be merged
| var gamma = require( '@stdlib/math/base/special/gamma' ); | ||
| var PINF = require( '@stdlib/constants/float32/pinf' ); | ||
| var FLOAT32_MAX_NTH_FACTORIAL = require( '@stdlib/constants/float32/max-nth-factorial' ); | ||
| var float64ToFloat32 = require( '@stdlib/number/float64/base/to-float32' ); |
There was a problem hiding this comment.
Shorten the variable to f32 instead, other packages follow this pattern.
|
/stdlib merge |
Description
Related Issues
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
{{TODO: add disclosure if applicable}}
@stdlib-js/reviewers