feat: add blas/ext/base/ndarray/dapx#9220
feat: add blas/ext/base/ndarray/dapx#9220Amansingh0807 wants to merge 9 commits intostdlib-js:developfrom
blas/ext/base/ndarray/dapx#9220Conversation
Coverage Report
The above coverage report was generated for the changes in this PR. |
| var dapx = require( '@stdlib/blas/ext/base/ndarray/dapx' ); | ||
| ``` | ||
|
|
||
| #### dapx( x, alpha ) |
There was a problem hiding this comment.
This isn't correct. You need to study https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/blas/ext/base/ndarray/dcusum. alpha should be a zero-dimensional ndarray and there should be only a single argument arrays containing two arrays: the input ndarray and the zero-dimensional ndarray corresponding to alpha.
This comment applies to your other PRs for *apx.
lib/node_modules/@stdlib/blas/ext/base/ndarray/dapx/benchmark/benchmark.js
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,27 @@ | |||
| {{alias}}( x, alpha ) | |||
There was a problem hiding this comment.
Missing empty line. Study other packages.
|
|
||
| // The function returns an ndarray... | ||
| { | ||
| const x: any = null; |
There was a problem hiding this comment.
This is very clearly not what we want to do.
|
|
||
| // The compiler throws an error if the function is provided a second argument which is not a number... | ||
| { | ||
| const x: any = null; |
| /** | ||
| * Adds a scalar constant to each element in a double-precision floating-point ndarray. | ||
| * | ||
| * @param {float64ndarray} x - input ndarray |
There was a problem hiding this comment.
This isn't a valid type for JSDoc. Study other packages.
| var buf; | ||
| var sx; | ||
| var ox; | ||
| var N; | ||
|
|
||
| N = numelDimension( x, 0 ); | ||
| if ( N <= 0 ) { | ||
| return x; | ||
| } | ||
| buf = getData( x ); | ||
| sx = getStride( x, 0 ); | ||
| ox = getOffset( x ); | ||
|
|
||
| strided( N, alpha, buf, sx, ox ); | ||
| return x; |
kgryte
left a comment
There was a problem hiding this comment.
Left initial comments. This PR (and related PRs) need significant clean-up before they can move forward.
|
Thanks for the detailed guidance @kgryte. It really clarified the architectural pattern for me. I have refactored the implementation to align with dcusum and other packages. The function now accepts a single arrays argument, and I am using ndarraylike2scalar for correct value extraction. I have also updated the JSDoc types and examples (using the new notation) and will apply these changes across all my related *apx PRs. This one is Ready for review. |
|
|
||
| # dapx | ||
|
|
||
| > Add a scalar constant to each element in a double-precision floating-point ndarray. |
There was a problem hiding this comment.
| > Add a scalar constant to each element in a double-precision floating-point ndarray. | |
| > Add a scalar constant to each element in a one-dimensional double-precision floating-point ndarray. |
Applies here and throughout this PR.
| Note that indexing is relative to the first index. To introduce an offset, use [`ndarray`][@stdlib/ndarray/ctor] view creation. | ||
|
|
||
| ```javascript | ||
| var Float64Array = require( '@stdlib/array/float64' ); | ||
| var ndarray = require( '@stdlib/ndarray/ctor' ); | ||
| var scalar2ndarray = require( '@stdlib/ndarray/from-scalar' ); | ||
|
|
||
| // Initial array: | ||
| var xbuf = new Float64Array( [ 1.0, 2.0, 3.0, 4.0, 5.0 ] ); | ||
|
|
||
| // Create an ndarray view: | ||
| var x = new ndarray( 'float64', xbuf, [ 3 ], [ 1 ], 2, 'row-major' ); | ||
|
|
||
| var alpha = scalar2ndarray( 5.0, { | ||
| 'dtype': 'float64' | ||
| }); | ||
|
|
||
| var out = dapx( [ x, alpha ] ); | ||
| // returns <ndarray> | ||
| ``` |
There was a problem hiding this comment.
| Note that indexing is relative to the first index. To introduce an offset, use [`ndarray`][@stdlib/ndarray/ctor] view creation. | |
| ```javascript | |
| var Float64Array = require( '@stdlib/array/float64' ); | |
| var ndarray = require( '@stdlib/ndarray/ctor' ); | |
| var scalar2ndarray = require( '@stdlib/ndarray/from-scalar' ); | |
| // Initial array: | |
| var xbuf = new Float64Array( [ 1.0, 2.0, 3.0, 4.0, 5.0 ] ); | |
| // Create an ndarray view: | |
| var x = new ndarray( 'float64', xbuf, [ 3 ], [ 1 ], 2, 'row-major' ); | |
| var alpha = scalar2ndarray( 5.0, { | |
| 'dtype': 'float64' | |
| }); | |
| var out = dapx( [ x, alpha ] ); | |
| // returns <ndarray> | |
| ``` |
| ## Notes | ||
|
|
||
| - The function **mutates** the input ndarray. | ||
|
|
There was a problem hiding this comment.
| ## Notes | |
| - The function **mutates** the input ndarray. |
| }); | ||
| var x = new ndarray( 'float64', xbuf, [ 10 ], [ 1 ], 0, 'row-major' ); | ||
|
|
||
| console.log( x.data ); |
There was a problem hiding this comment.
Study other packages. Use ndarray2array.
|
|
||
| The function has the following parameters: | ||
|
|
||
| - **arrays**: array-like object containing an input ndarray and a zero-dimensional ndarray containing the scalar constant. |
There was a problem hiding this comment.
| - **arrays**: array-like object containing an input ndarray and a zero-dimensional ndarray containing the scalar constant. | |
| - **arrays**: array-like object containing a one-dimensional input ndarray and a zero-dimensional ndarray containing a scalar constant. |
| }); | ||
|
|
||
| var out = dapx( [ x, alpha ] ); | ||
| // returns <ndarray> |
There was a problem hiding this comment.
| // returns <ndarray> | |
| // returns <ndarray>[ 6.0, 7.0, 8.0, 9.0 ] |
There was a problem hiding this comment.
Show what the expected values are.
| var dapx = require( './../lib' ); | ||
|
|
||
|
|
||
| // FUNCTIONS // |
There was a problem hiding this comment.
| // FUNCTIONS // | |
| // VARIABLES // | |
| var options = { | |
| 'dtype': 'float64' | |
| }; | |
| // FUNCTIONS // |
Prefer parameterization to limit copy-paste mistakes in other packages. This is one reason to first do one package (e.g., dapx) before moving on to other packages (e.g., sapx, gapx), as it reduces the number of changes you need to make. Currently, you are having to replicate changes.
| xbuf = discreteUniform( len, -100, 100, { | ||
| 'dtype': 'float64' | ||
| }); | ||
| x = new ndarray( 'float64', xbuf, [ len ], [ 1 ], 0, 'row-major' ); | ||
| alpha = scalar2ndarray( 5.0, { | ||
| 'dtype': 'float64' | ||
| }); |
There was a problem hiding this comment.
| xbuf = discreteUniform( len, -100, 100, { | |
| 'dtype': 'float64' | |
| }); | |
| x = new ndarray( 'float64', xbuf, [ len ], [ 1 ], 0, 'row-major' ); | |
| alpha = scalar2ndarray( 5.0, { | |
| 'dtype': 'float64' | |
| }); | |
| xbuf = discreteUniform( len, -100, 100, options ); | |
| x = new ndarray( options.dtype, xbuf, [ len ], [ 1 ], 0, 'row-major' ); | |
| alpha = scalar2ndarray( 5.0, options ); |
|
|
||
| var bench = require( '@stdlib/bench' ); | ||
| var discreteUniform = require( '@stdlib/random/array/discrete-uniform' ); | ||
| var isnanf = require( '@stdlib/math/base/assert/is-nanf' ); |
There was a problem hiding this comment.
It is not clear to me why you are using a single-precision assertion package here.
| @@ -0,0 +1,27 @@ | |||
| {{alias}}( arrays ) | |||
There was a problem hiding this comment.
See other packages. This was not the correct change.
| > var x = {{alias:@stdlib/ndarray/ctor}}( 'float64', xbuf, [ 4 ], [ 1 ], 0, 'row-major' ); | ||
| > var alpha = {{alias:@stdlib/ndarray/from-scalar}}( 5.0, { 'dtype': 'float64' } ); | ||
| > {{alias}}( [ x, alpha ] ) | ||
| <ndarray> |
| * var out = dapx( [ x, alpha ] ); | ||
| * // returns <ndarray> | ||
| */ | ||
| declare function dapx( arrays: ArrayLike<float64ndarray> ): float64ndarray; |
There was a problem hiding this comment.
| declare function dapx( arrays: ArrayLike<float64ndarray> ): float64ndarray; | |
| declare function dapx( arrays: [ float64ndarray, float64ndarray ] ): float64ndarray; |
By using ArrayLike, you end up losing type specificity. Here, we can specify that one must provide exactly two ndarrays.
| * }); | ||
| * | ||
| * var out = dapx( [ x, alpha ] ); | ||
| * // returns <ndarray> |
| * }); | ||
| * | ||
| * var out = dapx( [ x, alpha ] ); | ||
| * // returns <ndarray> |
| /** | ||
| * Adds a scalar constant to each element in a double-precision floating-point ndarray. | ||
| * | ||
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing an input ndarray and a zero-dimensional ndarray containing the scalar constant |
There was a problem hiding this comment.
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing an input ndarray and a zero-dimensional ndarray containing the scalar constant | |
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing a one-dimensional input ndarray and a zero-dimensional ndarray containing the scalar constant |
| // MAIN // | ||
|
|
||
| /** | ||
| * Adds a scalar constant to each element in a double-precision floating-point ndarray. |
There was a problem hiding this comment.
| * Adds a scalar constant to each element in a double-precision floating-point ndarray. | |
| * Adds a scalar constant to each element in a one-dimensional double-precision floating-point ndarray. |
| * }); | ||
| * | ||
| * var out = dapx( [ x, alpha ] ); | ||
| * // returns <ndarray> |
|
|
||
| if ( numelDimension( x, 0 ) <= 0 ) { | ||
| return x; | ||
| } |
There was a problem hiding this comment.
| if ( numelDimension( x, 0 ) <= 0 ) { | |
| return x; | |
| } |
| { | ||
| "name": "@stdlib/blas/ext/base/ndarray/dapx", | ||
| "version": "0.0.0", | ||
| "description": "Add a scalar constant to each element in a double-precision floating-point ndarray.", |
There was a problem hiding this comment.
| "description": "Add a scalar constant to each element in a double-precision floating-point ndarray.", | |
| "description": "Add a scalar constant to each element in a one-dimensional double-precision floating-point ndarray.", |
| "constant", | ||
| "plus", | ||
| "increment", | ||
| "augment", |
| "array", | ||
| "multidimensional", | ||
| "add", | ||
| "constant", |
| t.end(); | ||
| }); | ||
|
|
||
| tape( 'the function adds a scalar constant to each element in a double-precision floating-point ndarray', function test( t ) { |
There was a problem hiding this comment.
| tape( 'the function adds a scalar constant to each element in a double-precision floating-point ndarray', function test( t ) { | |
| tape( 'the function adds a scalar constant to each element in an ndarray', function test( t ) { |
There was a problem hiding this comment.
Minimize future copy-paste errors.
|
|
||
| dapx( [ x, alpha ] ); | ||
|
|
||
| t.deepEqual( x.data, expected, 'returns expected value' ); |
There was a problem hiding this comment.
Use ndarray2array or getData (see @stdlib/ndarray/data-buffer). Meaning, prefer functional accessors in order to minimize future migration burden should we ever rename properties, etc.
| t.end(); | ||
| }); | ||
|
|
||
| tape( 'the function supports ndarrays with an offset', function test( t ) { |
There was a problem hiding this comment.
| tape( 'the function supports ndarrays with an offset', function test( t ) { | |
| tape( 'the function supports ndarrays with non-zero offsets', function test( t ) { |
| tape( 'the function supports adding zero', function test( t ) { | ||
| var expected; | ||
| var alpha; | ||
| var xbuf; | ||
| var x; | ||
|
|
||
| xbuf = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | ||
| x = new ndarray( 'float64', xbuf, [ 4 ], [ 1 ], 0, 'row-major' ); | ||
|
|
||
| expected = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | ||
|
|
||
| alpha = scalar2ndarray( 0.0, { | ||
| 'dtype': 'float64' | ||
| }); | ||
|
|
||
| dapx( [ x, alpha ] ); | ||
|
|
||
| t.deepEqual( x.data, expected, 'returns expected value' ); | ||
| t.end(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
| tape( 'the function supports adding zero', function test( t ) { | |
| var expected; | |
| var alpha; | |
| var xbuf; | |
| var x; | |
| xbuf = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | |
| x = new ndarray( 'float64', xbuf, [ 4 ], [ 1 ], 0, 'row-major' ); | |
| expected = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | |
| alpha = scalar2ndarray( 0.0, { | |
| 'dtype': 'float64' | |
| }); | |
| dapx( [ x, alpha ] ); | |
| t.deepEqual( x.data, expected, 'returns expected value' ); | |
| t.end(); | |
| }); |
This test is not particularly informative.
| tape( 'the function supports negative constants', function test( t ) { | ||
| var expected; | ||
| var alpha; | ||
| var xbuf; | ||
| var x; | ||
|
|
||
| xbuf = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | ||
| x = new ndarray( 'float64', xbuf, [ 4 ], [ 1 ], 0, 'row-major' ); | ||
|
|
||
| expected = new Float64Array( [ -4.0, -3.0, -2.0, -1.0 ] ); | ||
|
|
||
| alpha = scalar2ndarray( -5.0, { | ||
| 'dtype': 'float64' | ||
| }); | ||
|
|
||
| dapx( [ x, alpha ] ); | ||
|
|
||
| t.deepEqual( x.data, expected, 'returns expected value' ); | ||
| t.end(); | ||
| }); |
There was a problem hiding this comment.
| tape( 'the function supports negative constants', function test( t ) { | |
| var expected; | |
| var alpha; | |
| var xbuf; | |
| var x; | |
| xbuf = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | |
| x = new ndarray( 'float64', xbuf, [ 4 ], [ 1 ], 0, 'row-major' ); | |
| expected = new Float64Array( [ -4.0, -3.0, -2.0, -1.0 ] ); | |
| alpha = scalar2ndarray( -5.0, { | |
| 'dtype': 'float64' | |
| }); | |
| dapx( [ x, alpha ] ); | |
| t.deepEqual( x.data, expected, 'returns expected value' ); | |
| t.end(); | |
| }); |
Neither is this one.
kgryte
left a comment
There was a problem hiding this comment.
Left another round of comments.
|
Actually, one package you should study is |
- Add 'one-dimensional' to all descriptions - Use ndarray2array to show expected values - Remove dimension check from main function - Update TypeScript types to tuple syntax - Use getData instead of .data in tests - Remove uninformative test cases - Add VARIABLES section in benchmark - Use options parameterization - Change isnanf to isnan for double precision - Remove Notes section from README - Update all documentation with expected outputs
83f8437 to
9943604
Compare
|
Thanks for the extensive review @kgryte. I have refactored this package
I have applied these changes ONLY to this PR first. Once you confirm this implementation meets the standard, I will immediately propagate the exact same pattern to the other pending PRs (sapx, gapx, etc.) to ensure consistency. |
Resolves None.
Description
This pull request:
blas/ext/base/ndarray/dapxRelated Issues
This pull request has the following related issues:
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
@stdlib-js/reviewers