Skip to content

common : use common_ prefix for common library functions #9805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 10, 2024

Conversation

slaren
Copy link
Member

@slaren slaren commented Oct 9, 2024

Prefix common library functions with common_ instead of llama_ to make it clear that these functions do not belong to the base API.

@slaren slaren force-pushed the sl/rename-common-funcs branch from 09ee5f9 to 4f7e4b5 Compare October 9, 2024 10:26
@github-actions github-actions bot added testing Everything test related examples server labels Oct 9, 2024
@slaren
Copy link
Member Author

slaren commented Oct 9, 2024

There are probably other functions that should be renamed as well - this was just a quick pass over common.h to see if there is interest in this change. I will do a more thorough pass if there is.

@ggerganov
Copy link
Member

Yes, this is a good change.

@slaren
Copy link
Member Author

slaren commented Oct 9, 2024

Should I also rename the gpt_ functions to common_?

@ggerganov
Copy link
Member

Yes, along with the structs and enums (e.g. struct gpt_params, enum gpt_sampler_type, gpt_log).

@slaren slaren force-pushed the sl/rename-common-funcs branch from 13b4249 to 3c0b862 Compare October 9, 2024 12:08
@github-actions github-actions bot added the android Issues specific to Android label Oct 9, 2024
@slaren slaren force-pushed the sl/rename-common-funcs branch from 1f1d6f4 to 6ea0304 Compare October 9, 2024 14:47
common/common.h Outdated
Comment on lines 413 to 416
struct common_init_result common_init_from_common_params(common_params & params);

struct llama_model_params llama_model_params_from_gpt_params (const gpt_params & params);
struct llama_context_params llama_context_params_from_gpt_params (const gpt_params & params);
struct llama_model_params common_model_params_from_common_params (const common_params & params);
struct llama_context_params common_context_params_from_common_params(const common_params & params);
Copy link
Member Author

Choose a reason for hiding this comment

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

These functions may need better names now.

Copy link
Member

Choose a reason for hiding this comment

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

common_init_from_common_params -> common_init_from_params

Maybe:

common_model_params_from_common_params -> common_model_params_to_llama
common_context_params_from_common_params -> common_context_params_to_llama

@slaren slaren merged commit 7eee341 into master Oct 10, 2024
54 checks passed
@slaren slaren deleted the sl/rename-common-funcs branch October 10, 2024 20:57
dsx1986 pushed a commit to dsx1986/llama.cpp that referenced this pull request Oct 29, 2024
* common : use common_ prefix for common library functions

---------

Co-authored-by: Georgi Gerganov <[email protected]>
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
* common : use common_ prefix for common library functions

---------

Co-authored-by: Georgi Gerganov <[email protected]>
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
* common : use common_ prefix for common library functions

---------

Co-authored-by: Georgi Gerganov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android Issues specific to Android examples server testing Everything test related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants