Skip to content
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

refactor: update blas/ext/base/grev to follow current project conventions #4659

Merged
merged 3 commits into from
Jan 11, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 10 additions & 15 deletions lib/node_modules/@stdlib/blas/ext/base/grev/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ limitations under the License.
var grev = require( '@stdlib/blas/ext/base/grev' );
```

#### grev( N, x, stride )
#### grev( N, x, strideX )

Reverses a strided array `x` in-place.
Reverses a strided array in-place.

```javascript
var x = [ -2.0, 1.0, 3.0, -5.0, 4.0, 0.0, -1.0, -3.0 ];
Expand All @@ -45,41 +45,36 @@ The function has the following parameters:

- **N**: number of indexed elements.
- **x**: input array.
- **stride**: index increment.
- **strideX**: stride length.

The `N` and `stride` parameters determine which elements in `x` are accessed at runtime. For example, to reverse every other element
The `N` and stride parameters determine which elements in the strided array are accessed at runtime. For example, to reverse every other element:

```javascript
var floor = require( '@stdlib/math/base/special/floor' );

var x = [ -2.0, 1.0, 3.0, -5.0, 4.0, 0.0, -1.0, -3.0 ];
var N = floor( x.length / 2 );

grev( N, x, 2 );
grev( 4, x, 2 );
// x => [ -1.0, 1.0, 4.0, -5.0, 3.0, 0.0, -2.0, -3.0 ]
```

Note that indexing is relative to the first index. To introduce an offset, use [`typed array`][mdn-typed-array] views.

```javascript
var Float64Array = require( '@stdlib/array/float64' );
var floor = require( '@stdlib/math/base/special/floor' );

// Initial array...
var x0 = new Float64Array( [ 1.0, -2.0, 3.0, -4.0, 5.0, -6.0 ] );

// Create an offset view...
var x1 = new Float64Array( x0.buffer, x0.BYTES_PER_ELEMENT*1 ); // start at 2nd element
var N = floor( x0.length/2 );

// Reverse every other element...
grev( N, x1, 2 );
grev( 3, x1, 2 );
// x0 => <Float64Array>[ 1.0, -6.0, 3.0, -4.0, 5.0, -2.0 ]
```

#### grev.ndarray( N, x, stride, offset )
#### grev.ndarray( N, x, strideX, offsetX )

Reverses a strided array `x` in-place using alternative indexing semantics.
Reverses a strided array in-place using alternative indexing semantics.

```javascript
var x = [ -2.0, 1.0, 3.0, -5.0, 4.0, 0.0, -1.0, -3.0 ];
Expand All @@ -90,9 +85,9 @@ grev.ndarray( x.length, x, 1, 0 );

The function has the following additional parameters:

- **offset**: starting index.
- **offsetX**: starting index.

While [`typed array`][mdn-typed-array] views mandate a view offset based on the underlying `buffer`, the `offset` parameter supports indexing semantics based on a starting index. For example, to access only the last three elements of `x`
While [`typed array`][mdn-typed-array] views mandate a view offset based on the underlying buffer, the offset parameter supports indexing semantics based on a starting index. For example, to access only the last three elements:

```javascript
var x = [ 1.0, -2.0, 3.0, -4.0, 5.0, -6.0 ];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
// MODULES //

var bench = require( '@stdlib/bench' );
var randu = require( '@stdlib/random/base/randu' );
var uniform = require( '@stdlib/random/base/uniform' ).factory;
var Float64Array = require( '@stdlib/array/float64' );
var gfillBy = require( '@stdlib/blas/ext/base/gfill-by' );
var isnan = require( '@stdlib/math/base/assert/is-nan' );
var pow = require( '@stdlib/math/base/special/pow' );
var pkg = require( './../package.json' ).name;
Expand All @@ -38,13 +40,7 @@ var grev = require( './../lib/main.js' );
* @returns {Function} benchmark function
*/
function createBenchmark( len ) {
var x;
var i;

x = [];
for ( i = 0; i < len; i++ ) {
x.push( ( randu()*20.0 ) - 10.0 );
}
var x = gfillBy( len, new Float64Array( len ), 1, uniform( -100, 100 ) );
Copy link
Member

@Planeshifter Planeshifter Jan 11, 2025

Choose a reason for hiding this comment

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

Can we please change this back to a generic array instead of Float64Array.

Suggested change
var x = gfillBy( len, new Float64Array( len ), 1, uniform( -100, 100 ) );
var x = gfillBy( len, new Array( len ), 1, uniform( -100, 100 ) );

Copy link
Member

Choose a reason for hiding this comment

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

Same comment.

return benchmark;

function benchmark( b ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
// MODULES //

var bench = require( '@stdlib/bench' );
var randu = require( '@stdlib/random/base/randu' );
var uniform = require( '@stdlib/random/base/uniform' ).factory;
var Float64Array = require( '@stdlib/array/float64' );
var gfillBy = require( '@stdlib/blas/ext/base/gfill-by' );
var isnan = require( '@stdlib/math/base/assert/is-nan' );
var pow = require( '@stdlib/math/base/special/pow' );
var pkg = require( './../package.json' ).name;
var grev = require( './../lib/main.js' ).ndarray;
var grev = require( './../lib/ndarray.js' );


// FUNCTIONS //
Expand All @@ -38,13 +40,7 @@ var grev = require( './../lib/main.js' ).ndarray;
* @returns {Function} benchmark function
*/
function createBenchmark( len ) {
var x;
var i;

x = [];
for ( i = 0; i < len; i++ ) {
x.push( ( randu()*20.0 ) - 10.0 );
}
var x = gfillBy( len, new Float64Array( len ), 1, uniform( -100, 100 ) );
Copy link
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 = gfillBy( len, new Float64Array( len ), 1, uniform( -100, 100 ) );
var x = gfillBy( len, new Array( len ), 1, uniform( -100, 100 ) );

Copy link
Member

Choose a reason for hiding this comment

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

@Planeshifter This wasn't the correct change. If we wanted generic arrays, then we should have used @stdlib/array/base/zeros to ensure that the resultant array is not in dictionary mode.

Copy link
Member

Choose a reason for hiding this comment

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

That's problematic, especially for large arrays, as it can tank performance, thus throwing off benchmark results.

Copy link
Member

Choose a reason for hiding this comment

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

@headlessNode Would you mind submitting a follow-up PR addressing this?

Copy link
Member

Choose a reason for hiding this comment

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

This would also be a good lint rule. Namely, we should disallow new Array(...).

Copy link
Member Author

Choose a reason for hiding this comment

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

@kgryte I actually did use the @stdlib/array/base/zeros, see next commit.

Copy link
Member

Choose a reason for hiding this comment

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

@headlessNode One step ahead of me!

Copy link
Member

Choose a reason for hiding this comment

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

I should have looked at the final code, rather than just relying on the e-mail thread of discussions.

return benchmark;

function benchmark( b ) {
Expand Down
33 changes: 15 additions & 18 deletions lib/node_modules/@stdlib/blas/ext/base/grev/docs/repl.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@

{{alias}}( N, x, stride )
{{alias}}( N, x, strideX )
Reverses a strided array in-place.

The `N` and `stride` parameters determine which elements in `x` are accessed
at runtime.
The `N` and stride parameters determine which elements in the strided array
are accessed at runtime.

Indexing is relative to the first index. To introduce an offset, use typed
array views.
Expand All @@ -18,8 +18,8 @@
x: ArrayLikeObject
Input array.

stride: integer
Index increment for `x`.
strideX: integer
Stride length.

Returns
-------
Expand All @@ -33,27 +33,25 @@
> {{alias}}( x.length, x, 1 )
[ -3.0, -1.0, 4.0, -5.0, 3.0, 1.0, -2.0 ]

// Using `N` and `stride` parameters:
// Using `N` and stride parameters:
> x = [ -2.0, 1.0, 3.0, -5.0, 4.0, -1.0, -3.0 ];
> var N = {{alias:@stdlib/math/base/special/floor}}( x.length / 2 );
> {{alias}}( N, x, 2 )
> {{alias}}( 3, x, 2 )
[ 4.0, 1.0, 3.0, -5.0, -2.0, -1.0, -3.0 ]

// Using view offsets:
> var x0 = new {{alias:@stdlib/array/float64}}( [ 1.0, -2.0, 3.0, -4.0, 5.0, -6.0 ] );
> var x1 = new {{alias:@stdlib/array/float64}}( x0.buffer, x0.BYTES_PER_ELEMENT*1 );
> N = {{alias:@stdlib/math/base/special/floor}}( x0.length / 2 );
> {{alias}}( N, x1, 2 )
> {{alias}}( 3, x1, 2 )
<Float64Array>[ -6.0, 3.0, -4.0, 5.0, -2.0 ]
> x0
<Float64Array>[ 1.0, -6.0, 3.0, -4.0, 5.0, -2.0 ]


{{alias}}.ndarray( N, x, stride, offset )
{{alias}}.ndarray( N, x, strideX, offsetX )
Reverses a strided array in-place using alternative indexing semantics.

While typed array views mandate a view offset based on the underlying
buffer, the `offset` parameter supports indexing semantics based on a
buffer, the offset parameter supports indexing semantics based on a
starting index.

Parameters
Expand All @@ -64,11 +62,11 @@
x: ArrayLikeObject
Input array.

stride: integer
Index increment for `x`.
strideX: integer
Stride length.

offset: integer
Starting index of `x`.
offsetX: integer
Starting index.

Returns
-------
Expand All @@ -84,8 +82,7 @@

// Using an index offset:
> x = [ 1.0, -2.0, 3.0, -4.0, 5.0, -6.0 ];
> var N = {{alias:@stdlib/math/base/special/floor}}( x.length / 2 );
> {{alias}}.ndarray( N, x, 2, 1 )
> {{alias}}.ndarray( 3, x, 2, 1 )
[ 1.0, -6.0, 3.0, -4.0, 5.0, -2.0 ]

See Also
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ interface Routine {
*
* @param N - number of indexed elements
* @param x - input array
* @param stride - stride length
* @param strideX - stride length
* @returns `x`
*
* @example
Expand All @@ -40,15 +40,15 @@ interface Routine {
* grev( x.length, x, 1 );
* // x => [ -3.0, -1.0, 0.0, 4.0, -5.0, 3.0, 1.0, -2.0 ]
*/
<T = unknown>( N: number, x: Collection<T>, stride: number ): Collection<T>;
<T = unknown>( N: number, x: Collection<T>, strideX: number ): Collection<T>;

/**
* Reverses a strided array in-place. using alternative indexing semantics.
*
* @param N - number of indexed elements
* @param x - input array
* @param stride - stride length
* @param offset - starting index
* @param strideX - stride length
* @param offsetX - starting index
* @returns `x`
*
* @example
Expand All @@ -57,15 +57,15 @@ interface Routine {
* grev.ndarray( x.length, x, 1, 0 );
* // x => [ -3.0, -1.0, 0.0, 4.0, -5.0, 3.0, 1.0, -2.0 ]
*/
ndarray<T = unknown>( N: number, x: Collection<T>, stride: number, offset: number ): Collection<T>;
ndarray<T = unknown>( N: number, x: Collection<T>, strideX: number, offsetX: number ): Collection<T>;
}

/**
* Reverses a strided array in-place.
*
* @param N - number of indexed elements
* @param x - input array
* @param stride - stride length
* @param strideX - stride length
* @returns `x`
*
* @example
Expand Down
14 changes: 7 additions & 7 deletions lib/node_modules/@stdlib/blas/ext/base/grev/lib/accessors.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ var floor = require( '@stdlib/math/base/special/floor' );
* @param {Object} x - input array object
* @param {Collection} x.data - input array data
* @param {Array<Function>} x.accessors - array element accessors
* @param {integer} stride - index increment
* @param {NonNegativeInteger} offset - starting index
* @param {integer} strideX - stride length
* @param {NonNegativeInteger} offsetX - starting index
* @returns {Object} input array object
*
* @example
Expand Down Expand Up @@ -63,7 +63,7 @@ var floor = require( '@stdlib/math/base/special/floor' );
* var view = reinterpret64( x.data, 0 );
* // view => <Float32Array>[ -1.0, -3.0, 4.0, 0.0, 3.0, -5.0, -2.0, 1.0 ]
*/
function grev( N, x, stride, offset ) {
function grev( N, x, strideX, offsetX ) {
var xbuf;
var set;
var get;
Expand All @@ -81,14 +81,14 @@ function grev( N, x, stride, offset ) {
set = x.accessors[ 1 ];

n = floor( N/2 );
ix = offset;
iy = ix + ((N-1)*stride);
ix = offsetX;
iy = ix + ( ( N - 1 ) * strideX );
for ( i = 0; i < n; i++ ) {
tmp = get( xbuf, ix );
set( xbuf, ix, get( xbuf, iy ) );
set( xbuf, iy, tmp );
ix += stride;
iy -= stride;
ix += strideX;
iy -= strideX;
}
return x;
}
Expand Down
Loading
Loading