Repository navigation
Conversation
Coverage Report
The above coverage report was generated for the changes in this PR. |
There was a problem hiding this comment.
Thanks for working on this! The approach for passing the boolean condition to C looks good. I've left a few line-wise suggestions to match the conventions used in the other blas/ext/base/ndarray add-ons (e.g., dxpy). Mind making these changes?
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition and assigns them to an output ndarray. | ||
| * | ||
| * @param arrays list containing a condition, two input ndarrays, and an output ndarray |
There was a problem hiding this comment.
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition and assigns them to an output ndarray. | |
| * | |
| * @param arrays list containing a condition, two input ndarrays, and an output ndarray | |
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition. | |
| * | |
| * ## Notes | |
| * | |
| * - The function expects the following ndarrays: | |
| * | |
| * - a one-dimensional condition ndarray. | |
| * - first one-dimensional input ndarray. | |
| * - second one-dimensional input ndarray. | |
| * - a one-dimensional output ndarray. | |
| * | |
| * @param arrays list containing ndarrays |
The description should match lib/main.js verbatim, and the Notes list matches other add-ons (e.g., blas/ext/base/ndarray/dxpy).
| const struct ndarray *condition = arrays[ 0 ]; | ||
| const struct ndarray *x = arrays[ 1 ]; | ||
| const struct ndarray *y = arrays[ 2 ]; | ||
| const struct ndarray *out = arrays[ 3 ]; | ||
|
|
||
| API_SUFFIX(stdlib_strided_swhere_ndarray)( | ||
| stdlib_ndarray_dimension( condition, 0 ), | ||
| (const bool *)stdlib_ndarray_data( condition ), | ||
| stdlib_ndarray_stride_elements( condition, 0 ), | ||
| stdlib_ndarray_offset_elements( condition ), | ||
| (const float *)stdlib_ndarray_data( x ), | ||
| stdlib_ndarray_stride_elements( x, 0 ), | ||
| stdlib_ndarray_offset_elements( x ), | ||
| (const float *)stdlib_ndarray_data( y ), | ||
| stdlib_ndarray_stride_elements( y, 0 ), | ||
| stdlib_ndarray_offset_elements( y ), | ||
| (float *)stdlib_ndarray_data( out ), | ||
| stdlib_ndarray_stride_elements( out, 0 ), | ||
| stdlib_ndarray_offset_elements( out ) | ||
| ); |
There was a problem hiding this comment.
| const struct ndarray *condition = arrays[ 0 ]; | |
| const struct ndarray *x = arrays[ 1 ]; | |
| const struct ndarray *y = arrays[ 2 ]; | |
| const struct ndarray *out = arrays[ 3 ]; | |
| API_SUFFIX(stdlib_strided_swhere_ndarray)( | |
| stdlib_ndarray_dimension( condition, 0 ), | |
| (const bool *)stdlib_ndarray_data( condition ), | |
| stdlib_ndarray_stride_elements( condition, 0 ), | |
| stdlib_ndarray_offset_elements( condition ), | |
| (const float *)stdlib_ndarray_data( x ), | |
| stdlib_ndarray_stride_elements( x, 0 ), | |
| stdlib_ndarray_offset_elements( x ), | |
| (const float *)stdlib_ndarray_data( y ), | |
| stdlib_ndarray_stride_elements( y, 0 ), | |
| stdlib_ndarray_offset_elements( y ), | |
| (float *)stdlib_ndarray_data( out ), | |
| stdlib_ndarray_stride_elements( out, 0 ), | |
| stdlib_ndarray_offset_elements( out ) | |
| ); | |
| const struct ndarray *condition = arrays[ 0 ]; | |
| const struct ndarray *x = arrays[ 1 ]; | |
| const struct ndarray *y = arrays[ 2 ]; | |
| const struct ndarray *out = arrays[ 3 ]; | |
| const CBLAS_INT N = stdlib_ndarray_dimension( condition, 0 ); | |
| const CBLAS_INT strideC = stdlib_ndarray_stride_elements( condition, 0 ); | |
| const CBLAS_INT offsetC = stdlib_ndarray_offset_elements( condition ); | |
| const CBLAS_INT strideX = stdlib_ndarray_stride_elements( x, 0 ); | |
| const CBLAS_INT offsetX = stdlib_ndarray_offset_elements( x ); | |
| const CBLAS_INT strideY = stdlib_ndarray_stride_elements( y, 0 ); | |
| const CBLAS_INT offsetY = stdlib_ndarray_offset_elements( y ); | |
| const CBLAS_INT strideOut = stdlib_ndarray_stride_elements( out, 0 ); | |
| const CBLAS_INT offsetOut = stdlib_ndarray_offset_elements( out ); | |
| const bool *dataC = (const bool *)stdlib_ndarray_data( condition ); | |
| const float *dataX = (const float *)stdlib_ndarray_data( x ); | |
| const float *dataY = (const float *)stdlib_ndarray_data( y ); | |
| float *dataOut = (float *)stdlib_ndarray_data( out ); | |
| API_SUFFIX(stdlib_strided_swhere_ndarray)( N, dataC, strideC, offsetC, dataX, strideX, offsetX, dataY, strideY, offsetY, dataOut, strideOut, offsetOut ); |
Same layout as blas/ext/base/ndarray/dxpy/src/main.c.
| #endif | ||
|
|
||
| /** | ||
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition and assigns them to an output ndarray. |
There was a problem hiding this comment.
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition and assigns them to an output ndarray. | |
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition. |
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition. | ||
| * | ||
| * @private | ||
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing ndarrays | ||
| * @returns {ndarray} output ndarray |
There was a problem hiding this comment.
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition. | |
| * | |
| * @private | |
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing ndarrays | |
| * @returns {ndarray} output ndarray | |
| * Takes elements from one of two one-dimensional single-precision floating-point ndarrays depending on a condition. | |
| * | |
| * ## Notes | |
| * | |
| * - The function expects the following ndarrays: | |
| * | |
| * - a one-dimensional condition ndarray. | |
| * - first one-dimensional input ndarray. | |
| * - second one-dimensional input ndarray. | |
| * - a one-dimensional output ndarray. | |
| * | |
| * @private | |
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing ndarrays | |
| * @returns {ndarray} output ndarray | |
| * | |
| * @example | |
| * var BooleanVector = require( '@stdlib/ndarray/vector/bool' ); | |
| * var Float32Vector = require( '@stdlib/ndarray/vector/float32' ); | |
| * | |
| * var condition = new BooleanVector( [ true, false, true, false, true ] ); | |
| * var x = new Float32Vector( [ 1.0, 2.0, 3.0, 4.0, 5.0 ] ); | |
| * var y = new Float32Vector( [ 6.0, 7.0, 8.0, 9.0, 10.0 ] ); | |
| * var out = new Float32Vector( 5 ); | |
| * | |
| * var v = swhere( [ condition, x, y, out ] ); | |
| * // returns <ndarray>[ 1.0, 7.0, 3.0, 9.0, 5.0 ] |
Notes and example copied from lib/main.js, as in blas/ext/base/ndarray/dxpy/lib/native.js.
| <!-- /.examples --> | ||
|
|
||
| <section class="c"> |
There was a problem hiding this comment.
| <!-- /.examples --> | |
| <section class="c"> | |
| <!-- /.examples --> | |
| <!-- C interface documentation. --> | |
| * * * | |
| <section class="c"> |
|
|
||
| ```c | ||
| const struct ndarray *arrays[] = { condition, x, y, out }; | ||
| stdlib_blas_ext_swhere( arrays ); |
There was a problem hiding this comment.
| stdlib_blas_ext_swhere( arrays ); | |
| #include "stdlib/ndarray/ctor.h" | |
| #include "stdlib/ndarray/dtypes.h" | |
| #include "stdlib/ndarray/index_modes.h" | |
| #include "stdlib/ndarray/orders.h" | |
| #include "stdlib/ndarray/base/bytes_per_element.h" | |
| #include <stdbool.h> | |
| #include <stdint.h> | |
| // Create ndarrays: | |
| const bool dataC[] = { true, false, true, false, true }; | |
| const float dataX[] = { 1.0f, 2.0f, 3.0f, 4.0f, 5.0f }; | |
| const float dataY[] = { 6.0f, 7.0f, 8.0f, 9.0f, 10.0f }; | |
| float dataOut[] = { 0.0f, 0.0f, 0.0f, 0.0f, 0.0f }; | |
| int64_t shape[] = { 5 }; | |
| int64_t cstrides[] = { STDLIB_NDARRAY_BOOL_BYTES_PER_ELEMENT }; | |
| int64_t strides[] = { STDLIB_NDARRAY_FLOAT32_BYTES_PER_ELEMENT }; | |
| int8_t submodes[] = { STDLIB_NDARRAY_INDEX_ERROR }; | |
| struct ndarray *condition = stdlib_ndarray_allocate( STDLIB_NDARRAY_BOOL, (uint8_t *)dataC, 1, shape, cstrides, 0, STDLIB_NDARRAY_ROW_MAJOR, STDLIB_NDARRAY_INDEX_ERROR, 1, submodes ); | |
| struct ndarray *x = stdlib_ndarray_allocate( STDLIB_NDARRAY_FLOAT32, (uint8_t *)dataX, 1, shape, strides, 0, STDLIB_NDARRAY_ROW_MAJOR, STDLIB_NDARRAY_INDEX_ERROR, 1, submodes ); | |
| struct ndarray *y = stdlib_ndarray_allocate( STDLIB_NDARRAY_FLOAT32, (uint8_t *)dataY, 1, shape, strides, 0, STDLIB_NDARRAY_ROW_MAJOR, STDLIB_NDARRAY_INDEX_ERROR, 1, submodes ); | |
| struct ndarray *out = stdlib_ndarray_allocate( STDLIB_NDARRAY_FLOAT32, (uint8_t *)dataOut, 1, shape, strides, 0, STDLIB_NDARRAY_ROW_MAJOR, STDLIB_NDARRAY_INDEX_ERROR, 1, submodes ); | |
| // Perform computation: | |
| const struct ndarray *arrays[] = { condition, x, y, out }; | |
| stdlib_blas_ext_swhere( arrays ); | |
| // Free allocated memory: | |
| stdlib_ndarray_free( condition ); | |
| stdlib_ndarray_free( x ); | |
| stdlib_ndarray_free( y ); | |
| stdlib_ndarray_free( out ); |
The usage block should show a full example, as in blas/ext/base/ndarray/dxpy/README.md.
| The function accepts the following argument: | ||
|
|
||
| - **arrays**: `[in] struct ndarray**` list containing a one-dimensional boolean condition ndarray, two one-dimensional single-precision floating-point input ndarrays, and a one-dimensional single-precision floating-point output ndarray. |
There was a problem hiding this comment.
| The function accepts the following argument: | |
| - **arrays**: `[in] struct ndarray**` list containing a one-dimensional boolean condition ndarray, two one-dimensional single-precision floating-point input ndarrays, and a one-dimensional single-precision floating-point output ndarray. | |
| The function accepts the following arguments: | |
| - **arrays**: `[in] struct ndarray**` list containing the following ndarrays: | |
| - `[in] struct ndarray*` a one-dimensional condition ndarray. | |
| - `[in] struct ndarray*` first one-dimensional input ndarray. | |
| - `[in] struct ndarray*` second one-dimensional input ndarray. | |
| - `[out] struct ndarray*` a one-dimensional output ndarray. |
|
|
||
| <section class="examples"> | ||
|
|
||
| ### Examples |
There was a problem hiding this comment.
The C Examples section is empty. It should contain the code from examples/c/example.c (minus the // cppcheck-suppress lines), as in blas/ext/base/ndarray/dxpy/README.md.
There was a problem hiding this comment.
Mind restructuring this to match the other add-on examples (e.g., blas/ext/base/ndarray/dxpy/examples/c/example.c): one commented step per setting (ndims, shape, strides, offset, order, index mode), // cppcheck-suppress invalidPointerCast on its own line above each stdlib_ndarray_allocate, and exit( 1 ) on allocation failure instead of freeing and returning.
There was a problem hiding this comment.
Same here: mind following the structure of blas/ext/base/ndarray/swax/benchmark/c/benchmark.length.c: variables declared at the top, random_uniform, the should not return NaN checks, and no goto?
There was a problem hiding this comment.
test/test.js should only contain the main-export and native/JS selection tests (see blas/ext/base/ndarray/dxpy/test/test.js). The remaining tests (arity, condition selection, strides, offsets, empty ndarrays) already live in test.main.js and test.native.js, so mind removing them from here?
| cbuf = new BooleanArray( [ false, true, false, true, false, false ] ); | ||
| xbuf = new Float32Array( [ 1.0, 100.0, 2.0, 100.0, 3.0, 100.0, 4.0 ] ); | ||
| ybuf = new Float32Array( [ 40.0, 30.0, 20.0, 10.0 ] ); | ||
| obuf = new Float32Array( 12 ); |
There was a problem hiding this comment.
| obuf = new Float32Array( 12 ); | |
| obuf = new Float32Array( [ 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0 ] ); |
The moved tests should stay the same as test/test.js on develop, with only the require changed. The comment edits in this test (// cbuf[4] = false => y[3] → // false => y[3]) should be reverted as well. Same in test.native.js.
|
Thanks @ujjwalv01 for reviewing the PR , I have addressed the requested changes |
| */ | ||
| static void print_summary( int total, int passing ) { | ||
| printf( "#\n" ); | ||
| printf( "1..%d\n", total ); |
There was a problem hiding this comment.
| printf( "1..%d\n", total ); | |
| printf( "1..%d\n", total ); // TAP plan |
Keep the shared boilerplate unchanged.
| } | ||
|
|
||
| /** | ||
| * Prints benchmark results. |
There was a problem hiding this comment.
| * Prints benchmark results. | |
| * Prints benchmarks results. |
| t.strictEqual( swhere( [ c, x, y, out ] ), out, 'returns the output ndarray' ); | ||
| t.strictEqual( isSameFloat32Array( obuf, new Float32Array( [ 1, 5, 3 ] ) ), true, 'selects the expected values' ); | ||
| t.end(); | ||
| }); |
There was a problem hiding this comment.
| }); |
test.native.js should mirror test/test.js on develop (only the native wiring changes), so mind removing this extra test L394 - L415 ?
| var tape = require( 'tape' ); | ||
| var isSameFloat32Array = require( '@stdlib/assert/is-same-float32array' ); | ||
| var Float32Array = require( '@stdlib/array/float32' ); | ||
| var Uint8Array = require( '@stdlib/array/uint8' ); |
There was a problem hiding this comment.
| var Uint8Array = require( '@stdlib/array/uint8' ); |
Not needed once the extra test is removed.
|
|
||
| ## C APIs | ||
|
|
||
| <section class="intro"> |
There was a problem hiding this comment.
| <section class="intro"> | |
| <!-- Section to include introductory text. Make sure to keep an empty line after the intro `section` element and another before the `/section` close. --> | |
| <section class="intro"> |
Keep the standard comments, as in dxpy.
|
|
||
| <!-- /.intro --> | ||
|
|
||
| <section class="usage"> |
There was a problem hiding this comment.
| <section class="usage"> | |
| <!-- C usage documentation. --> | |
| <section class="usage"> |
|
|
||
| <!-- /.usage --> | ||
|
|
||
| <section class="notes"> |
There was a problem hiding this comment.
| <section class="notes"> | |
| <!-- C API usage notes. Make sure to keep an empty line after the `section` element and another before the `/section` close. --> | |
| <section class="notes"> |
|
|
||
| <!-- /.notes --> | ||
|
|
||
| <section class="examples"> |
There was a problem hiding this comment.
| <section class="examples"> | |
| <!-- C API usage examples. --> | |
| <section class="examples"> |
|
Thanks for the updates, looks much better! A few small leftovers. Mind making these? |
addressed the remaining comments. |
Resolves a part of #14819
Description
This pull request:
swhere, using the existing strided C routine.Related Issues
This pull request has the following related issues:
blas/ext/base/ndarraypackages #14819Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
used AI for analyzing pattern of recently merged PRs and test and benchmarking generation.
@stdlib-js/reviewers