Skip to content
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

Introduce backend GUIDs #743

Merged
merged 8 commits into from
Feb 24, 2024
Merged

Introduce backend GUIDs #743

merged 8 commits into from
Feb 24, 2024

Conversation

uextm
Copy link
Contributor

@uextm uextm commented Feb 22, 2024

Initial proposed implementation of backend GUIDs
(Discussed in #741)

Hardcoded CPU backend GUID (for now)
Change ggml_backend_is_cpu logic to use GUID

Initial proposed implementation of backend GUIDs
(Discussed in ggerganov#741)

Hardcoded CPU backend GUID (for now)
Change ggml_backend_is_cpu logic to use GUID
src/ggml-backend-impl.h Outdated Show resolved Hide resolved
Remove redundant functions `ggml_backend_i::get_name` and `ggml_backend_guid` which are not desired for future expansion
Copy link
Collaborator

@slaren slaren left a comment

Choose a reason for hiding this comment

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

It would be good to keep the helper function ggml_backend_guid, just returning the value from the struct instead.

include/ggml/ggml-backend.h Outdated Show resolved Hide resolved
include/ggml/ggml.h Outdated Show resolved Hide resolved
src/ggml-backend.c Outdated Show resolved Hide resolved
src/ggml.c Outdated Show resolved Hide resolved
@uextm
Copy link
Contributor Author

uextm commented Feb 22, 2024

It would be good to keep the helper function ggml_backend_guid, just returning the value from the struct instead.

OK adding it back, it just seemed like this was redundant in the same ways the other things you wanted removed were.

I'm just trying to follow the style of the project and your comments; personally I would have done things very differently so I'm going off your guidance.

@slaren slaren merged commit 7c53153 into ggerganov:master Feb 24, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants