Skip to content

fix cblas_sgemm call in ggml_compute_forward_mul_mat_*? #838

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

bogdad
Copy link
Contributor

@bogdad bogdad commented Apr 7, 2023

Hey, I was trying to understand how blas was used in llama.cpp and maybe i found a bug?

at least i dont understand why - the int after the x in cblas_sgemm calls should be the pitch(or stride) of array x.
but x comes from src0 in all the ggml_compute_forward_mul_mat_*, so i am guessing the stride of src0 should be used, which is ne00?

sorry if i am missing something : (

@prusnak
Copy link
Collaborator

prusnak commented Apr 7, 2023

The code comes from 2a2e63c by @ggerganov

@prusnak prusnak requested a review from ggerganov April 7, 2023 17:03
Copy link
Member

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a bug, but you are correct that it is better to change it to ne00.

It is not a bug because: ne00 == ne10
In ggml we always multiply z = x * yT so that the src0 (x) and src1 (yT) have the same number of columns.

https://github.com/ggerganov/llama.cpp/blob/698f7b5d6316a1f8453b3b32fd0d637d24952ffd/ggml.c#L2859-L2866

@bogdad
Copy link
Contributor Author

bogdad commented Apr 7, 2023

ah, makes sense, thank you!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants