feat: add blas/ext/base/gmskrev#10925
Conversation
Coverage Report
The above coverage report was generated for the changes in this PR. |
Co-authored-by: Athan <kgryte@gmail.com> Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Co-authored-by: Athan <kgryte@gmail.com> Signed-off-by: Athan <kgryte@gmail.com>
| im = offsetMask; | ||
| jm = im + ( ( N - 1 ) * strideMask ); | ||
| for ( i = 0; i < n; i++ ) { | ||
| if ( mget( mbuf, im ) === 0 && mget( mbuf, jm ) === 0 ) { |
There was a problem hiding this comment.
This is not correct. Why do pairs need to both be unmasked? This means that one value can effectively mask another. A mask is intended to reduce the domain of which values are reversed.
Consider the following trivial example:
var x = [ 1, 2, 3 ];
var mask = [ 1, 0, 0 ];
gmskrev( ..., x, ..., mask, ... )By your logic, the result should be
[ 1, 2, 3 ]
But that is not the right result. It should be
[ 1, 3, 2 ]
This affects all of this and related PRs.
| return x; | ||
| } | ||
| for ( i = m; i < n; i += M ) { | ||
| if ( mask[ im ] === 0 && mask[ jm ] === 0 ) { |
There was a problem hiding this comment.
Loop unrolling is not useful for this implementation. You should refactor to use a single loop.
| iy = ix + ( (N-1) * strideX ); | ||
| jm = im + ( (N-1) * strideMask ); | ||
| for ( i = 0; i < n; i++ ) { | ||
| if ( mask[ im ] === 0 && mask[ jm ] === 0 ) { |
There was a problem hiding this comment.
While this implementation is incorrect, this is the only loop which is necessary. Loop unrolling is not particularly effective when you have nested conditionals.
| - **N**: number of indexed elements. | ||
| - **x**: input array. | ||
| - **strideX**: stride length for `x`. | ||
| - **mask**: mask array. |
There was a problem hiding this comment.
|
|
||
| - If `N <= 0`, both functions return `x` unchanged. | ||
| - Both functions support array-like objects having getter and setter accessors for array element access (e.g., [`@stdlib/array/complex64`][@stdlib/array/complex64]). | ||
| - A mask value of `0` indicates that the corresponding element in `x` participates in the reversal, while a non-zero mask value indicates that the corresponding element should be skipped. |
There was a problem hiding this comment.
This can be removed once you update above.
Co-authored-by: Athan <kgryte@gmail.com> Signed-off-by: Athan <kgryte@gmail.com>
kgryte
left a comment
There was a problem hiding this comment.
Left initial comments. This and related PRs need quite a bit of clean-up and refactoring.
Resolves stdlib-js/metr-issue-tracker#195.
Description
This pull request:
blas/ext/base/gmskrevRelated Issues
This pull request has the following related issues:
blas/ext/base/gmskrevmetr-issue-tracker#195Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
Primarily written by Claude Code.
@stdlib-js/reviewers