-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: add stats/base/ndarray/snanstdev
#9656
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
base: develop
Are you sure you want to change the base?
feat: add stats/base/ndarray/snanstdev
#9656
Conversation
---
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: passed
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- 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: passed
- task: lint_license_headers
status: passed
---
|
/stdlib update-copyright-years |
---
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: 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
---
---
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: 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
---
---
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: 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
---
Coverage Report
The above coverage report was generated for the changes in this PR. |
stats/base/ndarray/snanstdev
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.
This PR currently has a critical bug.
| */ | ||
| function snanstdev( arrays ) { | ||
| var x = arrays[ 0 ]; | ||
| return strided(numelDimension( x, 0 ), getData( x ), getStride( x, 0 ), getOffset( x )); // eslint-disable-line max-len |
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.
The call to strided() (which is snanstdevpn.ndarray) is missing the required correction parameter. Looking at the underlying function signature:
snanstdevpn( N, correction, x, stride, offset )The current call passes:
N=numelDimension(x, 0)✓correction=getData(x)✗ (the data buffer is being passed as correction!)x=getStride(x, 0)✗ (stride is being passed as the array)stride=getOffset(x)✗ (offset is being passed as stride)offset= missing ✗
This completely breaks the function. Looking at similar packages like sstdevpn and sstdev, the function should:
- Accept a second ndarray in
arraysfor the correction value - Extract it using
ndarraylike2scalar - Pass all 5 arguments to
strided()
See @stdlib/stats/base/ndarray/sstdevpn/lib/main.js for the correct pattern.
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing an input ndarray | ||
| * @returns {number} standard deviation | ||
| * | ||
| * @example | ||
| * var Float32Array = require( '@stdlib/array/float32' ); | ||
| * var ndarray = require( '@stdlib/ndarray/base/ctor' ); | ||
| * | ||
| * var xbuf = new Float32Array( [ 1.0, 3.0, NaN, 2.0 ] ); | ||
| * var x = new ndarray( 'float32', xbuf, [ 4 ], [ 1 ], 0, 'row-major' ); | ||
| * | ||
| * var v = snanstdev( [ x ] ); | ||
| * // returns NaN |
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.
The JSDoc says this accepts only one ndarray, but standard deviation packages in stdlib require a second zero-dimensional ndarray for the degrees of freedom adjustment (correction). The function signature should match packages like sstdev and sstdevpn:
* @param {ArrayLikeObject<Object>} arrays - array-like object containing a one-dimensional input ndarray and a zero-dimensional ndarray specifying a degrees of freedom adjustmentAlso, the // returns NaN comment is incorrect - for input [1.0, 3.0, 2.0] (ignoring the NaN), the standard deviation should be approximately 1.0, not NaN.
| > var ord = 'row-major'; | ||
| > var x = new {{alias:@stdlib/ndarray/ctor}}( dt, xbuf, sh, sx, ox, ord ); | ||
| > {{alias}}( [ x ] ) | ||
| NaN |
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.
The example output NaN is incorrect. For input [ 1.0, -2.0, 2.0 ] (none of which are NaN), the standard deviation should be a positive number, not NaN.
Once the correction parameter is added to the API, this example should also include the correction ndarray and show the correct numeric output.
| tape( 'the function supports ndarrays with multiple non-NaN values', function test( t ) { | ||
| var x = new Float32Array( [ 1.0, 2.0, NaN, 3.0 ] ); | ||
| var v = snanstdev( [ vector( x, 4, 1, 0 ) ] ); | ||
|
|
||
| t.ok( isnanf( v ) || v >= 0.0, 'returns NaN or non-negative number' ); |
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.
These assertions are too weak - they pass even when the function returns NaN for all inputs (which is what's happening due to the missing correction parameter bug).
Tests should verify the actual computed standard deviation value. For example, for input [1.0, 2.0, 3.0] (ignoring NaN), with correction=1, the expected standard deviation is 1.0.
Consider adding tests like:
tape( 'the function computes the standard deviation', function test( t ) {
var expected = 1.0; // stdev of [1, 2, 3] with correction=1
var x = new Float32Array( [ 1.0, 2.0, NaN, 3.0 ] );
var v = snanstdev( [ vector( x, 4, 1, 0 ), correction ] );
t.strictEqual( v, expected, 'returns expected value' );
t.end();
});| function benchmark( b ) { | ||
| var v; | ||
| var i; | ||
|
|
||
| b.tic(); | ||
| for ( i = 0; i < b.iterations; i++ ) { | ||
| v = variancech( [ x, correction ] ); | ||
| if ( isnan( v ) ) { | ||
| b.fail( 'should not return NaN' ); | ||
| } | ||
| snanstdev( [ x ] ); | ||
| } | ||
| b.toc(); | ||
| if ( isnan( v ) ) { | ||
| b.fail( 'should not return NaN' ); | ||
| } | ||
|
|
||
| b.pass( 'benchmark finished' ); | ||
| b.end(); | ||
| } |
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.
The benchmark doesn't capture or verify the return value. Looking at reference packages like snanmean, the benchmark should:
- Capture the return value:
v = snanstdev( [ x ] ); - Import
isnanfand add sanity checks inside and after the loop
| function benchmark( b ) { | |
| var v; | |
| var i; | |
| b.tic(); | |
| for ( i = 0; i < b.iterations; i++ ) { | |
| v = variancech( [ x, correction ] ); | |
| if ( isnan( v ) ) { | |
| b.fail( 'should not return NaN' ); | |
| } | |
| snanstdev( [ x ] ); | |
| } | |
| b.toc(); | |
| if ( isnan( v ) ) { | |
| b.fail( 'should not return NaN' ); | |
| } | |
| b.pass( 'benchmark finished' ); | |
| b.end(); | |
| } | |
| function benchmark( b ) { | |
| var v; | |
| var i; | |
| b.tic(); | |
| for ( i = 0; i < b.iterations; i++ ) { | |
| v = snanstdev( [ x ] ); | |
| if ( isnanf( v ) ) { | |
| b.fail( 'should not return NaN' ); | |
| } | |
| } | |
| b.toc(); | |
| if ( isnanf( v ) ) { | |
| b.fail( 'should not return NaN' ); | |
| } | |
| b.pass( 'benchmark finished' ); | |
| b.end(); | |
| } |
Note: You'll also need to add var isnanf = require( '@stdlib/math/base/assert/is-nanf' ); to the imports.
| function vector( buffer, length, stride, offset ) { | ||
| return new ndarray( 'float32', buffer, [ length ], [ stride ], offset, 'row-major' ); | ||
| } |
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.
This helper function needs JSDoc documentation to match stdlib conventions. See reference packages like snanmean/test/test.js:
| function vector( buffer, length, stride, offset ) { | |
| return new ndarray( 'float32', buffer, [ length ], [ stride ], offset, 'row-major' ); | |
| } | |
| /** | |
| * Returns a one-dimensional ndarray. | |
| * | |
| * @private | |
| * @param {Collection} buffer - underlying data buffer | |
| * @param {NonNegativeInteger} length - number of indexed elements | |
| * @param {integer} stride - stride length | |
| * @param {NonNegativeInteger} offset - index offset | |
| * @returns {ndarray} one-dimensional ndarray | |
| */ | |
| function vector( buffer, length, stride, offset ) { | |
| return new ndarray( 'float32', buffer, [ length ], [ stride ], offset, 'row-major' ); | |
| } |
| * // returns <number> | ||
| */ | ||
| declare function prependSingletonDimensions<T = unknown, U extends typedndarray<T> = typedndarray<T>>( x: U, n: number ): U; | ||
| declare function snanstdev( arrays: [ float32ndarray ] ): number; |
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.
The type signature needs to accept two ndarrays to match the expected API pattern for stdev functions. Looking at sstdev and sstdevpn, the signature should be:
declare function snanstdev( arrays: [ float32ndarray, ndarray ] ): number;The second element is a zero-dimensional ndarray containing the degrees of freedom adjustment (correction value).
|
Understood. Updated snanstdev to accept a second zero-dimensional ndarray specifying the degrees of freedom adjustment (correction), and now pass it correctly to snanstdevpn.ndarray. Updated JSDoc, TypeScript definitions, REPL docs, tests, and benchmarks to reflect the corrected API and verify the actual computed standard deviation values. |
---
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: na
- task: lint_repl_help
status: passed
- task: lint_javascript_src
status: passed
- 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: passed
- 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: 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: 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
---
---
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: 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
---
---
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: 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
---
---
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: 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
---
---
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: passed
- 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
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Description
This pull request:
Related Issues
This pull request has the following related issues:
Questions
No.
Other
No.
Checklist
AI Assistance
@stdlib-js/reviewers