Conversation
|
Hello! Thank you for your contribution to stdlib. We noticed that the contributing guidelines acknowledgment is missing from your pull request. Here's what you need to do:
This acknowledgment confirms that you've read the guidelines, which include:
We can't review or accept contributions without this acknowledgment. Thank you for your understanding and cooperation. We look forward to reviewing your contribution! |
| if ( trans === 'conjugate-transpose' ) { | ||
| aij = conjf( aij ); | ||
| } | ||
| y.set( caddf( y.get( iy ), cmulf( aij, tmp ) ), iy ); |
There was a problem hiding this comment.
Computing this way is not what we want. See zaxpy for an implementation which avoids complex instance materialization. caxpy still needs to be updated to use @stdlib/complex/float32/base/mul-add. You need to reinterpret as a real-valued strided array and then pass values to muladd.
There was a problem hiding this comment.
Addressed in c09f790
All operations now use reinterpreted real/imag views. No complex instances are created inside hot loops, and accumulation uses muladd.assign.
| iy = offsetY; | ||
| for ( i0 = 0; i0 < ylen; i0++ ) { | ||
| aij = A.get( ia ); | ||
| if ( trans === 'conjugate-transpose' ) { |
There was a problem hiding this comment.
Try to avoid burying conditionals within hot loops. If you need to duplicate loops, so be it.
There was a problem hiding this comment.
Addressed in c09f790
Transpose/conjugation logic is resolved before entering inner loops. No conditionals remain inside performance-critical loops.
| } else { | ||
| iy = offsetY; | ||
| for ( i0 = 0; i0 < ylen; i0++ ) { | ||
| aij = A.get( ia ); |
There was a problem hiding this comment.
Avoid materializing complex number instances. Pull directly real and imaginary components are real-valued reinterpretation, as in zaxpy.
There was a problem hiding this comment.
Complex values are handled via scalar real/imag components from reinterpreted views.
| ia = offsetA; | ||
| ix = offsetX; | ||
| for ( i1 = 0; i1 < xlen; i1++ ) { | ||
| tmp = cmulf( alpha, x.get( ix ) ); |
There was a problem hiding this comment.
Before entering this loop, you need to decompose alpha and beta into real and imaginary components.
There was a problem hiding this comment.
You'll likely want tmp to be Float32Array workspace to which you write into via cmul.assign (or similar). This wouldn't be threadsafe in C, but in JS, it is fine.
There was a problem hiding this comment.
alpha and beta are decomposed into real and imaginary parts prior to iteration.
| ix = offsetX; | ||
| for ( i1 = 0; i1 < xlen; i1++ ) { | ||
| tmp = cmulf( alpha, x.get( ix ) ); | ||
| if ( scabs1( tmp ) === 0.0 ) { |
There was a problem hiding this comment.
When you do, this line will change. When implementing complex number routines in JS, we do not exactly follow Fortran logic. Why? Because JS does not have built-in support for complex numbers.
There was a problem hiding this comment.
Implementation now follows stdlib’s explicit real/imag arithmetic pattern rather than Fortran-style logic.
| ylen = M; | ||
| } | ||
| // y = beta*y | ||
| if ( scabs1( beta ) === 0.0 ) { |
There was a problem hiding this comment.
Yes, scabs1 will work here, but I am not sure why we want to expend the extra effort. If you decompose alpha and beta into their respective components, then you can test the components directly.
There was a problem hiding this comment.
This comment applies here and elsewhere.
There was a problem hiding this comment.
Zero checks now test scalar components directly.
| * cgemv( 'no-transpose', 2, 3, alpha, A, 3, 1, 0, x, 1, 0, beta, y, 1, 0 ); | ||
| * // y => <Complex64Array>[ 7.0, 0.0, 16.0, 0.0 ] | ||
| */ | ||
| function cgemv(trans, M, N, alpha, A, strideA1, strideA2, offsetA, x, strideX, offsetX, beta, y, strideY, offsetY) { // eslint-disable-line max-params, max-len |
There was a problem hiding this comment.
Much of your spacing here and below is incorrect and deviates from stdlib conventions.
There was a problem hiding this comment.
Formatting updated to match stdlib conventions.
Coverage Report
The above coverage report was generated for the changes in this PR. |
|
Hi @kgryte , I’ve implemented the requested changes:
I’ve also updated and expanded the test cases to ensure full coverage. The numerical results have been cross-verified against SciPy. Please let me know if there are any remaining performance or style concerns you’d like addressed.
|
|
While testing , I noticed that when using a negative strideY, the resulting values appear reversed in the underlying buffer compared to the expected logical order. (eg: Numerically the results are correct, but the physical layout differs when strideY < 0. I wanted to confirm is this behavior expected under the current BLAS semantics ? |
| // Standard usage: | ||
| > var x = new {{alias:@stdlib/array/complex64}}([ 1.0, 1.0, 2.0, 2.0 ]); | ||
| > var y = new {{alias:@stdlib/array/complex64}}([ 1.0, 1.0, 2.0, 2.0 ]); | ||
| > var A = new {{alias:@stdlib/array/complex64}}([ 1.0, 1.0, 2.0, 2.0, 3.0, 3.0, 4.0, 4.0 ]); |
There was a problem hiding this comment.
| > var A = new {{alias:@stdlib/array/complex64}}([ 1.0, 1.0, 2.0, 2.0, 3.0, 3.0, 4.0, 4.0 ]); | |
| > var buf = [ 1.0, 1.0, 2.0, 2.0, 3.0, 3.0, 4.0, 4.0 ]; | |
| > var A = new {{alias:@stdlib/array/complex64}}( buf ); |
| > var x = new {{alias:@stdlib/array/complex64}}([ 2.0, 2.0, 1.0, 1.0 ]); | ||
| > var y = new {{alias:@stdlib/array/complex64}}([ 2.0, 2.0, 1.0, 1.0 ]); | ||
| > var A = new {{alias:@stdlib/array/complex64}}([ 1.0, 1.0, 2.0, 2.0, 3.0, 3.0, 4.0, 4.0 ]); | ||
| > var alpha = new {{alias:@stdlib/complex/float32/ctor}}( 0.5, 0.5 ); | ||
| > var beta = new {{alias:@stdlib/complex/float32/ctor}}( 0.5, -0.5 ); | ||
| > var ord = 'column-major'; | ||
| > var trans = 'no-transpose'; |
There was a problem hiding this comment.
| > var x = new {{alias:@stdlib/array/complex64}}([ 2.0, 2.0, 1.0, 1.0 ]); | |
| > var y = new {{alias:@stdlib/array/complex64}}([ 2.0, 2.0, 1.0, 1.0 ]); | |
| > var A = new {{alias:@stdlib/array/complex64}}([ 1.0, 1.0, 2.0, 2.0, 3.0, 3.0, 4.0, 4.0 ]); | |
| > var alpha = new {{alias:@stdlib/complex/float32/ctor}}( 0.5, 0.5 ); | |
| > var beta = new {{alias:@stdlib/complex/float32/ctor}}( 0.5, -0.5 ); | |
| > var ord = 'column-major'; | |
| > var trans = 'no-transpose'; | |
| > x = new {{alias:@stdlib/array/complex64}}([ 2.0, 2.0, 1.0, 1.0 ]); | |
| > y = new {{alias:@stdlib/array/complex64}}([ 2.0, 2.0, 1.0, 1.0 ]); | |
| > A = new {{alias:@stdlib/array/complex64}}([ 1.0, 1.0, 2.0, 2.0, 3.0, 3.0, 4.0, 4.0 ]); | |
| > alpha = new {{alias:@stdlib/complex/float32/ctor}}( 0.5, 0.5 ); | |
| > beta = new {{alias:@stdlib/complex/float32/ctor}}( 0.5, -0.5 ); | |
| > ord = 'column-major'; | |
| > trans = 'no-transpose'; |
| Performs one of the matrix-vector operations | ||
| `y = α*A*x + β*y`, | ||
| `y = α*A^T*x + β*y`, | ||
| or `y = α*A^H*x + β*y` | ||
| using alternative indexing semantics | ||
| and where `α` and `β` are complex scalars, | ||
| `x` and `y` are complex vectors, | ||
| and `A` is an `M` by `N` complex matrix. | ||
|
|
||
| While typed array views mandate a view offset | ||
| based on the underlying buffer, | ||
| the offset parameters support indexing semantics | ||
| based on starting indices. |
There was a problem hiding this comment.
This all needs to be wrapped at 80 characters. See https://github.com/stdlib-js/stdlib/blob/develop/docs/contributing/repl_text.md
f4efdb0 to
ff420ae
Compare

Progresses #2039.
Description
This pull request:
cgemvfor ndarray inputs.Related 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
{{TODO: add disclosure if applicable}}
@stdlib-js/reviewers