feat: add lapack/base/dlarft#12972
Conversation
---
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_pkg_readmes
status: passed
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- 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
---
|
/stdlib update-copyright-years |
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_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- 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: 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: na
- 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_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- 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: 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: na
- task: lint_license_headers
status: passed
---
| // Compute T_{2,1} = V_{2,2} | ||
| for ( j = 0; j < K - l; j++ ) { | ||
| for ( i = 0; i < l; i++ ) { | ||
| T[ offsetT + ( strideT1 * ( K - l + i ) ) + ( strideT2 * j ) ] = V[ offsetV + ( strideV1 * ( N - K + j ) ) + ( strideV2 * ( K - l + i ) ) ]; |
There was a problem hiding this comment.
You have a number of repeated arithmetic operations here. I suggest finding a way to simplify and consolidate.
| // The last case is RQ. Due to how we structured this, if the above 3 are false, then RQ must be true, so we never store this RQ happens when we have backward direction in row storage `RQ = ( !dirf ) & ( !colv )` | ||
| if ( qr ) { | ||
| /* | ||
| Break V apart into 6 components |
There was a problem hiding this comment.
For these extended comments, I suggest moving to a ## Method section in the JSDoc comment above. We do this in other packages (see, e.g., https://github.com/stdlib-js/stdlib/blob/71f4c292794a8e998319756da25e5c041f2b8e64/lib/node_modules/%40stdlib/stats/strided/dpcorrwd/lib/ndarray.js).
You can then use TeX for equation and matrix markup for each of the qr, lq, etc explainers.
| function wrapper( v ) { | ||
| return addon( v ); | ||
| function isDirection( direct ) { | ||
| return contains( [ 'forward', 'backward' ] )( direct ); |
There was a problem hiding this comment.
This doesn't make sense. You are creating a new function every time you invoke this.
| * @private | ||
| * @param {boolean} v - input value | ||
| * @returns {boolean} input value | ||
| * | ||
| * @example | ||
| * var v = wrapper( true ); | ||
| * // returns true | ||
| * @param {string} direct - direction | ||
| * @returns {boolean} boolean indicating if the string is a valid direction | ||
| */ | ||
| function wrapper( v ) { | ||
| return addon( v ); | ||
| function isDirection( direct ) { | ||
| return contains( [ 'forward', 'backward' ] )( direct ); | ||
| } |
There was a problem hiding this comment.
| * @private | |
| * @param {boolean} v - input value | |
| * @returns {boolean} input value | |
| * | |
| * @example | |
| * var v = wrapper( true ); | |
| * // returns true | |
| * @param {string} direct - direction | |
| * @returns {boolean} boolean indicating if the string is a valid direction | |
| */ | |
| function wrapper( v ) { | |
| return addon( v ); | |
| function isDirection( direct ) { | |
| return contains( [ 'forward', 'backward' ] )( direct ); | |
| } | |
| * @private | |
| * @name isDirection | |
| * @type {Function} | |
| * @param {string} direct - direction | |
| * @returns {boolean} boolean indicating if the string is a valid direction | |
| */ | |
| var isDirection= contains( [ 'forward', 'backward' ] ); |
|
|
||
| var main = require( './main.js' ); | ||
| function isStorage( store ) { | ||
| return contains( [ 'columnwise', 'rowwise' ] )( store ); |
| "LDV": 3, | ||
| "V_mat": [ | ||
| [ | ||
| 1, |
There was a problem hiding this comment.
Don't split matrix rows over multiple lines like this. The entire point of the *_mat entries is to provide a visual means of seeing the "shape"/contents of an equivalent matrix in linear memory. By splitting over multiple lines, you are making the reader do a lot more work.
| [ | ||
| 1, | ||
| 0.2, | ||
| 0.3, | ||
| -0.4, | ||
| 0.5 | ||
| ], |
There was a problem hiding this comment.
| [ | |
| 1, | |
| 0.2, | |
| 0.3, | |
| -0.4, | |
| 0.5 | |
| ], | |
| [ 1, 0.2, 0.3, -0.4, 0.5 ], |
| "strideTAU": 2, | ||
| "offsetTAU": 0, | ||
| "T": [ | ||
| 0, |
There was a problem hiding this comment.
Same thing here. Arrange the elements in matrix format (rows/columns). e.g.
[ 0, 9999, 0, 9999,
0, 9999, 0, 9999,
...
]
See how much easier it is to visually inspect what the matrix is?
| "offsetTAU": 0, | ||
| "T": [ | ||
| 0, | ||
| 9999, |
There was a problem hiding this comment.
These comments apply to all of your fixtures.
---
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_pkg_readmes
status: na
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- 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: na
- task: lint_license_headers
status: passed
---
|
Made the changes as per the review. |
Description
This pull request:
lapack/base/dlarftRelated Issues
No.
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
I consulted ChatGPT for testcases to get 100% code coverage.
@stdlib-js/reviewers