Skip to content

Conversation

@ggerganov
Copy link
Member

copilot:all

@github-actions github-actions bot added testing Everything test related ggml changes relating to the ggml tensor library for machine learning labels Jul 11, 2024
GGML_API GGML_CALL int ggml_blck_size(enum ggml_type type);
GGML_API GGML_CALL size_t ggml_type_size(enum ggml_type type); // size in bytes for all elements in a block
GGML_API GGML_CALL size_t ggml_row_size (enum ggml_type type, int64_t ne); // size in bytes for all elements in a row
GGML_API GGML_CALL int64_t ggml_blck_size(enum ggml_type type);
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 more than just a name change but returning a larger integer should be safe I think.

ggml/src/ggml.c Outdated
if (ne % ggml_blck_size(info->type) != 0) {
fprintf(stderr, "%s: tensor '%s' of type %d (%s) number of elements (%" PRId64 ") is not a multiple of block size (%d)\n",
__func__, info->name.data, (int)info->type, ggml_type_name(info->type), ne, ggml_blck_size(info->type));
__func__, info->name.data, (int) info->type, ggml_type_name(info->type), ne, (int) ggml_blck_size(info->type));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of casting to int I think it would be better to use the PRId64 macro like ne does.

ggml/src/ggml.c Outdated
Comment on lines 15201 to 15202
enum ggml_type const kq_vec_dot_type = type_traits[k->type].vec_dot_type;
ggml_from_float_t const kq_from_float = type_traits[kq_vec_dot_type].from_float;
Copy link
Collaborator

Choose a reason for hiding this comment

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

My personal opinion is that the old names were better because kq_ as a prefix is ambiguous but I don't really care either way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for taking a look - changed back to the original names

@ggerganov ggerganov merged commit 370b1f7 into master Jul 12, 2024
@ggerganov ggerganov deleted the gg/ggml-naming branch July 12, 2024 07:46
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 13, 2024
* ggml : minor naming changes

ggml-ci

* ggml : use PRId64 [no ci]

* ggml : revert FA K/Q names
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 13, 2024
* ggml : minor naming changes

ggml-ci

* ggml : use PRId64 [no ci]

* ggml : revert FA K/Q names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples ggml changes relating to the ggml tensor library for machine learning testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants