Conversation
|
Explore the complete analysis inside the Version Insights I've successfully generated a summary report for your project. Here are the key findings: Summary Report for llama.cpp PR #871Good News: ✅ No significant performance regressions were detected in this pull request. Key Points:
Recommendation: This pull request appears safe to merge from a performance perspective, as all functions maintained stable performance within acceptable thresholds. The analysis indicates that the changes in PR #871 for the auroralabs-loci/llama.cpp repository have not introduced any measurable performance degradation. |
4d071b3 to
8e509d5
Compare
d664a5a to
48924ee
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirrored from ggml-org/llama.cpp#18724
In ggml_cuda_mul_mat_q(), when in the MoE path and when quantizing src1, s13 is incorrectly declared to nb[2] instead of nb[3].
Earlier in the function:
const int64_t s03 = src0->nb[3] / ts_src0;
const int64_t s3 = dst->nb[3] / ts_dst;
...
if (!ids) {
...
{
const int64_t s11 = src1->nb[1] / ts_src1;
const int64_t s12 = src1->nb[2] / ts_src1;
const int64_t s13 = src1->nb[3] / ts_src1;
}
Later, in the ids path:
const int64_t s11 = src1->nb[1] / ts_src1;
const int64_t s12 = src1->nb[2] / ts_src1;
const int64_t s13 = src1->nb[2] / ts_src1; <--- [2] here, repeated, should be [3].
ne13 is currently asserted to 1 , so this very likely doesn't affect anything yet, but it should still be fixed for correctness.
Copilot summary:
This pull request fixes an indexing bug in the
ggml_cuda_mul_mat_qfunction inmmq.cu. The change corrects the calculation of thes13variable to use the correct dimension of thesrc1tensor, which improves the accuracy of subsequent operations.s13to usesrc1->nb[3]instead ofsrc1->nb[2], ensuring the correct tensor dimension is used in matrix multiplication operations.