feat: add lapack/base/dla-gbrpvgrw#10262
feat: add lapack/base/dla-gbrpvgrw#10262prajjwalbajpai wants to merge 5 commits intostdlib-js:developfrom
lapack/base/dla-gbrpvgrw#10262Conversation
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: passed
- task: lint_repl_help
status: passed
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
Coverage Report
The above coverage report was generated for the changes in this PR. |
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
| @@ -0,0 +1,120 @@ | |||
| {{alias}}( order, N, KL, KU, NCOLS, AB, LDAB, AFB, LDAFB ) | |||
There was a problem hiding this comment.
| {{alias}}( order, N, KL, KU, NCOLS, AB, LDAB, AFB, LDAFB ) | |
| {{alias}}( order, N, KL, KU, NCOLS, AB, LDAB, AFB, LDAFB ) |
| amax = 0.0; | ||
| umax = 0.0; | ||
|
|
||
| rowStart = max( j - KU, 0 ); // Starting index for row |
There was a problem hiding this comment.
| rowStart = max( j - KU, 0 ); // Starting index for row | |
| rowStart = max( j - KU, 0 ); // Starting index for row |
Don't align comments with tabs. Use spaces. Applies here and on the next line.
| sab1 = N; | ||
| sab2 = 1; | ||
| safb1 = N; | ||
| safb2 = 1; |
There was a problem hiding this comment.
Out of curiosity: is this the same way it is handled in LAPACKE? Just want to make sure that, when order is row-major, we simply ignore LDAB and LDAFB entirely.
There was a problem hiding this comment.
It felt off to me as well, but in LAPACK, LDAB and LDAFB both are not used in the code. Their use is only limited to strides.
* .. Executable Statements ..
*
RPVGRW = 1.0D+0
KD = KU + 1
DO J = 1, NCOLS
AMAX = 0.0D+0
UMAX = 0.0D+0
DO I = MAX( J-KU, 1 ), MIN( J+KL, N )
AMAX = MAX( ABS( AB( KD+I-J, J)), AMAX )
END DO
DO I = MAX( J-KU, 1 ), J
UMAX = MAX( ABS( AFB( KD+I-J, J ) ), UMAX )
END DO
IF ( UMAX /= 0.0D+0 ) THEN
RPVGRW = MIN( AMAX / UMAX, RPVGRW )
END IF
END DO
DLA_GBRPVGRW = RPVGRW
*
* End of DLA_GBRPVGRW| * ## Notes | ||
| * | ||
| * - Matrix `AB` is the matrix A in band storage, in rows 1 to KL+KU+1. The j-th column of A is stored in the j-th column of the matrix `AB` as `AB(KU+1+i-j,j) = A(i,j)` for max(1,j-KU)<=i<=min(N,j+kl). | ||
| * - Matrix `AFB` stores the details of the LU factorization of the band matrix A, as computed by `DGBTRF`. `U` is stored as an upper triangular band matrix with KL+KU superdiagonals in rows 1 to `KL`+`KU`+1, and the multipliers used during the factorization are stored in rows `KL`+`KU`+2 to 2*`KL`+`KU`+1. | ||
| * - The leading dimension of `AB`, `LDAB` >= KL+KU+1. | ||
| * - The leading dimension of `AFB`, `LDAFB` >= 2*KL+KU+1. | ||
| * |
There was a problem hiding this comment.
| * ## Notes | |
| * | |
| * - Matrix `AB` is the matrix A in band storage, in rows 1 to KL+KU+1. The j-th column of A is stored in the j-th column of the matrix `AB` as `AB(KU+1+i-j,j) = A(i,j)` for max(1,j-KU)<=i<=min(N,j+kl). | |
| * - Matrix `AFB` stores the details of the LU factorization of the band matrix A, as computed by `DGBTRF`. `U` is stored as an upper triangular band matrix with KL+KU superdiagonals in rows 1 to `KL`+`KU`+1, and the multipliers used during the factorization are stored in rows `KL`+`KU`+2 to 2*`KL`+`KU`+1. | |
| * - The leading dimension of `AB`, `LDAB` >= KL+KU+1. | |
| * - The leading dimension of `AFB`, `LDAFB` >= 2*KL+KU+1. | |
| * |
These notes don't need to be included in the module entry point.
| * limitations under the License. | ||
| */ | ||
|
|
||
| /* eslint-disable camelcase */ |
There was a problem hiding this comment.
Prefer disabling the precise line. So move this to L26 as a comment after the expression.
// eslint-disable-line camelcase
| * limitations under the License. | ||
| */ | ||
|
|
||
| /* eslint-disable camelcase */ |
There was a problem hiding this comment.
Same thing. Disable the exact line, rather than for the entire file.
| [ | ||
| 0, | ||
| 2, | ||
| 5, | ||
| 8 | ||
| ], | ||
| [ | ||
| 1, | ||
| 4, | ||
| 7, | ||
| 10 | ||
| ], | ||
| [ | ||
| 3, | ||
| 6, | ||
| 9, | ||
| 0 | ||
| ] |
There was a problem hiding this comment.
| [ | |
| 0, | |
| 2, | |
| 5, | |
| 8 | |
| ], | |
| [ | |
| 1, | |
| 4, | |
| 7, | |
| 10 | |
| ], | |
| [ | |
| 3, | |
| 6, | |
| 9, | |
| 0 | |
| ] | |
| [ 0, 2, 5, 8 ], | |
| [ 1, 4, 7, 10 ], | |
| [ 3, 6, 9, 0 ] |
Prefer formatting this and the other matrices as matrices in order to aid readability when examining fixture files. Applies here and throughout the fixture files in this PR.
| ] | ||
| ], | ||
| "AB": [ | ||
| 0, |
There was a problem hiding this comment.
Similarly, arrange these visually to match the matrix. Otherwise, how is a reader supposed to know when a row begins and ends? You are making the reader do a lot of work here.
| * limitations under the License. | ||
| */ | ||
|
|
||
| /* eslint-disable camelcase */ |
| * limitations under the License. | ||
| */ | ||
|
|
||
| /* eslint-disable camelcase */ |
There was a problem hiding this comment.
Same comment. Applies here and everywhere else.
| { | ||
| "name": "@stdlib/lapack/base/dla_gbrpvgrw", | ||
| "version": "0.0.0", | ||
| "description": "LAPACK routine to compute the LU factorization with complete pivoting of the general n-by-n matrix.", |
There was a problem hiding this comment.
This isn't the correct description, right?
|
|
||
| The `dla_gbrpvgrw` routine computes the reciprocal pivot growth factor for a general banded matrix `A`. The factorization has the form: | ||
|
|
||
| <!-- <equation class="equation" label="eq:rpg_factor" align="center" raw="RPGFactor = norm(A) / norm(U)" alt="Equation for calculating RPG Factor."> --> |
| <!-- <equation class="equation" label="eq:rpg_factor" align="center" raw="RPGFactor = norm(A) / norm(U)" alt="Equation for calculating RPG Factor."> --> | ||
|
|
||
| ```math | ||
| \text{RPGFactor} = \frac{\text||A||}{\text||U||} |
There was a problem hiding this comment.
This needs improving. I suggest studying TeX a bit more.
There was a problem hiding this comment.
By improving do you mean formatting or the text itself? I just showed the reciprocal pivot growth factor formula, should i include formation of AB and AFB from the matrix A for better understanding?
| var AFB = new Float64Array( [ 0.0, 0.0, 5.0, 8.0, 0.0, 4.0, 7.0, 10.0, 3.0, 6.0, 9.0, 1.8272, 0.3333, 0.1111, -0.2716, 0.0 ] ); | ||
|
|
||
| dla_gbrpvgrw( 'row-major', 4, 1, 1, 4, AB, 3, AFB, 4 ); | ||
| // 1.0 |
There was a problem hiding this comment.
This isn't how we do returns annotations.
| var AFB = new Float64Array( AFB0.buffer, AFB0.BYTES_PER_ELEMENT*1 ); // start at 2nd element | ||
|
|
||
| dla_gbrpvgrw( 'row-major', 4, 1, 1, 4, AB, 3, AFB, 4 ); | ||
| // 1.0 |
There was a problem hiding this comment.
Same comment here and elsewhere.
|
|
||
| ## Notes | ||
|
|
||
| - Matrix `AB` is the matrix A in band storage, in rows 1 to KL+KU+1. The j-th column of A is stored in the j-th column of the matrix `AB` as `AB(KU+1+i-j,j) = A(i,j)` for max(1,j-KU)<=i<=min(N,j+kl). |
There was a problem hiding this comment.
You need to ensure everything is from the viewpoint of 0-based indexing, not 1-based indexing. Applies here and throughout this PR.
| * ## Notes | ||
| * | ||
| * - Matrix `AB` is the matrix A in band storage, in rows 1 to KL+KU+1. The j-th column of A is stored in the j-th column of the matrix `AB` as `AB(KU+1+i-j,j) = A(i,j)` for max(1,j-KU)<=i<=min(N,j+kl). | ||
| * - Matrix `AFB` stores the details of the LU factorization of the band matrix A, as computed by `DGBTRF`. `U` is stored as an upper triangular band matrix with KL+KU superdiagonals in rows 1 to `KL`+`KU`+1, and the multipliers used during the factorization are stored in rows `KL`+`KU`+2 to 2*`KL`+`KU`+1. |
There was a problem hiding this comment.
Your comments mix tabs and spaces. In Markdown (including Markdown in JSDoc), always use spaces. Applies here and elsewhere throughout this PR.
| * | ||
| * @example | ||
| * var Float64Array = require( '@stdlib/array/float64' ); | ||
| * var Int32Array = require( '@stdlib/array/int32' ); |
There was a problem hiding this comment.
We are you importing Int32Array here. You are not using it.
| * var AFB = new Float64Array( [ 0.0, 0.0, 5.0, 8.0, 0.0, 4.0, 7.0, 10.0, 3.0, 6.0, 9.0, 1.8272, 0.3333, 0.1111, -0.2716, 0.0 ] ); | ||
| * | ||
| * dla_gbrpvgrw( 'row-major', 4, 1, 1, 4, AB, 3, AFB, 4 ); | ||
| * // 1.0 |
There was a problem hiding this comment.
This isn't how we do returns annotations. Applies here and elsewhere in this PR.
| * - Matrix `AB` is the matrix A in band storage, in rows 1 to KL+KU+1. The j-th column of A is stored in the j-th column of the matrix `AB` as `AB(KU+1+i-j,j) = A(i,j)` for max(1,j-KU)<=i<=min(N,j+kl). | ||
| * - Matrix `AFB` stores the details of the LU factorization of the band matrix A, as computed by `DGBTRF`. `U` is stored as an upper triangular band matrix with KL+KU superdiagonals in rows 1 to `KL`+`KU`+1, and the multipliers used during the factorization are stored in rows `KL`+`KU`+2 to 2*`KL`+`KU`+1. | ||
| * - The leading dimension of `AB`, `LDAB` >= KL+KU+1. | ||
| * - The leading dimension of `AFB`, `LDAFB` >= 2*KL+KU+1. |
| * | ||
| * @example | ||
| * var Float64Array = require( '@stdlib/array/float64' ); | ||
| * var Int32Array = require( '@stdlib/array/int32' ); |
There was a problem hiding this comment.
Same thing. You are not using this import here and elsewhere in this PR.
| 'dtype': 'float64' | ||
| }; | ||
|
|
||
| // Random input matrix; |
There was a problem hiding this comment.
| // Random input matrix; |
| }; | ||
|
|
||
| // Random input matrix; | ||
| AFB = uniform( 4*N*N, -1.0, 1.0, opts ); |
There was a problem hiding this comment.
Don't we need to be creating random banded matrices?
There was a problem hiding this comment.
Generating AFB from a banded matrix a requires routine DGBTRF. Since, it hasn't been implemented yet, I decided to write benchmarks like this. Maybe you can suggest a better way.
!> AFB is DOUBLE PRECISION array, dimension (LDAFB,N)
!> Details of the LU factorization of the band matrix A, as
!> computed by DGBTRF. U is stored as an upper triangular
!> band matrix with KL+KU superdiagonals in rows 1 to KL+KU+1,
!> and the multipliers used during the factorization are stored
!> in rows KL+KU+2 to 2*KL+KU+1.
| for ( i = min; i <= max; i++ ) { | ||
| N = floor( pow( pow( 10, i ), 1.0/2.0 ) ); | ||
| f = createBenchmark( ord, N ); | ||
| bench( format( '%s:order=%s,size=%d', pkg, ord, (N*N) ), f ); |
There was a problem hiding this comment.
| bench( format( '%s:order=%s,size=%d', pkg, ord, (N*N) ), f ); | |
| bench( format( '%s:order=%s,size=%d', pkg, ord, N*N ), f ); |
Avoid unnecessary parentheses.
| for ( i = min; i <= max; i++ ) { | ||
| N = floor( pow( pow( 10, i ), 1.0/2.0 ) ); | ||
| f = createBenchmark( ord, N ); | ||
| bench( format( '%s:ndarray:order=%s,size=%d', pkg, ord, (N*N) ), f ); |
There was a problem hiding this comment.
| bench( format( '%s:ndarray:order=%s,size=%d', pkg, ord, (N*N) ), f ); | |
| bench( format( '%s:ndarray:order=%s,size=%d', pkg, ord, N*N ), f ); |
| var AB = new Float64Array( [ 0.0, | ||
| 2.0, | ||
| 5.0, | ||
| 8.0, | ||
| 1.0, | ||
| 4.0, | ||
| 7.0, | ||
| 10.0, | ||
| 3.0, | ||
| 6.0, | ||
| 9.0, | ||
| 0.0 ] ); |
There was a problem hiding this comment.
| var AB = new Float64Array( [ 0.0, | |
| 2.0, | |
| 5.0, | |
| 8.0, | |
| 1.0, | |
| 4.0, | |
| 7.0, | |
| 10.0, | |
| 3.0, | |
| 6.0, | |
| 9.0, | |
| 0.0 ] ); | |
| var AB = new Float64Array([ | |
| 0.0, | |
| 2.0, | |
| 5.0, | |
| 8.0, | |
| 1.0, | |
| 4.0, | |
| 7.0, | |
| 10.0, | |
| 3.0, | |
| 6.0, | |
| 9.0, | |
| 0.0 | |
| ]); |
Not sure why you are formatting this way, as deviates from throughout stdlib. Applies here and elsewhere throughout this PR.
| * limitations under the License. | ||
| */ | ||
|
|
||
| /* eslint-disable camelcase */ |
| * limitations under the License. | ||
| */ | ||
|
|
||
| /* eslint-disable camelcase */ |
| * ## Notes | ||
| * | ||
| * - Matrix `AB` is the matrix A in band storage, in rows 1 to KL+KU+1. The j-th column of A is stored in the j-th column of the matrix `AB` as `AB(KU+1+i-j,j) = A(i,j)` for max(1,j-KU)<=i<=min(N,j+kl). | ||
| * - Matrix `AFB` stores the details of the LU factorization of the band matrix A, as computed by `DGBTRF`. `U` is stored as an upper triangular band matrix with KL+KU superdiagonals in rows 1 to `KL`+`KU`+1, and the multipliers used during the factorization are stored in rows `KL`+`KU`+2 to 2*`KL`+`KU`+1. |
There was a problem hiding this comment.
| * - Matrix `AFB` stores the details of the LU factorization of the band matrix A, as computed by `DGBTRF`. `U` is stored as an upper triangular band matrix with KL+KU superdiagonals in rows 1 to `KL`+`KU`+1, and the multipliers used during the factorization are stored in rows `KL`+`KU`+2 to 2*`KL`+`KU`+1. | |
| * - Matrix `AFB` stores the details of the LU factorization of the band matrix A, as computed by `DGBTRF`. `U` is stored as an upper triangular band matrix with KL+KU superdiagonals in rows 1 to `KL+KU+1`, and the multipliers used during the factorization are stored in rows `KL+KU+2` to `2*KL+KU+1`. |
Don't split code syntax like this. Write it like you would Markdown, and, in Markdown, we would not write a + b. We'd write a+b.
There was a problem hiding this comment.
Applies here and everywhere else in this PR.
kgryte
left a comment
There was a problem hiding this comment.
Left initial comments. This PR needs quite a bit of clean-up before it is ready for another review.
|
Another thing. This package should be renamed to |
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: passed
- task: lint_repl_help
status: passed
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: passed
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
lapack/base/dla_gbrpvgrwlapack/base/dla-gbrpvgrw
Description
This pull request:
lapack/base/dla-gbrpvgrwRelated Issues
No.
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
I used ChatGPT to generate the test cases, and ran the testcases in fortran to confirm the output.
@stdlib-js/reviewers