Skip to content

feat: add stats/strided/snanmeankbn - #14175

Open
AryanSharma48 wants to merge 10 commits into
stdlib-js:developfrom
AryanSharma48:feat/stats-strided-snanmeankbn
Open

feat: add stats/strided/snanmeankbn#14175
AryanSharma48 wants to merge 10 commits into
stdlib-js:developfrom
AryanSharma48:feat/stats-strided-snanmeankbn

Conversation

@AryanSharma48

@AryanSharma48 AryanSharma48 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Description

What is the purpose of this pull request?

This pull request:

  • Adds @stdlib/stats/strided/snanmeankbn, implementing single-precision NaN-ignoring strided mean calculation using Kahan-Babuška-Neumaier (KBN) compensated summation.
  • Inlines the KBN summation loop in src/main.c and lib/ndarray.js to perform single-pass sum computation while dynamically filtering NaN values and tracking valid count ($n$).
  • Adds edge-case safeguards returning NaN when $n = 0$ (all elements are NaN or $N \le 0$).
  • Implements C public headers, TypeScript declarations, and comprehensive unit tests verifying $1\text{D}$, ndarray, and negative stride behavior.

Related Issues

Does this pull request have any related issues?

This pull request has the following related issues:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of AI assistance?

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance. This helps reviewers determine how much scrutiny to apply when reviewing your contribution. Example disclosures: "This PR was written primarily by Claude Code." or "I consulted ChatGPT to understand the codebase, but the proposed changes were fully authored manually by myself.".

I consulted an AI assistant to verify stdlib C header guard conventions, edge cases, and contribution workflows, but the code implementation, stride logic, and test verifications were reviewed and authored manually by myself.


@stdlib-js/reviewers

Signed-off-by: Aryan Sharma <aryansharma24106@gmail.com>
@stdlib-bot

Copy link
Copy Markdown
Contributor

👋 Hi there! 👋

And thank you for opening your first pull request! We will review it shortly. 🏃 💨

Getting Started

Next Steps

  1. A project maintainer will approve GitHub Actions workflows for your PR.
  2. All CI checks must pass before your submission can be fully reviewed.
  3. You'll need to address any failures in linting or unit tests.

Running Tests Locally

You can use make to run any of the CI commands locally from the root directory of the stdlib repository:

# Run tests for all packages in the math namespace:
make test TESTS_FILTER=".*/@stdlib/math/.*"

# Run benchmarks for a specific package:
make benchmark BENCHMARKS_FILTER=".*/@stdlib/math/base/special/sin/.*"

If you haven't heard back from us within two weeks, please ping us by tagging the "reviewers" team in a comment on this PR.

If you have any further questions while waiting for a response, please join our Zulip community to chat with project maintainers and other community members.

We appreciate your contribution!

Documentation Links

@stdlib-bot stdlib-bot added Statistics Issue or pull request related to statistical functionality. First-time Contributor A pull request from a contributor who has never previously committed to the project repository. labels Aug 11, 2026
@AryanSharma48
AryanSharma48 marked this pull request as ready for review August 11, 2026 20:14
@AryanSharma48
AryanSharma48 requested a review from a team August 11, 2026 20:14
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Aug 11, 2026
Signed-off-by: Aryan Sharma <aryansharma24106@gmail.com>
This fixes an issue where the native test cases were asserting invalid return values instead of expecting NaNs to be properly ignored, aligning them with the pure JS tests so all tests pass in the C-build.

Signed-off-by: Aryan Sharma <aryansharma24106@gmail.com>
@stdlib-bot stdlib-bot removed the First-time Contributor A pull request from a contributor who has never previously committed to the project repository. label Aug 12, 2026

@0PrashantYadav0 0PrashantYadav0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make all the changes I have suggested.

