Skip to content

Conversation

0cc4m
Copy link
Collaborator

@0cc4m 0cc4m commented Jul 27, 2025

No description provided.

@0cc4m 0cc4m requested a review from jeffbolznv July 27, 2025 08:57
@github-actions github-actions bot added Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Jul 27, 2025
}
} else if (tensor->op == GGML_OP_SET_ROWS) {
tensor_clone = ggml_set_rows(ggml_ctx, src_clone[0], src_clone[1]);
tensor_clone = ggml_set_rows(ggml_ctx, src_clone[0], src_clone[1], src_clone[2]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what the parameters should be here. Do we need to clone the dst tensor and pass that as the 'a' parameter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't look at it in detail, but this fixed the issue I had with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a weird operation because it doesn't keep around it's original 'a' parameter, and I think src_clone[2] will be null. And since it only writes sparsely to the destination we probably need to dup the original contents. So maybe something like:

   tensor_clone = ggml_dup(ggml_ctx, dst->view_src);
   tensor_clone = ggml_set_rows(ggml_ctx, tensor_clone, src_clone[0], src_clone[1]);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I checked again, my code just fixed the compile problem, it does not work if actually used. But neither does yours, since the tensor data is still on GPU, so dup does not work. I think manually copying the view tensor to CPU and using it as view_src would work, but that would mean reworking the check_result functions to support this.

I just deactivated SET_ROWS for now to fix the issue, if we need set_rows checks they can be readded at a later point.

@0cc4m 0cc4m force-pushed the 0cc4m/vulkan-debug-fixes branch from dbf7d78 to 573670b Compare July 31, 2025 13:11
@0cc4m 0cc4m requested a review from jeffbolznv July 31, 2025 13:12
@0cc4m 0cc4m merged commit e08a988 into master Jul 31, 2025
50 checks passed
@0cc4m 0cc4m deleted the 0cc4m/vulkan-debug-fixes branch July 31, 2025 15:46
infinitalo pushed a commit to infinitalo/qvac-ext-lib-llama.cpp that referenced this pull request Sep 29, 2025
* vulkan: fix debug mode issues

* vulkan: remove broken check_results GGML_OP_SET_ROWS support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning Vulkan Issues specific to the Vulkan backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants