Repository navigation
Expose llama_model_params.devices as IModelParams.Devices - #1447
Conversation
LLamaModelParams.devices was private with a todo, so the only way to pick a device from managed code was main_gpu. llama.cpp's default device selection (llama_prepare_model_devices) only considers integrated GPUs when no discrete GPU is present, and main_gpu indexes into that filtered list, so an iGPU next to a dGPU could never be selected. llama.cpp's escape hatch for this is the devices list (--device on the CLI). Add IModelParams.Devices (device names as returned by ggml_backend_dev_name, e.g. "Vulkan1"). ToLlamaModelParams resolves the names against the available ggml backend devices, builds a NULL-terminated array pinned in the existing GroupDisposable and assigns it to devices. Empty list or no matching names keeps devices NULL, i.e. the current behaviour. Also adds the ggml_backend_dev_name P/Invoke, implements the member in LLama.Web's ModelOptions, and covers the property in the ModelParams JSON round-trip test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The only finding is a non-blocking documentation nit.
Review effort: Lite
Findings: None
What changed in this PR
Adds managed llama.cpp device selection via IModelParams.Devices, including native device resolution and serialization support.
Changes:
- Exposes configurable device names through model parameters and web options.
- Resolves and pins native device handles during model loading.
- Adds interop and JSON round-trip test coverage.
| File | Summary |
|---|---|
LLama/Native/NativeApi.cs |
Adds device-name interop. |
LLama/Native/LLamaModelParams.cs |
Exposes the native devices pointer. |
LLama/Extensions/IModelParamsExtensions.cs |
Resolves and pins configured devices. |
LLama/Common/ModelParams.cs |
Adds managed device configuration. |
LLama/Abstractions/IModelParams.cs |
Defines the new API member; documentation nit noted for integrated-device fallback wording. |
LLama.Web/Common/ModelOptions.cs |
Supports device configuration. |
LLama.Unittest/ModelsParamsTests.cs |
Tests device-list serialization. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
martindevans
left a comment
There was a problem hiding this comment.
Just one minor issue with error handling, otherwise this looks pretty good. Thanks for working on it.
Review feedback: a device name that does not match any available device was silently dropped. ConvertDevices now resolves each name with an exact match first, then a case insensitive match, and throws UnknownDeviceException (new, in LLama/Exceptions) if neither matches. The exception carries the requested name and the list of available device names, and both appear in the message. Adds unit tests for the unknown-name and case-insensitive cases. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Thanks for the review. Pushed d704751 addressing both points:
|
…loaded The DllImport resolver only handled "llama" and "mtmd" and returned IntPtr.Zero for "ggml" and "ggml-base", leaving them to the default runtime probing. NativeLibraryUtils loads both by full path as dependencies of llama, which the default probing finds again by name on Windows and Linux but not on macOS (dlopen of a bare "libggml.dylib" does not match the @rpath install name of the already-loaded library), so any P/Invoke into ggml failed there with DllNotFoundException. This broke the new Devices tests on the macOS CI job and also affects TensorBufferOverrides, which uses the same imports. Keep the ggml and ggml-base handles when they are loaded as dependencies, and have the resolver return them (loading llama first if needed). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The new tests failed on the macOS job with Pushed 6789992: |
martindevans
left a comment
There was a problem hiding this comment.
Thanks for looking into that additional MacOS issue. This looks good to go to me 👍
Problem
llama_model_params.deviceshas been declared inLLamaModelParamsfor a while but isprivatewith atodocomment, so there is no way to set it from managed code. The only knob exposed isMainGpu.That matters because of how llama.cpp picks devices when
devicesis NULL (llama_prepare_model_devicesinsrc/llama.cpp):GGML_BACKEND_DEVICE_TYPE_GPU) are candidates;GGML_BACKEND_DEVICE_TYPE_IGPU) are only added when no discrete GPU exists;main_gpuindexes into that candidate list.So on a laptop with e.g. an NVIDIA dGPU plus an Intel Arc iGPU, the iGPU can never be selected through LLamaSharp, whatever
MainGpuis set to.TensorBufferOverridescannot work around it either: the KV cache still follows the candidate list, so weights and KV end up on different devices and the load crashes.llama.cpp's own escape hatch for this is
devices(--device Vulkan1on the CLI). This PR wires it through.Changes
IModelParams.Devices/ModelParams.Devices(List<string>, default empty): device names as returned byggml_backend_dev_name, e.g."Vulkan1","CUDA0","CPU". Empty keeps the current behaviour (llama.cpp chooses).IModelParamsExtensions.ToLlamaModelParams: resolves the names againstggml_backend_dev_count/get/name, builds a NULL-terminatedggml_backend_dev_t[], pins it in the existingGroupDisposable, and assignsdevices. Unknown names are ignored; if nothing matches,devicesstays NULL. Same pattern asConvertOverrides.LLamaModelParams.devicesbecomespublic IntPtr*(same size and offset as before, so the native layout is unchanged).NativeApi.ggml_backend_dev_nameP/Invoke added (ggml-base).LLama.WebModelOptionsimplements the new member.ModelsParamsTests.SerializeRoundTripSystemTextJsoncovers the new property.Breaking change
Adding a member to
IModelParamsbreaks external implementers of that interface (default interface members are not available onnetstandard2.0).ModelParamsandLLama.Web'sModelOptionsare updated here; third-party implementations need a one-lineList<string> Devices { get; } = new();.Testing
LLama,LLama.WebandLLama.Unittestbuild;ModelsParamsTestspasses.Devices = { "Vulkan1" }, and selecting the iGPU / CPU explicitly on an Intel N100.