Skip to content

fix: address off-by-one indexing bug #13436

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.

Sign up for GitHub

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

Merged
kgryte merged 3 commits into from
Jul 14, 2026
Merged

fix: address off-by-one indexing bug #13436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
06ee33c
fix: indexing issue, make the representation of lastv consistent with…
aayush0325 Jul 12, 2026
1afb143
Apply suggestions from code review
kgryte Jul 12, 2026
4277e81
fix: add test case that fails on incorrect indexing and passes on the…
aayush0325 Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions lib/node_modules/@stdlib/lapack/base/dlarf1f/lib/base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,17 @@ function dlarf1f( side, M, N, V, strideV, offsetV, tau, C, strideC1, strideC2, o
i = offsetV + ( ( lastv-1 ) * strideV );

// Move `i` to the last non-zero element in `V`, where we assume that V[0] = 1, and it is not stored, so we shouldn't access it...
while ( lastv > 0 && V[ i ] === 0.0 ) {
while ( lastv > 1 && V[ i ] === 0.0 ) { // `lastv` should be greater than 1 as it represents the last one-based index with a zero value
lastv -= 1;
i -= strideV;
}

if ( isLeftSide( side ) ) {
// Scan for the last non-zero column in `C`:
lastc = iladlc( lastv + 1, N, C, strideC1, strideC2, offsetC ) + 1; // adjust by `+1` to account for the difference between zero-based and one-based indexing
lastc = iladlc( lastv, N, C, strideC1, strideC2, offsetC ) + 1; // adjust by `+1` to account for the difference between zero-based and one-based indexing
} else {
// Scan for the last non-zero row in `C`:
lastc = iladlr( M, lastv + 1, C, strideC1, strideC2, offsetC ) + 1; // // adjust by `+1` to account for the difference between zero-based and one-based indexing
lastc = iladlr( M, lastv, C, strideC1, strideC2, offsetC ) + 1; // // adjust by `+1` to account for the difference between zero-based and one-based indexing
}
// Return `C` unchanged if all elements in `C` are zero...
if ( lastc === 0 ) {
Expand All @@ -127,7 +128,7 @@ function dlarf1f( side, M, N, V, strideV, offsetV, tau, C, strideC1, strideC2, o
// Form: H*C

// If `lastv = 1`, this means `V = 1`, so we just need to compute `C = H*C = (1-tau)*C`...
if ( lastv === 0 ) {
if ( lastv === 1 ) {
// C[0,0:lastc] = (1-tau)*C[0,0:lastc]
dscal( lastc, 1.0-tau, C, strideC2, offsetC ); // scale the first row
} else {
Expand All @@ -154,7 +155,7 @@ function dlarf1f( side, M, N, V, strideV, offsetV, tau, C, strideC1, strideC2, o
// Form: C*H

// If `N = 1`, then `V = 1`, so we just need to compute `C = CH = C*(1-tau)`...
if ( lastv === 0 ) {
if ( lastv === 1 ) {
// C[0:lastc,0] = ( 1-tau ) * C[0:lastc,0]
dscal( lastc, 1.0-tau, C, strideC1, offsetC ); // scale the first column
return C;
Expand Down
36 changes: 36 additions & 0 deletions lib/node_modules/@stdlib/lapack/base/dlarf1f/test/fixtures/indexing.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"order": "column-major",

"side": "left",

"M": 3,
"N": 1,

"V": [ 1, 0, 0, 0, -1, 0 ],
"strideV": 1,
"offsetV": 0,

"tau": 2,

"C": [ 1, 0, 0, 0, -1, 0 ],
"strideC1": 1,
"strideC2": 3,
"offsetC": 0,
"LDC": 3,
"C_mat": [
[ 1.0, 0.0 ],
[ 0.0, -1.0 ],
[ 0.0, 0.0 ]
],

"work": [ 64, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0 ],
"strideWork": 1,
"offsetWork": 0,

"C_out": [ -1, 0, 0, -0, -1, -0 ],
"C_out_mat": [
[ -1.0, 0.0 ],
[ 0.0, -1.0 ],
[ 0.0, 0.0 ]
]
}
22 changes: 22 additions & 0 deletions lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
var RIGHT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/right_lastv_eq_zero_col_major.json' );
var RIGHT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/right_lastv_gt_zero_row_major.json' );
var RIGHT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/right_lastv_gt_zero_col_major.json' );
var INDEXING = require( './fixtures/indexing.json' );


// TESTS //
Expand Down Expand Up @@ -200,7 +201,7 @@
t.end();
});

tape( 'the function applies an elementary refactor (side=left, lastv=0, row-major)', function test( t ) {

Check warning on line 204 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -221,7 +222,7 @@
t.end();
});

tape( 'the function applies an elementary refactor (side=left, lastv=0, column-major)', function test( t ) {

Check warning on line 225 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -242,7 +243,7 @@
t.end();
});

tape( 'the function applies an elementary refactor (side=left, lastv > 0, row-major)', function test( t ) {

Check warning on line 246 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -263,7 +264,7 @@
t.end();
});

tape( 'the function applies an elementary refactor (side=left, lastv > 0, column-major)', function test( t ) {

Check warning on line 267 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -284,7 +285,7 @@
t.end();
});

tape( 'the function applies an elementary refactor (side=right, lastv=0, row-major)', function test( t ) {

Check warning on line 288 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -305,7 +306,7 @@
t.end();
});

tape( 'the function applies an elementary refactor (side=right, lastv=0, column-major)', function test( t ) {

Check warning on line 309 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -326,7 +327,7 @@
t.end();
});

tape( 'the function applies an elementary refactor (side=right, lastv > 0, row-major)', function test( t ) {

Check warning on line 330 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -347,7 +348,7 @@
t.end();
});

tape( 'the function applies an elementary refactor (side=right, lastv > 0, column-major)', function test( t ) {

Check warning on line 351 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.dlarf1f.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -367,3 +368,24 @@
t.deepEqual( out, expectedC, 'returns expected value' );
t.end();
});

tape( 'the function follows consistent indexing with LAPACK', function test( t ) {
var expectedC;
var data;
var work;
var out;
var V;
var C;

data = INDEXING;

V = new Float64Array( data.V );
C = new Float64Array( data.C );
work = new Float64Array( data.work );

out = dlarf1f( data.order, data.side, data.M, data.N, V, data.strideV, data.tau, C, data.LDC, work );
expectedC = new Float64Array( data.C_out );

t.deepEqual( out, expectedC, 'returns expected value' );
t.end();
});
30 changes: 22 additions & 8 deletions lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.ndarray.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,46 +38,39 @@
var RIGHT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/right_lastv_eq_zero_col_major.json' );
var RIGHT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/right_lastv_gt_zero_row_major.json' );
var RIGHT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/right_lastv_gt_zero_col_major.json' );

var MIXED_STRIDES_RIGHT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/mixed_strides/right_lastv_gt_zero_row_major.json' );
var NEGATIVE_STRIDES_RIGHT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/negative_strides/right_lastv_gt_zero_row_major.json' );
var LARGE_STRIDES_RIGHT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/large_strides/right_lastv_gt_zero_row_major.json' );
var OFFSETS_RIGHT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/offsets/right_lastv_gt_zero_row_major.json' );

var NEGATIVE_STRIDES_RIGHT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/negative_strides/right_lastv_gt_zero_col_major.json' );
var MIXED_STRIDES_RIGHT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/mixed_strides/right_lastv_gt_zero_col_major.json' );
var LARGE_STRIDES_RIGHT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/large_strides/right_lastv_gt_zero_col_major.json' );
var OFFSETS_RIGHT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/offsets/right_lastv_gt_zero_col_major.json' );

var MIXED_STRIDES_RIGHT_LASTV_EQ_ZERO_ROW_MAJOR = require( './fixtures/mixed_strides/right_lastv_eq_zero_row_major.json' );
var NEGATIVE_STRIDES_RIGHT_LASTV_EQ_ZERO_ROW_MAJOR = require( './fixtures/negative_strides/right_lastv_eq_zero_row_major.json' );
var LARGE_STRIDES_RIGHT_LASTV_EQ_ZERO_ROW_MAJOR = require( './fixtures/large_strides/right_lastv_eq_zero_row_major.json' );
var OFFSETS_RIGHT_LASTV_EQ_ZERO_ROW_MAJOR = require( './fixtures/offsets/right_lastv_eq_zero_row_major.json' );

var NEGATIVE_STRIDES_RIGHT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/negative_strides/right_lastv_eq_zero_col_major.json' );
var MIXED_STRIDES_RIGHT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/mixed_strides/right_lastv_eq_zero_col_major.json' );
var LARGE_STRIDES_RIGHT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/large_strides/right_lastv_eq_zero_col_major.json' );
var OFFSETS_RIGHT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/offsets/right_lastv_eq_zero_col_major.json' );

var NEGATIVE_STRIDES_LEFT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/negative_strides/left_lastv_gt_zero_row_major.json' );
var MIXED_STRIDES_LEFT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/mixed_strides/left_lastv_gt_zero_row_major.json' );
var LARGE_STRIDES_LEFT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/large_strides/left_lastv_gt_zero_row_major.json' );
var OFFSETS_LEFT_LASTV_GT_ZERO_ROW_MAJOR = require( './fixtures/offsets/left_lastv_gt_zero_row_major.json' );

var MIXED_STRIDES_LEFT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/mixed_strides/left_lastv_gt_zero_col_major.json' );
var LARGE_STRIDES_LEFT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/large_strides/left_lastv_gt_zero_col_major.json' );
var OFFSETS_LEFT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/offsets/left_lastv_gt_zero_col_major.json' );
var NEGATIVE_STRIDES_LEFT_LASTV_GT_ZERO_COL_MAJOR = require( './fixtures/negative_strides/left_lastv_gt_zero_col_major.json' );

var MIXED_STRIDES_LEFT_LASTV_EQ_ZERO_ROW_MAJOR = require( './fixtures/mixed_strides/left_lastv_eq_zero_row_major.json' );
var LARGE_STRIDES_LEFT_LASTV_EQ_ZERO_ROW_MAJOR = require( './fixtures/large_strides/left_lastv_eq_zero_row_major.json' );
var OFFSETS_LEFT_LASTV_EQ_ZERO_ROW_MAJOR = require( './fixtures/offsets/left_lastv_eq_zero_row_major.json' );
var NEGATIVE_STRIDES_LEFT_LASTV_EQ_ZERO_ROW_MAJOR = require( './fixtures/negative_strides/left_lastv_eq_zero_row_major.json' );

var MIXED_STRIDES_LEFT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/mixed_strides/left_lastv_eq_zero_col_major.json' );
var LARGE_STRIDES_LEFT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/large_strides/left_lastv_eq_zero_col_major.json' );
var OFFSETS_LEFT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/offsets/left_lastv_eq_zero_col_major.json' );
var NEGATIVE_STRIDES_LEFT_LASTV_EQ_ZERO_COL_MAJOR = require( './fixtures/negative_strides/left_lastv_eq_zero_col_major.json' );
var INDEXING = require( './fixtures/indexing.json' );


// TESTS //
Expand Down Expand Up @@ -172,7 +165,7 @@
t.end();
});

tape( 'the function applies an elementary reflector (side=left, lastv = 0, row-major)', function test( t ) {

Check warning on line 168 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.ndarray.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand All @@ -193,7 +186,7 @@
t.end();
});

tape( 'the function applies an elementary reflector (side=left, lastv = 0, column-major)', function test( t ) {

Check warning on line 189 in lib/node_modules/@stdlib/lapack/base/dlarf1f/test/test.ndarray.js

View workflow job for this annotation

GitHub Actions / Lint Changed Files 

Unknown word: "lastv"
var expectedC;
var data;
var work;
Expand Down Expand Up @@ -340,6 +333,27 @@
t.end();
});

tape( 'the function follows indexing consistent with LAPACK', function test( t ) {
var expectedC;
var data;
var work;
var out;
var V;
var C;

data = INDEXING;

V = new Float64Array( data.V );
C = new Float64Array( data.C );
work = new Float64Array( data.work );

out = dlarf1f( data.side, data.M, data.N, V, data.strideV, data.offsetV, data.tau, C, data.strideC1, data.strideC2, data.offsetC, work, data.strideWork, data.offsetWork );
expectedC = new Float64Array( data.C_out );

t.deepEqual( out, expectedC, 'returns expected value' );
t.end();
});

tape( 'the function applies an elementary reflector (side=right, lastv > 0, row-major, mixed strides)', function test( t ) {
var expectedC;
var data;
Expand Down