[Feature] Wire Metal device selection into the router FFI device paths - #3517
Draft
Bevisy wants to merge 1 commit into
Draft
[Feature] Wire Metal device selection into the router FFI device paths#3517Bevisy wants to merge 1 commit into
Bevisy wants to merge 1 commit into
Conversation
Every FFI device-selection site (embedding-model init, similarity, unified-classifier model managers, MLP selector device_type=2) hardcoded a CUDA-or-CPU fallback, so on macOS use_cpu=false ran on the CPU even when the binding is built with --features metal (the Metal device resolver from vllm-project#3165 was only wired into the ModernBERT loader). Route all sites through core::device::resolve_device instead, so use_cpu=false selects Metal on Apple Silicon builds while Linux/CUDA/ROCm and CPU-only behavior stays unchanged. Add Metal-selection unit tests for resolve_device and device_from_type(2), and a metal-build-only device log line. Related: vllm-project#3516 Signed-off-by: Binbin Zhang <binbin36520@gmail.com>
✅ Deploy Preview for vllm-semantic-router ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
the
pr/needs-author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related #3516
Purpose
The Metal device resolver added in #3165 Add Metal (Apple Silicon) support to candle-binding (
core::device::resolve_device, bounded Metal inference thread pool, queue draining) was only wired into the ModernBERT loader. The router-facing FFI entrypoints that load embeddings, similarity, and MLP selectors still hardcoded a CUDA-or-CPU fallback, so on macOS (Apple Silicon)use_cpu=falseran on the CPU even when the binding is built with--features metal.This slice routes every inference device-selection site through the shared
core::device::resolve_device:ffi/embedding.rs— allinit_*_embedding*entrypointscore/similarity.rs—BertSimilarity::newclassifiers/unified/model_managers.rs—LoRAModelManagerffi/mlp.rs—device_from_type(2)(Metal selector device type; the Go binding already definesMLPDeviceMetal)core/device.rs— Metal-selection unit tests + a metal-build-only device log lineNon-macOS / non-metal builds are unchanged: the non-metal branch of
resolve_deviceis identical to the previous inline code, so Linux/CUDA/ROCm and CPU-only behavior is byte-for-byte equivalent.Test Plan
cargo fmt/cargo check/ clippy pass under the CI toolchain (rust 1.90).--features metalon darwin/arm64:resolve_device(false).is_metal(),resolve_device(true)stays CPU,device_from_type(2).is_metal().metalfeature built,InitModel(<MiniLM dir>, use_cpu=false)+GetEmbeddinglogs[candle-binding] inference device: Metal(MetalDevice(DeviceId(..)))and returns a 384-dim embedding — a real BERT forward on the GPU.cargo test --no-default-features --features metal --libshows zero regressions vs the base tree (identical failure set, all pre-existing metal-feature kernel gaps / missing local test fixtures unrelated to this slice).Test Result
All checks above pass locally on darwin/arm64 (rust 1.90 override = CI toolchain). Remaining risk: qwen3/gemma embedding kernels are not yet Metal-covered (tracked in #3516's qualification scope), so this PR only makes the device selection wiring correct — it does not claim full model coverage on Metal.