Skip to content

feat: add ndarray/base/where#10659

Open
LoayAhmed304 wants to merge 34 commits intostdlib-js:developfrom
LoayAhmed304:feat/base-ndarray-where
Open

feat: add ndarray/base/where#10659
LoayAhmed304 wants to merge 34 commits intostdlib-js:developfrom
LoayAhmed304:feat/base-ndarray-where

Conversation

@LoayAhmed304
Copy link
Copy Markdown
Contributor

Resolves #{{TODO: add issue number}}.

Description

What is the purpose of this pull request?

This pull request:

  • Implements the where function for ndarrays, which applies a condition to elements in two input ndarrays and assign results to elements in an output ndarray.

Related Issues

Does this pull request have any related issues?

This pull request has the following related issues:

  • #{{TODO: add related issue number}}

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.".

{{TODO: add disclosure if applicable}}


@stdlib-js/reviewers

@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Mar 2, 2026
@stdlib-bot
Copy link
Copy Markdown
Contributor

stdlib-bot commented Mar 2, 2026

Coverage Report

Package Statements Branches Functions Lines
ndarray/base/where $\color{red}7704/14877$
$\color{green}+51.78%$
$\color{red}188/196$
$\color{green}+95.92%$
$\color{red}16/46$
$\color{green}+34.78%$
$\color{red}7704/14877$
$\color{green}+51.78%$

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

@LoayAhmed304 LoayAhmed304 marked this pull request as draft March 2, 2026 21:49
@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Mar 2, 2026
@kgryte kgryte changed the title feat: create ndarray/base/where feat: add ndarray/base/where Mar 2, 2026
@kgryte kgryte added Feature Issue or pull request for adding a new feature. difficulty: 4 Likely to be moderately difficult. labels Mar 2, 2026
@LoayAhmed304 LoayAhmed304 force-pushed the feat/base-ndarray-where branch 2 times, most recently from 44fccf8 to 8fa22ff Compare March 4, 2026 22:19
@LoayAhmed304 LoayAhmed304 force-pushed the feat/base-ndarray-where branch from 8fa22ff to 2df0faf Compare March 4, 2026 22:23
@LoayAhmed304 LoayAhmed304 force-pushed the feat/base-ndarray-where branch from 7eaac76 to 5b71c61 Compare March 4, 2026 22:47
@LoayAhmed304 LoayAhmed304 force-pushed the feat/base-ndarray-where branch 2 times, most recently from cc80ed0 to c2816cb Compare March 4, 2026 23:28
@LoayAhmed304 LoayAhmed304 force-pushed the feat/base-ndarray-where branch from c2816cb to b672f66 Compare March 4, 2026 23:54
@LoayAhmed304 LoayAhmed304 force-pushed the feat/base-ndarray-where branch from 5e38042 to f470119 Compare March 5, 2026 14:29
Comment on lines +366 to +374
// Verify that the input and output arrays have the same number of dimensions...
shc = c.shape;
shx = x.shape;
shy = y.shape;
sho = o.shape;
ndims = shc.length;
if ( ndims !== shx.length || ndims !== shy.length || ndims !== sho.length ) {
throw new Error( format( 'invalid arguments. Arrays must have the same number of dimensions (i.e., same rank). ndims(c) == %d. ndims(x) == %d. ndims(y) == %d. ndims(o) == %d.', shc.length, shx.length, shy.length, sho.length ) );
}
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.

@LoayAhmed304 If someone passes arrays with mismatched shapes but valid dtypes, the function will do unnecessary work (reinterpreting arrays) before eventually throwing.

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.

Done. Moved shapes validation to the top.

I believe I took it from assign here, but we're dealing with 4 arrays here not 2, so maybe the unnecessary work will affect performance here.

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.

Note that this will cause re-assignment of shapes again after the reinterpretation, since reinterpreting complex ndarrays mutates the shape.

Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/main.js
@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Mar 24, 2026
@github-actions github-actions bot mentioned this pull request Mar 24, 2026
@LoayAhmed304 LoayAhmed304 force-pushed the feat/base-ndarray-where branch from 8daca0d to 0c3a27c Compare March 24, 2026 23:47
Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/main.js
Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/main.js Outdated
Co-authored-by: Muhammad Haris <101793258+headlessNode@users.noreply.github.com>
Signed-off-by: Loay Ahmed <loayahmed304@gmail.com>
Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/main.js
@LoayAhmed304
Copy link
Copy Markdown
Contributor Author

Note that tests will have updated cases to cover the varying dtypes of x and y, since the test cases were made while assuming they would have the exact same dtype.

Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/1d_accessors.js Outdated
Comment on lines +151 to +154
var dc0;
var dx0;
var dy0;
var do0;
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 dc0;
var dx0;
var dy0;
var do0;
var dc;
var dx;
var dy;
var do;

Applies to below and 1d.js as well.

Copy link
Copy Markdown
Contributor Author

@LoayAhmed304 LoayAhmed304 Mar 25, 2026

Choose a reason for hiding this comment

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

do is a reserved keyword. I'll use dout instead, but will keep it do# for the rest of the kernels.

Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/nd.js Outdated
Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/2d_accessors.js Outdated
Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/0d_accessors.js
Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/2d.js Outdated
Comment thread lib/node_modules/@stdlib/ndarray/base/where/lib/1d.js Outdated

The function accepts the following arguments:

- **arrays**: array-like object containing three input ndarrays and one output ndarray.
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.

Not sure if this is okay since in the main description 'a condition' and 'two input ndarrays' is used.

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 you decide to change, it would apply to repl and index.d.ts file as well.

Comment thread lib/node_modules/@stdlib/ndarray/base/where/README.md Outdated
Comment thread lib/node_modules/@stdlib/ndarray/base/where/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/ndarray/base/where/test/test.js Outdated
Comment thread lib/node_modules/@stdlib/ndarray/base/where/test/test.2d.js Outdated
Comment on lines +106 to +111
// broadcast extra dimension along last axis for the condition array for complex compatibility.
stride = shape2strides( shape, order );
cshape = shape.slice();
cshape.push( 1 );
cstrides = stride.slice();
cstrides.push( 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.

Why is this required here? condition becomes 3d while all other are 2d. What happens if the condition is 2d like others? Is the output affected?

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.

This is what main.js does when dealing with complex input ndarrays, and actually this is something I wanted to discuss.

// main.js

if ( isComplexArray( x.data ) && isComplexArray( y.data ) && isComplexArray( o.data ) ) {
		x = complex2real( x );
		y = complex2real( y );
		o = complex2real( o );

		c.shape.push( 2 ); // real and imaginary components
		c.strides.push( 0 ); // broadcast
// ....

Currently what I'm doing is something similar to what complex2real does when interpreting the input complex ndarray, which manipulates the shape and stride after reinterpretation. So this basically does the same to the condition array, so each element in the condition ndarray compares both the real and imaginary parts. Hence for complex, the condition ndarray buffer can be half the size of the complex buffer. Like this:

// test.js

var cbuf = new BooleanArray([
		true,
		false,
		false,
		true
	]);
var xbuf = new Complex128Array([
		10.0,
		10.0,
		10.0,
		10.0,
		10.0,
		10.0,
		10.0,
		10.0
	]);
// ....

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.

However I believe this will break when having 1 complex input ndarray and 1 real input ndarray, since I was assuming both will have the same dtype.

@headlessNode
Copy link
Copy Markdown
Member

@LoayAhmed304 Would good to check all the files against the above comments, especially the dimensional kernels.

@LoayAhmed304 LoayAhmed304 force-pushed the feat/base-ndarray-where branch from f06d2db to fb2a747 Compare March 25, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

difficulty: 4 Likely to be moderately difficult. Feature Issue or pull request for adding a new feature. Needs Changes Pull request which needs changes before being merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants