-
-
Notifications
You must be signed in to change notification settings - Fork 46
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
la: fix matrix multiplication #198
la: fix matrix multiplication #198
Conversation
WalkthroughThe recent updates to the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- la/blas.v (5 hunks)
Additional comments: 11
la/blas.v (11)
- 139-140: The call to
vlas.dgemv
correctly updates the array dimensions and strides for the matrix-vector multiplication. However, ensure that thea.data
array passed toarr_to_f64arr[T]
is correctly converted to a[]f64
array, as this is crucial for the correctness of the operation. The stride parameters and the leading dimension (a.n
) seem correctly set according to the standarddgemv
usage.- 171-171: Similar to the
dgemv
call inmatrix_vector_mul
, thedgemv
call inmatrix_tr_vector_mul
appears to correctly handle the transpose operation by setting the first parameter totrue
. Again, verify the correct conversion ofa.data
andu
to[]f64
arrays and that the stride and leading dimension are set appropriately.- 203-203: The
vlas.dger
function call for performing the vector-vector transpose multiplication (vector_vector_tr_mul
) correctly sets the dimensions, alpha scaling factor, and strides. Ensure that the input vectorsu
andv
are correctly converted to[]f64
arrays. This operation is sensitive to the correct dimensionality of the input arrays, so double-checking the conversion logic inarr_to_f64arr[T]
is recommended.- 243-244: The
vlas.dgemm
call inmatrix_matrix_mul
correctly sets the parameters for matrix-matrix multiplication without transposition. It's important to ensure thata.data
andb.data
are correctly passed as[]f64
arrays. The leading dimensions (a.m
fora
andb.m
forb
) are correctly used, aligning with thedgemm
function's expectations.- 263-264: In
matrix_tr_matrix_mul
, thevlas.dgemm
call correctly handles the transposition of matrixa
by setting the first transposition parameter totrue
. As with otherdgemm
calls, verify the correct data type conversion and that the leading dimensions are set correctly, considering the transposition.- 272-273: The
vlas.dgemm
call inmatrix_matrix_tr_mul
correctly accounts for the transposition of matrixb
by setting the second transposition parameter totrue
. Ensure the data arrays are correctly typed and that the leading dimensions reflect the transposed dimensions ofb
.- 281-282: For
matrix_tr_matrix_tr_mul
, both matricesa
andb
are transposed as indicated by both transposition parameters being set totrue
. Verify the correctness of array conversions and that the leading dimensions are appropriately set for the transposed matrices.- 290-291: The
vlas.dgemm
call inmatrix_matrix_muladd
correctly performs matrix multiplication with addition. The parameters, including the alpha scaling factor and the addition of the result toc.data
, are set correctly. Ensure the data type conversions and leading dimensions are accurate.- 299-300: In
matrix_tr_matrix_muladd
, the transposition of matrixa
is correctly handled. The addition of the result toc.data
is also correctly implemented. As with other operations, verify the correctness of the data type conversions and the setting of leading dimensions.- 308-309: The
vlas.dgemm
call inmatrix_matrix_tr_muladd
correctly handles the transposition of matrixb
and the addition of the result toc.data
. Ensure the correctness of the data type conversions and the leading dimensions, especially considering the transposition ofb
.- 317-318: For
matrix_tr_matrix_tr_mul_add
, both matricesa
andb
are transposed, and the result is added toc.data
. Verify the correctness of the data type conversions and that the leading dimensions are set correctly for the transposed matrices.
@Eliyaan I think this PR is not correct. Are your matrices row major? will test it later |
I just fixed it on master along some other fixes! |
Fix #197
(I'm not sure that this PR is correct but it fixes the issue)
Summary by CodeRabbit