Support broadcast add & mul on CUDA (fixed) #2192
Conversation
| // compute | ||
| mul_f32_cuda(src0_ddf_i01, src1_ddf_i01, dst_ddf_i01, ne00, ne10, cudaStream_main); | ||
| } | ||
| mul_f32_cuda(src0_ddf_i, src1_ddf_i, dst_ddf_i, ne00*i01_diff, ne10*ne11, cudaStream_main); |
There was a problem hiding this comment.
The broadcasting logic here is still incorrect. The implementation on master broadcasts the values per row while this broadcasts the values after flattening both tensors. As long as ne11 == 1 this doesn't make a difference but I don't think that this is the implementation that we should be using.
There was a problem hiding this comment.
I noticed that the column number of src0 and src1 is ensured to be the same by checking ggml_can_repeat_rows, so broadcasting after flattening 2d sub blocks should be the same as broadcasting every row. Did I miss something?
There was a problem hiding this comment.
Okay, according to the commit history the following seems to have happened: I implemented broadcasting for multiplication both in dimension 0 and dimension 1. @ggerganov then added the additional requirement that dimension 0 must be equal via ggml_can_repeat_rows which effectively limits broadcasting to dimension 1. If that is indeed the specification to which broadcasting should be implemented then the broadcasting logic in this PR is correct.
There was a problem hiding this comment.
For now let's broadcast only over dimension 1 - later we'll fix the TODOs and support dimension 0 broadcasts.
Can also obsolete ggml_scale() when we do that.
Should fix #2191. Missed
ne11term forkyin the previous version. Now fixed and generation is correct.