ggml : remove GGML_KQ_MASK_PAD constant - #17910
Conversation
|
|
||
| // q: [n_embd_k, n_batch, n_head, ne3 ] | ||
| // k: [n_embd_k, n_kv, n_head_kv, ne3 ] | ||
| // v: [n_embd_v, n_kv, n_head_kv, ne3 ] !! not transposed !! |
There was a problem hiding this comment.
There's a use of GGML_KQ_MASK_PAD in the comment on the next line.
| } | ||
|
|
||
| for (int i = n_tokens; i < GGML_PAD(n_tokens, GGML_KQ_MASK_PAD); ++i) { | ||
| for (int i = n_tokens; i < n_tokens; ++i) { |
There was a problem hiding this comment.
@ggerganov Totally random, but while looking at some of the ModernBert stuff, I (and @hansolosan) came across this line which now seems to do nothing with the removal of the padding. Should this whole set of nested loops be removed?
There was a problem hiding this comment.
cc @ryan-mangeno in case this has anything to do with your ongoing Modern Bert divergence issues
There was a problem hiding this comment.
I don't think it related to Modern Bert after all. From what I can tell, this (as well as build_inp_pos_bucket_enc) are only used in t5-dec.cpp and t5-enc.cpp respectively. Both of these input builders have similar properties (outer h loops that only loop once for example).
There was a problem hiding this comment.
I don't think this is actually causing any problems, but I made a PR to clean it up for future readers: #18795
There was a problem hiding this comment.
I don't think it related to Modern Bert after all. From what I can tell, this (as well as
build_inp_pos_bucket_enc) are only used int5-dec.cppandt5-enc.cpprespectively. Both of these input builders have similar properties (outerhloops that only loop once for example).
alright, thanks for letting me know though @gabe-l-hart ! I just got back to school but I have been doing quite a bit of debugging between transformers and llama.cpp in terms of finding where they diverge, if its useful ... I have been inspecting the pooling process much closer and using the llama-eval-callback tool, but there is no pooling flag to be passed, would it make sense to add support for this / expose it. In my case I can hardcode it in llama-embedding.cpp and view the tensors made during build_pooling.
The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in #17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org/llama.cpp#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* ggml : remove GGML_KQ_MASK_PAD constant * cont : remove comment
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* ggml : remove GGML_KQ_MASK_PAD constant * cont : remove comment
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* ggml : remove GGML_KQ_MASK_PAD constant * cont : remove comment
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* ggml : remove GGML_KQ_MASK_PAD constant * cont : remove comment
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* ggml : remove GGML_KQ_MASK_PAD constant * cont : remove comment
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* ggml : remove GGML_KQ_MASK_PAD constant * cont : remove comment
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org/llama.cpp#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org/llama.cpp#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
* ggml : remove GGML_KQ_MASK_PAD constant * cont : remove comment
* fix: Remove unnecessary `h` loops where `h` was only ever 0 Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * fix: Remove unnecessary padding loop that is never hit anymore The upper bound used to use GGML_PAD(n_tokens, GGML_KQ_MASK_PAD), but was removed in ggml-org#17910 leaving the loop dead. Branch: CleanUpT5InputBuilders Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> --------- Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
cont #16309
Simplify code now that we no longer need to pad the KQ mask for flash attention.