-
-
Notifications
You must be signed in to change notification settings - Fork 447
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
[RFC]: add blas/base/grot
#2190
Comments
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 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();
});
This means that basically when the arguments are passed onto the function, it isn't calling the implementation in I checked other implementations ( So, could you please guide me on calling and testing the accessor array implementation? Thanks! |
@performant23 That's a bit odd, as when I run the tests for
|
I'd need to see your complete test file to be able to debug. |
Yeah I can open up a draft PR for this! |
Hmm...can you tell me about bit more about how you are running the tests? Also, in the stdlib REPL, run
|
So, I used the usual test filter i.e. Will get back to you regarding the REPL run. |
Also, what version of Node.js are you running? |
Whoa. That is bizarre. |
In the REPL, do
|
Yeah, this test gives a positive:
|
Okay, that is weird. As |
Can you also run from the terminal
|
|
yeah, sure! |
In the source code of |
It logged (additionally, dt is:
|
Also, logging the same test value for var x = new Complex64Array( 10 );
console.log(isAccessorArray(x)); Additionally,
|
Wait...why are you logging |
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 |
Yes, that would probably be best. |
You use convert a normal array to an accessor array using |
So, the current tests for (Hope you don't mind me being pedantic :), just wanted to keep things as clear as possible) |
Yes, but you'll want to add separate complex array tests, as the algorithm, IIRC, is different depending on whether real or complex. |
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 But what if To elaborate, if we have Complex128Array, we'd need to use 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
|
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. |
oh, yeah right this is what I wanted to say |
Note that the test cases in |
Right, so we'd need to modify our |
That's right. An example of the idea:
|
Ah, got it! That implementation makes things really clear now! |
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
RFC:
.The text was updated successfully, but these errors were encountered: