feat: add lapack/base/dlaset
#2558
Conversation
Pranavchiku
added
the
LAPACK
| // A0 => <Float64Array>[ 0.0, 3.7, 0.0, 0.0, 3.1, 3.7, 0.0, 3.1, 3.1, 3.7 ] | ||
| ``` | ||
|
|
||
| #### dlaset.ndarray( order, uplo, M, N, alpha, beta, A, LDA, offsetA ) |
There was a problem hiding this comment.
@Pranavchiku Would you minding updating the implementation to support separate dimension strides, as discussed elsewhere?
There was a problem hiding this comment.
On it, thus marking it as draft.
| var i; | ||
| var j; | ||
|
|
||
| if ( uplo === 'upper' ) { |
There was a problem hiding this comment.
I suspect that it may be possible to perform loop interchange in this implementation, which may allow consolidating loops.
There was a problem hiding this comment.
There should be an equivalence relation between, e.g., upper and column-major and lower and row-major. You should verify. But if true, you should be able to reduce nested loop duplication.
| return A; | ||
| } | ||
| // All of the matrix `A` | ||
| if ( order === 'column-major' ) { |
There was a problem hiding this comment.
These loops should definitely support loop interchange, thus allowing consolidation into a single set of nested loops.
There was a problem hiding this comment.
@Pranavchiku You can have a look at the most recent changes to the dlacpy PR. #2548
There was a problem hiding this comment.
As in that PR, I suggest splitting this implementation up into separate sub-functions.
kgryte
added
the
Needs Changes
Pranavchiku
added
Needs Review
kgryte
added
Needs Changes
Pranavchiku
added
Needs Review
kgryte
added
Needs Changes
There was a problem hiding this comment.
@Pranavchiku It is not clear to me (a) why you did not leverage loop interchange in the implementations below and (b) why you are not accounting for row- and column-major stride order. E.g., if strides are in column-major and need to set the upper triangular part, I would expect that the loop would be different than if the strides are in row-major and need to set the upper triangular part. Currently, your implementations favor one order over the other.
Additionally, when M == N, you can have "optimized" implementations where column-major strides upper is equivalent to row-major strides lower. I would expect that we'd include such implementations here.
There was a problem hiding this comment.
Additionally, when M == N, you can have "optimized" implementations where column-major strides upper is equivalent to row-major strides lower. I would expect that we'd include such implementations here.
The trouble here is that now we need to take a transpose of output matrix before returning.
There was a problem hiding this comment.
Or let me check again
| for ( i = 0; i < M; i++ ) { | ||
| ia = offsetA + ( i * strideA1 ); | ||
| for ( j = i + 1; j < min( M, N ); j++ ) { | ||
| if ( i !== j ) { |
There was a problem hiding this comment.
@Pranavchiku Given that j = i + 1, in what circumstance is i equal to j?
There was a problem hiding this comment.
Interesting, it fails without that check.
| if ( isRowMajor( [ strideA1, strideA2 ] ) ) { | ||
| for ( i = 0; i < M; i++ ) { | ||
| ia = offsetA + ( i * strideA1 ); | ||
| for ( j = i; j >= 0; j-- ) { |
There was a problem hiding this comment.
If we're setting the diagonal at L150, why initialize j = i here?
Description
This pull request adds JS implementation for
lapack/base/dlasetRelated Issues
No.
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers