Modify ggml_backend_is_cpu to use ggml_backend_i::get_name callback

[uextm]

This function was not actually calling the callback.

Instead, it was comparing the callback function to the static CPU name function. This does not work for sharing backends between modules in multi module programs.

[uextm]

Another thing to consider, would an int or enum value be better than a name to avoid a string comparison?

I only modified it to use the existing functions.

[slaren]

Can you elaborate on why you need this change? How are you trying to use ggml?

[uextm]

@slaren The better question is why was the original code comparing function pointers? This logic should be changed simply because it doesn't even use the callback properly.

Here is an actual scenario that cause an issue to explain why this needs to change:

1. You have a multi-module program.
2. In one module you create a llama.cpp context.
3. In another module you use that llama.cpp context to call llama_decode
4. llama_decode calls ggml_backend_is_cpu which returns false, because backend->iface.get_name == ggml_backend_cpu_name is false due to backend->iface.get_name and ggml_backend_cpu_name belonging to different modules

I'm surprised you're asking me to elaborate on why this change is needed when it's clear that the comparison in the code is just plain the incorrect comparison to make.

[slaren]

The original code is comparing function pointers because backend names are not meant to be unique identifiers. What you are doing is linking multiple times to ggml, and then using objects created with one copy in a different copy. That's not a supported scenario. You could try using a dynamic build of ggml instead, but in any case, this is not a good way to address this problem.

[uextm]

@slaren It works just fine using multiple modules. This is the only issue that stops it from working correctly.

The original code is comparing function pointers because backend names are not meant to be unique identifiers

Please explain why the function get_name returns any values at all if it will never be called and you're just going to compare its function pointer.

this is not a good way to address this problem
I agree this is not the best solution to the problem but I was attempting to make minimal changes.

This comparison should have been comparing an enum and it shouldn't be called a "backend name" since it isn't treated as one.

The only thing that absolutely shouldn't be the solution is comparing function pointers.

[slaren]

Backend names are meant to be informative strings that can be shown to the user. They were never meant to be used as unique identifiers. This wouldn't work with multi GPU backends that assign different names depending on the device being used, for example. Adding an enum is not an option because there isn't a fixed list of backends.

In any case, I think that you can address this by linking dynamically to ggml, and using a single copy of the binary. This will also save you some memory.

[uextm]

@slaren I think you misunderstood what I was trying to say.

Right now backend names aren't being used except in this function to check if the backend is CPU.

I understand if get_name is intended to be used by others to get a display name, but it shouldn't be the basis for this logic.

I'm suggesting that the logic for determining the value of ggml_backend_is_cpu should be based on a boolean/integer/enum that has nothing to do with get_name.

Conflating the purpose of get_name as a function for getting a user displayable string and using its pointer for determining if a given backend is CPU makes the function unclear at the very least.

[slaren]

The reason the get_name function is used in the ggml_backend_is_xxx functions is just because it was a simple way to implement these functions in the absence of other unique identifiers. It could be any other function in the ggml-backend interface, get_name just happens to be the first one so I choose to use that one. That does not mean that the get_name function is meant to be used in this way.

We cannot really have an enum because I don't want to have a centralized list of backends - users should be able to add their own backends without having to modify the ggml source code. What we could do instead is use a GUID to identify the backends, and add it to the ggml_backend struct. If you are willing to implement that, that could be merged.

In the future, the plan is to make the backends dynamic libraries that can be loaded at runtime. This will also require changes to the way backend-specific functions are currently implemented, including the ggml_backend_is_xxx functions. At that time I plan to re-evaluate this.

[uextm]

@slaren
The way I was thinking about it at least, an enum doesn't need to be an exhaustive list.
It could simply be CPU / Other which is more expandable in the future than a boolean and is more descriptive than a naked integer.

I am not against the GUID solution either and I can implement that if you'd like.
Would you want this done via a getter in ggml_backend_i or as a member of ggml_backend? (or a different method)

[slaren]

Since this is a constant, I think it would be fine to add this directly to the ggml_backend struct instead of adding a getter in the ggml_backend_i interface. We would also need to backend-specific global functions to obtain the GUID of each backend, akin to the current ggml_backend_is_xxx functions.

[uextm]

@slaren

For the GUID implementation do you want me to use platform UUID providers (meaning a wrapper for the Windows/Linux/MacOS/iOS/Android calls and value buffer) ?

Or did you want something much simpler that just used a sequence or random ID generated completely within ggml?

I'm asking because of the "No third-party dependencies" goal.
I also saw llama.cpp had this listed even though it depends on ggml, so I'm a bit unclear on what counts as "third-party".

[uextm]

Also should it be in a separate header/c pair (ggml-guid.h / ggml-guid.c)?

This way the GUID implementation could be used by other non-backend specific things (unsure if wanted/needed).

[slaren]

I don't think that we need to generate UUIDs, they can be generated with externals tools and be pre-defined for each backend. The only goal here is to avoid accidental collisions. So basically all that we need is a 128-bit UUID, in a standard format so that we can use existing tools to generate them, and a function to compare them.

[uextm]

What about generating the UUID for the CPU backend?

Shouldn't that be set when the backend is initialized in ggml_backend_cpu_init?

[slaren]

Yes, but it doesn't need to be generated on the fly, it can a constant pre-defined in the code. Just keep the GUID of each backend in a global, or in as a static in the function that returns it, and set it in ggml_backend struct in the backend init functions.

[uextm]

What value do you want to use as the pre-defined constant for the CPU backend GUID?

[slaren]

It can be any random value. For example I just found this site: https://www.uuidgenerator.net/. From what I understand, UUID v4 are just 128-bit completely random number without any real structure (other than the version I guess).

[uextm]

I'm aware it can be any value, I'm asking for your preference since you're the maintainer.

If you're fine with any random value than I'll do that.