feat: add C implementation for stats/base/dists/signrank/quantile#10059
feat: add C implementation for stats/base/dists/signrank/quantile#10059nirmaljb wants to merge 8 commits intostdlib-js:developfrom
stats/base/dists/signrank/quantile#10059Conversation
Coverage Report
The above coverage report was generated for the changes in this PR. |
|
/stdlib update-copyright-years |
13bc930 to
b3908cf
Compare
|
The test is stuck at test.native.js test with large n values (up to 150) from the fixtures. The recursive weights function has O(2^n) complexity, making it extremely slow. |
stats/base/dists/signrank/quantile
|
Hello Maintainer, just a heads up.
|
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: missing_dependencies
- task: lint_c_examples
status: missing_dependencies
- task: lint_c_benchmarks
status: missing_dependencies
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
…chmark, removed the test to validate for decimal in N in test.native.js
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: missing_dependencies
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: missing_dependencies
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
…age.json
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
65323b3 to
4551870
Compare
…age.json
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
…age.json
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: passed
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
… snippet
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: missing_dependencies
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
Planeshifter
left a comment
There was a problem hiding this comment.
Thanks for the PR! Left round of feedback.
| double pui; | ||
| double q; | ||
| double r; | ||
| if ( stdlib_base_is_nan( n ) || !stdlib_base_is_positive_integer( n ) || !stdlib_base_is_finite( n ) ) { |
There was a problem hiding this comment.
Remove the dead NaN/finite checks on int32_t and match signrank/pdf with just n <= 0. Also drop the unused is_finite and is_positive_integer from includes and manifest.json. We do not need runtime validation when n is appropriately typed already.
| return 0; | ||
| } | ||
| if ( p == 1.0 ) { | ||
| return ( n * ( n + 1 ) ) / 2; |
There was a problem hiding this comment.
n * ( n + 1 ) overflows in int32_t for n >= 46341. Let's cast:
| return ( n * ( n + 1 ) ) / 2; | |
| return ( (double)n * ( (double)n + 1.0 ) ) / 2.0; |
| * | ||
| * @param p input value | ||
| * @param n number of observations | ||
| * @returns evaluated quantile function |
There was a problem hiding this comment.
| * @returns evaluated quantile function | |
| * @return evaluated quantile function |
|
|
||
| double stdlib_base_dists_signrank_quantile( const double p, const int32_t n ) { |
There was a problem hiding this comment.
We can drop the blank line between the closing */ and the function definition.
| double stdlib_base_dists_signrank_quantile( const double p, const int32_t n ) { | |
| double stdlib_base_dists_signrank_quantile( const double p, const int32_t n ) { |
| return 0.0 / 0.0; | ||
| } | ||
| if ( p == 0.0 ) { | ||
| return 0; |
There was a problem hiding this comment.
Use a float literal — function returns double.
| return 0; | |
| return 0.0; |
|
|
||
| // VARIABLES // | ||
|
|
||
| var quantile = tryRequire(resolve(__dirname, './../lib/native.js')); |
There was a problem hiding this comment.
Missing spaces inside the parens.
| var quantile = tryRequire(resolve(__dirname, './../lib/native.js')); | |
| var quantile = tryRequire( resolve( __dirname, './../lib/native.js' ) ); |
|
|
||
| var quantile = tryRequire(resolve(__dirname, './../lib/native.js')); | ||
| var opts = { | ||
| 'skip': (quantile instanceof Error) |
There was a problem hiding this comment.
Missing spaces inside parens.
| 'skip': (quantile instanceof Error) | |
| 'skip': ( quantile instanceof Error ) |
| p = data.p; | ||
| n = data.n; | ||
| for ( i = 0; i < p.length; i++ ) { | ||
| if ( n[ i ] >= 20) { |
There was a problem hiding this comment.
Missing space before closing ).
| if ( n[ i ] >= 20) { | |
| if ( n[ i ] >= 20 ) { |
| p = random_uniform( 0.0, 1.0 ); | ||
| n = (int32_t)stdlib_base_ceil( random_uniform( 1.0, 20.0 ) ); | ||
| y = stdlib_base_dists_signrank_quantile( p, n ); | ||
| printf( "p: %lf, n: %d, f(x;n): %lf\n", p, n, y ); |
There was a problem hiding this comment.
This is a quantile, not a PDF — label should be Q(p;n) (matches the JS examples section).
| printf( "p: %lf, n: %d, f(x;n): %lf\n", p, n, y ); | |
| printf( "p: %lf, n: %d, Q(p;n): %lf\n", p, n, y ); |
| p = random_uniform( 0.0, 1.0 ); | ||
| n = (int32_t)stdlib_base_ceil( random_uniform( 1.0, 20.0 ) ); | ||
| y = stdlib_base_dists_signrank_quantile( p, n ); | ||
| printf( "p: %lf, n: %d, f(x;n): %lf\n", p, n, y ); |
There was a problem hiding this comment.
Same thing here — should be Q(p;n), not f(x;n).
| printf( "p: %lf, n: %d, f(x;n): %lf\n", p, n, y ); | |
| printf( "p: %lf, n: %d, Q(p;n): %lf\n", p, n, y ); |
Resolves #3885 .
Description
This pull request:
Related Issues
This pull request has the following related issues:
@stdlib/stats/base/dists/signrank/quantile#3885Questions
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