vulkan: Fix newly added tests for permuted mul_mat and 1D im2col #10226
Conversation
| @@ -3630,9 +3630,19 @@ static void ggml_vk_mul_mat_vec_nc_f16_f32(ggml_backend_vk_context * ctx, vk_con | |||
|
|
|||
| static void ggml_vk_mul_mat(ggml_backend_vk_context * ctx, vk_context& subctx, const ggml_tensor * src0, const ggml_tensor * src1, ggml_tensor * dst, bool dryrun = false) { | |||
| VK_LOG_DEBUG("ggml_vk_mul_mat(" << src0 << ", " << src1 << ", " << dst << ")"); | |||
| if (src0->type == GGML_TYPE_F16 && ggml_is_permuted(src0) && ggml_is_permuted(src1) && dst->ne[1] == 1) { | |||
| if (src0->type == GGML_TYPE_F16 && ggml_is_permuted(src0) && ggml_is_permuted(src1) && dst->ne[1] == 1 && | |||
| // detect 0213 permutation, and batch size of 1 | |||
There was a problem hiding this comment.
Is this correct, or is there a better way to detect this permutation?
There was a problem hiding this comment.
There isn't a better way currently. We can add a helper function in ggml.h to check permutations if needed.
But I'm wondering how come the CUDA backend does not do these checks and still passes the tests?
There was a problem hiding this comment.
Because CUDA only uses this function if any_gpus_with_slow_fp16 is true, otherwise it falls back to another function, in this case ggml_cuda_op_mul_mat_cublas according to my debugger.
There was a problem hiding this comment.
If I run the tests with a Tesla P40 (which does have slow fp16) the CUDA tests do in fact fail in the same way, so I guess this fix should also be applied to CUDA.
There was a problem hiding this comment.
Thank you for this. I ran the tests again on my GPUs and it fixes the issues.
I think the permutation check should go into ggml.h to make it easier to understand and so we can reuse it in other backends. @ggerganov Should that be done in this PR or separately with the fix for CUDA?
|
Can be done in a separate PR. Maybe add a function |
This fixes recently added tests for permuted mul_mat and 1D im2col in the Vulkan backend. The new tests were added by c39665f and 80273a3.
The im2col fix is just applying the same code change as in 80273a3. The mul_mat fix is a combination of fixes, disabling fast paths that don't support certain combinations, and making some new tests unsupported. I verified that preexisting tests didn't change which code path they used and that no preexisting tests became unsupported.