[RFC]: add `blas/base/grot`

[performant23]

Description

This RFC proposes adding the generic JavaScript interface grot to apply plane rotation on input arrays of any type.

Related Issues

N/A

Questions

No.

Other

No.

Checklist

• [X] I have read and understood the Code of Conduct.
• [X] Searched for existing issues and pull requests.
• [X] The issue name begins with RFC:.

[performant23]

Hi @kgryte, I was just testing the implementation and was encountering some errors for the tests pertaining to the accessor arrays. I checked the logs on the implementation at lib/main.js and for the below testcase, got ox and oy as:

tape( 'the function supports an `x` stride (accessors)', function test( t ) {
	var xe;
	var ye;
	var x;
	var y;

	x = new Complex128Array([
		1.0, // 0
		2.0,
		3.0, // 1
		4.0
	]);
	y = new Complex128Array([
		6.0, // 0
		7.0, // 1
		8.0,
		9.0
	]);

	grot( 2, x, 2, y, 1, 0.8, 0.6 );

	xe = new Complex128Array ( [ 4.4, 2.0, 6.6, 4.0 ] );
	ye = new Complex128Array( [ 4.2, 3.8, 8.0, 9.0 ] );

	isApprox( t, reinterpret128( x, 0 ), reinterpret128( xe, 0 ), 2.0 );
	isApprox( t, reinterpret128( y, 0 ), reinterpret128( ye, 0 ), 2.0 );

	t.end();
});

{
  data: Complex128Array {},
  dtype: 'complex128',
  accessorProtocol: false,
  accessors: [ [Function: getArrayLike], [Function: setArrayLike] ]
}
{
  data: Complex128Array {},
  dtype: 'complex128',
  accessorProtocol: false,
  accessors: [ [Function: getArrayLike], [Function: setArrayLike] ]
}

This means that basically when the arguments are passed onto the function, it isn't calling the implementation in accessors.js but instead executing operations intended for numeric arrays.

I checked other implementations (gswap and gcopy) which had accessor array support but I got the same logs and the tests aimed at the implementation for the accessor arrays are failing.

So, could you please guide me on calling and testing the accessor array implementation? Thanks!

[kgryte]

@performant23 That's a bit odd, as when I run the tests for gcopy, I get

the function supports an `x` stride (accessors)

    {
      data: Complex128Array {},
      dtype: 'complex128',
      accessorProtocol: true,
      accessors: [ [Function: getComplex128], [Function: setComplex128] ]
    } {
      data: Complex128Array {},
      dtype: 'complex128',
      accessorProtocol: true,
      accessors: [ [Function: getComplex128], [Function: setComplex128] ]
    }
    ✔ returns expected value

[kgryte]

I'd need to see your complete test file to be able to debug.

[performant23]

Yeah, strange indeed! When I ran gcopy tests, got this:

[performant23]

I'd need to see your complete test file to be able to debug.

Yeah I can open up a draft PR for this!

[kgryte]

Hmm...can you tell me about bit more about how you are running the tests? Also, in the stdlib REPL, run

In [1]: var arraylike2object = require( '@stdlib/array/base/arraylike2object' );

In [2]: arraylike2object( new Complex128Array( [ 1.0, 2.0, 3.0, 4.0 ] ) )
???

[performant23]

So, I used the usual test filter i.e. $ make TESTS_FILTER=".*/blas/base/gcopy/.*" test.

Will get back to you regarding the REPL run.

[kgryte]

Also, what version of Node.js are you running?

[performant23]

My Node.js version is v18.16.0

Also, here's the result from the REPL run:

[kgryte]

Whoa. That is bizarre.

[kgryte]

In the REPL, do

var isAccessorArray = require( '@stdlib/array/base/assert/is-accessor-array' );

var z = new Complex128Array( [ 1.0, 2.0 ] );

isAccessorArray( z )

typeof z.get

typeof z.set

[performant23]

Yeah, this test gives a positive:

In [1]: var isAccessorArray = require( '@stdlib/array/base/assert/is-accessor-array' );

In [2]: var z = new Complex128Array( [ 1.0, 2.0 ] );

In [3]: isAccessorArray( z )
Out[3]: true

In [4]: typeof z.get
Out[4]: 'function'

In [5]: typeof z.set
Out[5]: 'function'

[kgryte]

Okay, that is weird. As isAccessorArray is what is used by arraylike2object.

[kgryte]

Can you also run from the terminal

$ make test TESTS_FILTER=".*/array/base/arraylike2object/.*"

[kgryte]

array/base/arraylike2object has an explicit test for complex number arrays.

[performant23]

yeah, sure!

[performant23]

Got all tests passing except the data buffer accessors one:

[kgryte]

In the source code of arraylike2object, log the values of x and the result of isAccessorArray( x ) (L55-56), and run the tests again.

[performant23]

It logged (additionally, dt is: complex64):

  the function converts an array to a standardized object (data buffer accessors)

    x:  Complex64Array {}
    isAccessorArray(x): false
    √ returns expected value
    √ returns expected value

    × returns expected value
    -------------------------
      operator: equal
      expected: true
      actual:   false
      at: process.processImmediate (node:internal/timers:476:21)

    √ returns expected value
    √ returns expected value
    √ returns expected value
    √ returns expected value
    √ returns expected value
    √ returns expected value

[performant23]

Also, logging the same test value for lib/main.js of is-accessor-array is giving false to me:

var x = new Complex64Array( 10 );
console.log(isAccessorArray(x));

Additionally, logging Array.isArray(value.accessors) is giving false and logging typeof value.accessors[0] is giving this message (Since the previous evaluation is false):

d:\stdlib\lib\node_modules\@stdlib\array\base\assert\is-accessor-array\lib\main.js:46
	console.log("typeof value.accessors[0]", typeof value.accessors[0]);
	                                                               ^

TypeError: Cannot read properties of undefined (reading '0')
    at isAccessorArray (d:\stdlib\lib\node_modules\@stdlib\array\base\assert\is-accessor-array\lib\main.js:46:65)
    at Object.<anonymous> (d:\stdlib\lib\node_modules\@stdlib\array\base\assert\is-accessor-array\lib\main.js:56:13)
    at Module._compile (node:internal/modules/cjs/loader:1254:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Module._load (node:internal/modules/cjs/loader:958:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47

Node.js v18.16.0

[kgryte]

Wait...why are you logging value.accessors? Where is this happening?

[performant23]

This is resolved, thank you @kgryte! I have fixed the implementation. Also, just to confirm, accessor tests are mainly testing the stride support (x, y, negative) and the main implementation working. I have finished the tests for strides. Since for drot as per recent changes, we have bifurcated the main implementation tests into 4 cases for varying (sx, sy) as (1, 1), (2,-2), (-2, 1), and (-1, -2), we'd need the accessor tests for all of those cases right?

[kgryte]

Yes, that would probably be best.

[kgryte]

You use convert a normal array to an accessor array using @stdlib/array/base/to-accessor-array.

[performant23]

You use convert a normal array to an accessor array using @stdlib/array/base/to-accessor-array.

So, the current tests for grot and also gcopy, gswap use Complex128 for accessor tests. And to-accessor-array leaves the array unchanged for the arrays with such data types ("If the provided array-like object already supports the accessor protocol, the function returns the input array unchanged."). In this case, we won't need to pass to-accessor-array right?

(Hope you don't mind me being pedantic :), just wanted to keep things as clear as possible)

[kgryte]

Yes, but you'll want to add separate complex array tests, as the algorithm, IIRC, is different depending on whether real or complex.

[performant23]

Edit: Now that I think about it, maybe we shouldn't use Complex128 for the accessor tests actually cause if we do, we have to set the values for x and y in the form: new Complex 128 ( real, imaginary ) always and so, our output return type remains fixed at Complex128() for any accessor array which we might not want. A counterexample would be converting a numeric array to an accessor array which won't have any real or imaginary components.

But what if Complex128Array is tested against the accessor array implementation? Since to-accessor-array returns the same array back and we pass it to accessors.js, for complex numbers, the result has to be returned in the form of Complex128Array.

To elaborate, if we have Complex128Array, we'd need to use @stdlib/complex/real and @stdlib/complex/imag' to perform rotations on the complex numbers for each parts seperately. Something like:

	for ( i = 0; i < N; i++ ) {
		tmp = new Complex128(  (  (  c * real( get( xbuf, ix ) ) ) + ( s * real( get(ybuf, iy ) ) ) ), ( ( c * imag( get(xbuf, ix ) ) ) + ( s * imag( get(ybuf, iy ) ) ) ) );
		set( ybuf, iy, new Complex128( ( ( c * real( get(ybuf, iy ) ) ) - ( s * real( get(xbuf, ix ) ) ) ), ( ( c * imag( get(ybuf, iy ) ) ) - ( s * imag( get(xbuf, ix ) ) ) ) ) );
		set( xbuf, ix, tmp );
		ix += strideX;
		iy += strideY;
	}

But this won't work for numeric arrays converted to accessor arrays using to-accessor-array since the function would return something like

AccessorArray {
  _buffer: [ 1, 2, 3 ],
  _getter: [Function: getGeneric],
  _setter: [Function: setGeneric]
}

[kgryte]

For complex arrays, you need to conjugate and use complex number arithmetic. That will likely entail a separate internal implementation than a plain accessor array where we can assume real values.

[performant23]

Yes, but you'll want to add separate complex array tests, as the algorithm, IIRC, is different depending on whether real or complex.

oh, yeah right this is what I wanted to say

[kgryte]

bifurcated the main implementation tests into 4 cases for varying (sx, sy) as (1, 1), (2,-2), (-2, 1), and (-1, -2)

Note that the test cases in drot for these particular variants were pulled directly from the reference BLAS test suite. You'll need to tailor for complex arrays by consulting the same test suite.