*/
interface Routine {
/**
* Computes the arithmetic mean of a single-precision floating-point strided array using an improved Kahan–Babuška algorithm.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Computes the arithmetic mean of a single-precision floating-point strided array using an improved Kahan–Babuška algorithm.
* Computes the arithmetic mean of a single-precision floating-point strided array, ignoring `NaN` values and using an improved Kahan–Babuška algorithm.

* @example
* var Float32Array = require( '@stdlib/array/float32' );
*
* var x = new Float32Array( [ 1.0, -2.0, 2.0 ] );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* var x = new Float32Array( [ 1.0, -2.0, 2.0 ] );
* var x = new Float32Array( [ 1.0, -2.0, NaN, 2.0 ] );

( N: number, x: Float32Array, strideX: number ): number;

/**
* Computes the arithmetic mean of a single-precision floating-point strided array using an improved Kahan–Babuška algorithm and alternative indexing semantics.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Computes the arithmetic mean of a single-precision floating-point strided array using an improved Kahan–Babuška algorithm and alternative indexing semantics.
* Computes the arithmetic mean of a single-precision floating-point strided array, ignoring `NaN` values and using an improved Kahan–Babuška algorithm and alternative indexing semantics.

* @example
* var Float32Array = require( '@stdlib/array/float32' );
*
* var x = new Float32Array( [ 1.0, -2.0, 2.0 ] );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* var x = new Float32Array( [ 1.0, -2.0, 2.0 ] );
* var x = new Float32Array( [ 1.0, -2.0, NaN, 2.0 ] );

}

/**
* Computes the arithmetic mean of a single-precision floating-point strided array using an improved Kahan–Babuška algorithm.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Computes the arithmetic mean of a single-precision floating-point strided array using an improved Kahan–Babuška algorithm.
* Computes the arithmetic mean of a single-precision floating-point strided array, ignoring `NaN` values and using an improved Kahan–Babuška algorithm.

Comment thread lib/node_modules/@stdlib/stats/strided/snanmeankbn/README.md Outdated
## See Also

- <span class="package-name">[`@stdlib/stats/strided/dmeankbn`][@stdlib/stats/strided/dmeankbn]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a double-precision floating-point strided array using an improved Kahan–Babuška algorithm.</span>
- <span class="package-name">[`@stdlib/stats/strided/meankbn`][@stdlib/stats/strided/meankbn]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a strided array using an improved Kahan–Babuška algorithm.</span>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- <span class="package-name">[`@stdlib/stats/strided/meankbn`][@stdlib/stats/strided/meankbn]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a strided array using an improved Kahan–Babuška algorithm.</span>
- <span class="package-name">[`@stdlib/stats/strided/snanmean`][@stdlib/stats/strided/snanmean]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a single-precision floating-point strided array, ignoring NaN values.</span>


- <span class="package-name">[`@stdlib/stats/strided/dmeankbn`][@stdlib/stats/strided/dmeankbn]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a double-precision floating-point strided array using an improved Kahan–Babuška algorithm.</span>
- <span class="package-name">[`@stdlib/stats/strided/meankbn`][@stdlib/stats/strided/meankbn]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a strided array using an improved Kahan–Babuška algorithm.</span>
- <span class="package-name">[`@stdlib/stats/strided/smean`][@stdlib/stats/strided/smean]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a single-precision floating-point strided array.</span>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- <span class="package-name">[`@stdlib/stats/strided/smean`][@stdlib/stats/strided/smean]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a single-precision floating-point strided array.</span>
- <span class="package-name">[`@stdlib/stats/strided/snanmeanors`][@stdlib/stats/strided/snanmeanors]</span><span class="delimiter">: </span><span class="description">calculate the arithmetic mean of a single-precision floating-point strided array, ignoring NaN values and using ordinary recursive summation.</span>


[@stdlib/stats/strided/dmeankbn]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/dmeankbn

[@stdlib/stats/strided/meankbn]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/meankbn

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[@stdlib/stats/strided/meankbn]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/meankbn
[@stdlib/stats/strided/smeankbn]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/smeankbn


[@stdlib/stats/strided/meankbn]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/meankbn

[@stdlib/stats/strided/smean]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/smean

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[@stdlib/stats/strided/smean]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/smean
[@stdlib/stats/strided/snanmean]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/snanmean
[@stdlib/stats/strided/snanmeanors]: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/stats/strided/snanmeanors

@0PrashantYadav0 0PrashantYadav0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make all the changes I have suggested.

@0PrashantYadav0 0PrashantYadav0 added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Aug 12, 2026
@AryanSharma48
AryanSharma48 force-pushed the feat/stats-strided-snanmeankbn branch from 1e3a746 to 34da390 Compare August 12, 2026 14:10
@AryanSharma48

AryanSharma48 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review @0PrashantYadav0 ! I've addressed all the review feedback across the files, updated the style formatting, and resolved the CI check failures. Local tests and linters are passing cleanly now. It's ready for another look when you have a moment!

@stdlib-bot

stdlib-bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
stats/strided/snanmeankbn $\\color{green}398/398$
$\\color{green}+0.00\\%$
$\\color{green}25/25$
$\\color{green}+0.00\\%$
$\\color{green}4/4$
$\\color{green}+0.00\\%$
$\\color{green}398/398$
$\\color{green}+0.00\\%$

The above coverage report was generated for the changes in this PR.

- Update C function signatures and parameter naming according to project conventions
- Align header includes and standard type definitions
- Fix linting errors and formatting inconsistencies across strided math implementation
- Resolve build and test runner CI failures

Signed-off-by: Aryan Sharma <aryansharma24106@gmail.com>
@AryanSharma48
AryanSharma48 force-pushed the feat/stats-strided-snanmeankbn branch from 34da390 to 421cb9b Compare August 12, 2026 14:26
@0PrashantYadav0

Copy link
Copy Markdown
Member

Hey! @AryanSharma48 Please take another look at the review—it seems like a couple of the suggested changes slipped through. (I find it helpful to resolve the comments one by one as I apply them so I don't lose track).

Also, let's avoid force-pushing. Before you start working on the remaining changes, make sure to pull/fetch the latest updates from your branch. Let me know if you run into any issues!

Remove the double-precision dmeankbn package from the See Also section, leaving only single-precision package references as requested in PR review.

Signed-off-by: Aryan Sharma <aryansharma24106@gmail.com>
@AryanSharma48

Copy link
Copy Markdown
Contributor Author

Thanks for pointing that out @0PrashantYadav0!

Regarding the force-push: I ran into a branch sync issue after missing a local pull, so I force-pushed to align with develop, apologies for the extra noise, I'll stick to standard incremental commits going forward!

As for the reviews, a few of the suggestions were overlapping with changes already applied, but I went back through all open threads one-by-one, addressed the remaining ones, and resolved them on GitHub.

I’ve pushed a fresh commit with those final fixes and local tests are passing cleanly. Ready for your review!

Signed-off-by: Aryan Sharma <aryansharma24106@gmail.com>

@0PrashantYadav0 0PrashantYadav0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the changes required little more discussion.

Comment thread lib/node_modules/@stdlib/stats/strided/snanmeankbn/benchmark/c/benchmark.length.c Outdated
Comment thread lib/node_modules/@stdlib/stats/strided/snanmeankbn/benchmark/benchmark.js Outdated
Comment thread lib/node_modules/@stdlib/stats/strided/snanmeankbn/benchmark/benchmark.js Outdated
Comment thread lib/node_modules/@stdlib/stats/strided/snanmeankbn/benchmark/c/benchmark.length.c Outdated
if ( n === 0 ) {
return NaN;
}
return float64ToFloat32( (sum + c) / n );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating the complete logic from scratch we can use already existing modules. For example here we can use @stdlib/blas/ext/base/snansumkbn.

You can replace this file with:

'use strict';

// MODULES //

var float64ToFloat32 = require( '@stdlib/number/float64/base/to-float32' );
var snansumkbn = require( '@stdlib/blas/ext/base/snansumkbn' ).ndarray;


// MAIN //

/**
* Computes the arithmetic mean of a single-precision floating-point strided array, ignoring `NaN` values and using an improved Kahan–Babuška algorithm.
*
* ## Method
*
* -   This implementation uses an "improved Kahan–Babuška algorithm", as described by Neumaier (1974).
*
* ## References
*
* -   Neumaier, Arnold. 1974. "Rounding Error Analysis of Some Methods for Summing Finite Sums." _Zeitschrift Für Angewandte Mathematik Und Mechanik_ 54 (1): 39–51. doi:[10.1002/zamm.19740540106](https://doi.org/10.1002/zamm.19740540106).
*
* @param {PositiveInteger} N - number of indexed elements
* @param {Float32Array} x - input array
* @param {integer} strideX - stride length
* @param {NonNegativeInteger} offsetX - starting index
* @returns {number} arithmetic mean
*
* @example
* var Float32Array = require( '@stdlib/array/float32' );
*
* var x = new Float32Array( [ 2.0, 1.0, 2.0, -2.0, -2.0, 2.0, 3.0, 4.0, NaN, NaN ] );
*
* var v = snanmeankbn( 5, x, 2, 1 );
* // returns 1.25
*/
function snanmeankbn( N, x, strideX, offsetX ) {
	var ix;
	var v;
	var n;
	var i;

	if ( N <= 0 ) {
		return NaN;
	}
	if ( N === 1 || strideX === 0 ) {
		return x[ offsetX ];
	}
	ix = offsetX;
	n = 0;
	for ( i = 0; i < N; i++ ) {
		v = x[ ix ];
		if ( v === v ) {
			n += 1;
		}
		ix += strideX;
	}
	if ( n === 0 ) {
		return NaN;
	}
	return float64ToFloat32( snansumkbn( N, x, strideX, offsetX ) / n );
}


// EXPORTS //

module.exports = snanmeankbn;

Still we have to discuss this further.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0PrashantYadav0 Good point on reusing @stdlib/blas/ext/base/snansumkbn.
Delegating to snansumkbn keeps the codebase DRY, but the main trade-off seems to be single-pass vs. two-pass traversal:

Current inline logic: Does everything in a single pass over x (accumulates Kahan–Babuška sum and counts n simultaneously).

Delegating to snansumkbn: Requires two passes over x (one loop to count non-NaN elements, and a second pass inside snansumkbn to sum).
For large strided arrays, the single-pass inline implementation might be noticeably faster due to better cache locality.

Should we benchmark both approaches to see if the two-pass overhead is significant, or do you prefer prioritizing DRYness here?

@0PrashantYadav0 0PrashantYadav0 Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I prefers reusability of code as this is very important for large codebases like us. But again the logic itself is not wrong. For now you should go ahead and do some benchmark for both approaches and based on those result we will discuss this in next office hours. Also you should create a thread in zulip and share all the finding there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0PrashantYadav0 Makes total sense! I've opened the topic Benchmark: single-pass vs two-pass in snanmean* under #dev on Zulip and tagged everyone.

I'm running the benchmarks locally for both single-pass and two-pass variations across different array sizes and will post the results table over in that Zulip thread shortly!

Comment thread lib/node_modules/@stdlib/stats/strided/snanmeankbn/src/main.c
Comment thread lib/node_modules/@stdlib/stats/strided/snanmeankbn/src/main.c Outdated
"libpath": [],
"dependencies": [
"@stdlib/blas/base/shared",
"@stdlib/strided/base/stride2offset"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we move forward with my approach we make to changes the dependencies here as well.

Suggested change
"@stdlib/strided/base/stride2offset"
"@stdlib/strided/base/stride2offset",
"@stdlib/blas/ext/base/snansumkbn"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have to make changes in all the dependencies.

Signed-off-by: Aryan Sharma <aryansharma24106@gmail.com>
@MeKaustubh07 MeKaustubh07 changed the title feat: add stats/strided/snanmeankbn feat: add stats/strided/snanmeankbn Aug 13, 2026
@AryanSharma48

Copy link
Copy Markdown
Contributor Author

@0PrashantYadav0 Hey Prashant! Whenever you get a chance, could you take a look at the latest updates?

Benchmarks: Updated all benchmark files to conform with standard stdlib base generator patterns and verified that local tests and linters pass cleanly.

filledarrayBy / rand() question: Left a quick comment on the benchmark thread regarding the filledarrayBy and rand() usage.

ndarray.js trade-off: Left my thoughts on the ndarray.js thread comparing the single-pass inline loop (better cache locality) vs. delegating to snansumkbn (two-pass, but DRY).

Let me know your thoughts whenever you're free!

Comment thread lib/node_modules/@stdlib/stats/strided/snanmeankbn/benchmark/benchmark.js Outdated
@0PrashantYadav0

0PrashantYadav0 commented Aug 14, 2026

Copy link
Copy Markdown
Member

@AryanSharma48 Remove the Resolves # 13955 line from PR description coz the taged issues is just for tracking the modules status. Instead add this as reference. If you add that as resolves the merging of this PR will close the tagged issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Changes Pull request which needs changes before being merged. Statistics Issue or pull request related to statistical functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants