-
-
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
feat: add lapack/base/iladlr
#2854
base: develop
Are you sure you want to change the base?
Conversation
var out; | ||
var A = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | ||
|
||
out = iladlr( 'row-major', 2, 2, A, 2 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var out; | |
var A = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | |
out = iladlr( 'row-major', 2, 2, A, 2 ); | |
var A = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | |
var out = iladlr( 'row-major', 2, 2, A, 2 ); |
This needs to be fixed here and in other examples.
```javascript | ||
var Float64Array = require( '@stdlib/array/float64' ); | ||
|
||
// Initial an array. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Initial an array. | |
// Initial an array: |
Nowhere do we end line comments with only a period. Needs to be fixed here and elsewhere.
var A1 = new Float64Array( A0.buffer, A0.BYTES_PER_ELEMENT*1 ); // start at 2nd element | ||
|
||
var out = iladlr( 'row-major', 2, 2, A1, 2 ); | ||
// out => 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't how we do return annotations for scalar values.
var A = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | ||
var out = iladlr( 'row-major', 2, 2, A, 2 ); | ||
console.log( out ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example isn't doing anything "new". Perhaps, instead, use random/array/bernoulli
with a low probability to generate almost empty arrays.
var A = new Float64Array( [ 4.0, 3.0, 2.0, 4.0 ] ); | ||
|
||
out = iladlr.ndarray( 2, 2, A, -2, -1, 3 ); | ||
// returns 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your examples all do the same thing. Here it is not obvious how the change in stride and offset have any bearing on the output.
In your examples, you should strive to make cause and effect more obvious.
* var out; | ||
* var A = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | ||
* | ||
* out = iladlr( 2, 2, A, 2, 1, 0 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* var out; | |
* var A = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | |
* | |
* out = iladlr( 2, 2, A, 2, 1, 0 ); | |
* var A = new Float64Array( [ 1.0, 2.0, 3.0, 4.0 ] ); | |
* | |
* var out = iladlr( 2, 2, A, 2, 1, 0 ); |
Here and everywhere.
if ( A[ offsetA + ( ( M - 1 ) * sa0 ) ] !== 0.0 || A[ offsetA + ( ( M - 1 ) * sa0 ) + ( ( N - 1 ) * sa1 ) ] ) { | ||
return M - 1; | ||
} | ||
// Scan up each column tracking the last zero row seen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Scan up each column tracking the last zero row seen. | |
// Scan up each column tracking the last zero row seen... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, for row-major matrices, this comment doesn't make much sense. I would argue that row-major matrices deserve their own set of nested loops, as the column-major algorithm is not efficient. Just think about the problem. You want to find a row with at least one non-zero element. That should be trivial to implement in row-major.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accordingly, I would refactor and remove the attempt at loop interchange.
var i; | ||
var j; | ||
|
||
if ( isRowMajor( [ strideA1, strideA2 ] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the goal is loop interchange, you need to also swap dimensions.
Towards #2464.
Description
This pull request adds JS implementation for
lapack/base/iladlr
.Related Issues
NA
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers