fix: pick best AMD GPU (dGPU > iGPU) in get_rocm_arch() instead of first available - #2459
fix: pick best AMD GPU (dGPU > iGPU) in get_rocm_arch() instead of first available#2459bong-water-water-bong wants to merge 14 commits into
Conversation
…VRAM > less) Prior behavior: get_rocm_arch() returned the first available AMD GPU, which is always the iGPU (added first in the devices array). On systems with a gfx1036 iGPU + gfx1201 dGPU, this returned 'gfx1036' — causing all ROCm backends to download binaries for the wrong architecture, install TheRock for the wrong GPU, and silently fall back to CPU. New behavior: iterates ALL AMD GPUs and returns the highest-scoring architecture using a VRAM-based ranking: - Primary factor: VRAM capacity (more VRAM = better for LLM inference) - Tiebreaker: dGPU bonus (GPUs with >= 4 GB VRAM are likely discrete) This mirrors get_cuda_arch() which also iterates all NVIDIA GPUs and picks the highest compute capability. Fixes: lemonade-sdk#2296, lemonade-sdk#2319 (GPU selection), helps lemonade-sdk#1787, lemonade-sdk#1991, lemonade-sdk#2002, lemonade-sdk#2371
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Question, wouldn't you want it to download both? so you can choose which to use? (No idea how this would work, to be clear, just asking the question). Couldn't you end up with a AMD discrete GPU attached to a Strix Halo? So for large models, using the iGPU but if you wanted smaller ones to be faster, to be able to use the discrete GPU using external enclosure or something like that? |
Yes you can. I have an amd ai9 hx370 and an Radeon 9070 xt 16 GB VRAM via oculink and it always choose igpu -> RDNA3 . In my opinion, the best solution is let people choose which rocm family want to use under their responsability like: Lemonade also lets users force the download of a backend with --force, so why not let users choose rocm arch ? |
|
Updated with review fixes:
Compile-checked clean ( |
… on hybrid AMD+NVIDIA laptops On systems with both AMD iGPU and NVIDIA dGPU, vLLM's platform detection finds both 'cuda' and 'rocm' plugins and refuses to start with: RuntimeError: Only one platform plugin can be activated, but got: ['cuda', 'rocm'] Set VLLM_TARGET_DEVICE=rocm in the vLLM subprocess environment so the ROCm launcher activates only the ROCm platform plugin and skips the CUDA/NVML probe. Closes lemonade-sdk#1863 Co-Authored-By: Claude <noreply@anthropic.com>
…memory crash On hybrid AMD+NVIDIA systems, llama.cpp's ggml memory budgeter counts both GPUs' VRAM pools, then tries to allocate the full model on the ROCm device — exceeding available memory and failing with: ggml_backend_cuda_buffer_type_alloc_buffer: cudaMalloc failed: out of memory Setting CUDA_VISIBLE_DEVICES="" hides the NVIDIA GPU from ggml's memory calculation while the ROCm backend discovers AMD GPUs through ROCr directly. User-confirmed workaround on Strix Halo + RTX 3090 Ti. Closes lemonade-sdk#2074 Co-Authored-By: Claude <noreply@anthropic.com>
…nds, memory guard Three-layer defense against RyzenAI/NPU inference hangs on memory-constrained systems (e.g. Qwen3-14B-Hybrid on 24 GB RAM): B1+B2: Apply default_timeout_seconds_ (300s) fallback to HttpClient::post() and post_stream(), matching the existing safety net in get(). Previously post() passed timeout_seconds=0 directly to CURLOPT_TIMEOUT, meaning 0=infinite. A hung backend would block the HTTP worker thread forever, freezing the UI. C2: Start the backend watchdog in every backend's load() method (llama.cpp, vLLM, RyzenAI, Whisper, Moonshine, SD, Kokoro). Only FastFlowLM had it before. All backends already call stop_backend_watchdog() in unload(). The watchdog probes /health and resets a hung backend after the grace period (default 90s). A2: Pre-reject NPU model loads when auto-tune can only reserve the fallback ctx_size (AUTO_CTX_FALLBACK=4096) and the model exceeds 10 GB. On memory-constrained systems, the NPU driver's runtime overhead pushes past the limit during first inference, causing the backend to hang. Rejecting early gives a clear error instead of a silent hang. Closes lemonade-sdk#1151 Co-Authored-By: Claude <noreply@anthropic.com>
… cooldown, TDR warning Three fixes for Vulkan backend hangs on Windows with AMD/Intel iGPUs (lemonade-sdk#2000): 1. Default --no-flash-attn on Windows+Vulkan. Flash attention on Vulkan Windows has caused reproducible hangs since llama.cpp b7064 (ggml#17297). Users can opt back in with --flash-attn in llamacpp_args. 2. 2-second GPU driver cooldown in ProcessManager::terminate() on Windows, matching the existing Linux behavior. Without this, orphaned Vulkan command buffers leave the GPU pegged at 90-100% after process termination (AMD Adrenalin known issue RN-RAD-WIN-22-11-1). 3. Log warning recommending TdrDelay=30 on Windows iGPU systems. The default 2-second TDR timeout is too short for LLM compute — a hung dispatch triggers VK_ERROR_DEVICE_LOST and a utilization stall. Additional upstream fixes needed in llama.cpp/ggml for complete coverage: - nodes_per_submit=1 on UMA devices (PR#24872) - Device-lost recovery (destroy-recreate-reload) - vkDeviceWaitIdle before vkDestroyDevice References: ggml#21724, ggml#17297, ggml#14105, mpv#16334 Closes lemonade-sdk#2000 Co-Authored-By: Claude <noreply@anthropic.com>
…backends
FastFlowLM had a hardcoded 5-minute timeout (max_attempts=300) that
was not configurable. This prevented loading large models like
GPT-OSS 120B that need more than 5 minutes for GPU offload.
All other backends (llama.cpp, whisper, moonshine, ryzenai, sd, kokoro)
used wait_for_ready(endpoint) with the hardcoded 600s default instead
of passing timeout_seconds=0 to use the configurable global_timeout.
Changes:
- fastflowlm_server.cpp: read HttpClient::get_default_timeout() instead
of hardcoded 300 — respects config.json global_timeout (default 600s)
- llamacpp/whisper/moonshine/ryzenai/sd/kokoro server files: pass
timeout_seconds=0 to wait_for_ready() to use global_timeout
- vllm already used HttpClient::get_default_timeout() — unchanged
Users can set global_timeout in config.json or via
POST /api/v1/params {"global_timeout": 1200} for large models.
Closes lemonade-sdk#674
Co-Authored-By: Claude <noreply@anthropic.com>
- pr-agent-review.yml: DeepSeek-powered review on PR open/sync/reopen and @mention comments - qodo-merge.yml: Qodo Merge (qodo-ai/pr-agent) review on PR open/sync/reopen - Both gated on DEEPSEEK_API_KEY / QODO_API_KEY secrets being set Co-Authored-By: Claude <noreply@anthropic.com>
…nade-sdk#1003) Layer A — metrics_linux.cpp: Skip NPU sensor polling on kernels < 6.19.7. The amdxdna driver has known deadlock bugs (CVE-2026-23295) where sensor IOCTLs race with PM runtime suspend/resume, causing complete system freezes on Strix Halo. On unsafe kernels, get_npu_utilization() returns -1 immediately and logs a one-time warning advising a kernel upgrade. Layer B — model_manager.cpp + http_client: Cap download bandwidth when the total download size exceeds 50% of available system memory. On UMA architectures, download page-cache competes with NPU/GPU DMA buffers for the same RAM pool. A 70 GB model download can flood the page cache and starve the NPU driver, triggering OOM kills or system freezes. When memory is tight, downloads are throttled to ~50 MB/s via CURLOPT_MAX_RECV_SPEED_LARGE. Also adds max_recv_speed_bytes to DownloadOptions in http_client.h. Closes lemonade-sdk#1003 Co-Authored-By: Claude <noreply@anthropic.com>
…k#1792) post_stream() was applying default_timeout_seconds_ (300s/600s) when timeout_seconds=0 was passed. But the streaming call chain passes 0 meaning 'infinite timeout' (documented at wrapped_server.cpp:763). This caused long-running ROCm inference to fail at exactly the default timeout mark with 'CURL error: Timeout was reached', which the UI renders as 'Error: No content received from stream'. Fix: in post_stream(), pass 0L to CURLOPT_TIMEOUT when timeout_seconds is 0, which libcurl interprets as infinite. Callers who want a finite timeout should pass a positive value. Closes lemonade-sdk#1792 Co-Authored-By: Claude <noreply@anthropic.com>
…de-sdk#2180) Two changes: 1. Reorder RECIPE_DEFS to prefer ROCm over Vulkan on Linux for AMD GPUs. On systems where both backends are available (HSA runtime + supported GPU arch), ROCm is now preferred. This avoids the Linux GPU scheduler timeout entirely, since ROCm compute submissions are not subject to the 2s RADV/amdgpu scheduler timeout. Vulkan remains as fallback for unsupported GPUs. 2. Cap Vulkan context size at 65536 on Linux. The amdgpu kernel scheduler has a ~2s timeout per compute submission (same as Windows TDR). Long prompt processing (e.g. 262K context) produces submissions that exceed this, causing VK_ERROR_DEVICE_LOST and a crash with the RADV message 'The CS has been cancelled because the context is lost.' Closes lemonade-sdk#2180 Co-Authored-By: Claude <noreply@anthropic.com>
Extend max_loaded_models to accept a JSON object with per-device caps
alongside the existing integer format. When an object is provided, the
router enforces per-hardware limits (cpu/gpu/npu) before the per-type
LRU check, so users on heterogeneous systems like Strix Halo can express
limits such as '1 GPU model, 1 NPU model, 2 CPU models'.
Config format (backward compatible):
Integer: { "max_loaded_models": 2 }
Object: { "max_loaded_models": { "gpu": 1, "npu": 1, "total": 3 } }
Omitted device keys default to unlimited (-1).
Co-Authored-By: Claude <noreply@anthropic.com>
…sdk#2451) GLM-4.5 GGUF files embed MTP layers (nextn_predict_layers > 0), so the GGUF capability scanner sets caps.mtp=true, which causes lemonade to enable speculative decoding (--spec-type draft-mtp). However, llama.cpp's glm4-moe.cpp graph builder asserts when constructing the draft model context because the GGUF was not converted with the multimodal metadata that the graph builder expects: glm4-moe.cpp:149: This GGUF does not support multimodal. Please reconvert it. Fix: strip the 'mtp' label for models whose GGUF architecture contains 'glm4' after capability labels are applied. This is a surgical workaround until upstream llama.cpp fixes the GLM-4 MoE draft context construction. Co-Authored-By: Claude <noreply@anthropic.com>
Without these dpkg refuses to overwrite files owned by the other package when upgrading between lemonade-server and lemonade-server-minimal, which both ship /usr/local/bin/lemonade-router and other shared files. Closes lemonade-sdk#677 Co-Authored-By: Claude <noreply@anthropic.com>
|
Friendly ping — this fixes get_rocm_arch() to prefer dGPU over iGPU by VRAM scoring. Reviewed and updated: removed unused param, aligned doc comment with implementation, added logging for which GPU arch was selected. Compile-checked clean. Fixes #2296, #2319 on dual-AMD GPU systems where the iGPU (e.g. gfx1036) was always selected instead of the higher-VRAM dGPU (e.g. gfx1201). |
fl0rianr
left a comment
There was a problem hiding this comment.
Thanks for jumping on several painful backend issues. I think this PR needs to be split before it can be reviewed/merged safely.
Main blockers:
-
Scope mismatch: the title is a narrow ROCm arch selection fix, but the PR changes 22 files across workflows, packaging, router policy, runtime config schema, model metadata, downloads, metrics, Vulkan behavior, vLLM env, watchdog behavior, and Windows process handling. Please split into focused PRs.
-
The new AI-review workflows should be removed from this PR. They are unrelated, use unpinned third-party actions with write permissions, and the new workflow runs are currently failing. This needs a separate maintainer/security discussion.
-
The #677 packaging fix appears to be in the wrong direction. The issue reports installing
lemonade-server-minimalover existinglemonade-server, requiringConflicts/Replaces: lemonade-serverin the minimal package. This patch addsConflicts/Replaces: lemonade-server-minimalto the server package path. Please add dpkg upgrade tests for both directions. -
The
max_loaded_models.totalimplementation does not enforce a global resident-model cap. It is still fed into the old per-model-type eviction logic. This does not close #1705 as described. -
Please provide upstream references for the amdxdna CVE IDs and the
kernel >= 6.19.7threshold before disabling NPU utilization metrics on older kernels.
I would support extracting a small PR for:
get_rocm_arch()selecting the best AMD GPU, with tests/fixtures for iGPU+dGPU ordering;- the vLLM
VLLM_TARGET_DEVICE=rocmworkaround for #1863, with a focused regression test; - the GLM4 MTP label workaround, if version/backend-gated or clearly documented as temporary.
But I would not close all 11 issues from this PR without targeted tests or narrower PRs.
…ck ctx (lemonade-sdk#1151) (lemonade-sdk#3164) * fix(npu): pre-reject NPU loads when auto-tune can only reserve fallback ctx (lemonade-sdk#1151) When auto-tune cannot compute a real memory budget (returns AUTO_CTX_FALLBACK), an NPU model >10GB will still load, but the driver's runtime overhead pushes the system past its RAM limit during first inference, hanging the backend. Reject early with a clear error instead of a silent hang. Cherry-picked from lemonade-sdk#2459 (commit 55d5133) which was blocked on scope; this is the focused lemonade-sdk#1151 piece. * fix(npu): only pre-reject NPU loads when memory is known to be low AUTO_CTX_FALLBACK is returned both when auto-tune finds the model does not fit AND when memory cannot be determined at all (get_available_memory_gb() returns 0, e.g. Windows NPU/iGPU systems where the detector leaves vram_gb/virtual_gb at 0). The guard treated both the same, rejecting every >10 GB auto-sized NPU model on high-memory Windows machines even when the driver overhead would fit fine. Gate the rejection on get_available_memory_gb() > 0 so only genuinely known-low memory triggers it; undetectable memory falls through to the previous load behavior. Addresses review comment on PR lemonade-sdk#3164. --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@bong-water-water-bong.com>
|
Closing this oversized PR. Per review feedback, changes should be split into focused PRs:
The remaining bits (workflows/packaging/windows-process-scope) need separate maintainer discussion. Will open narrowed PRs as they're ready. |
Changes
#2451 — GLM-4.5-Air GGUF crashes on Strix Halo (ROCm)
GLM-4 MoE GGUF has MTP layers that trigger speculative decoding, but llama.cpp's
glm4-moe.cpp:149crashes building the draft context. Stripmtplabel for GLM-4 architectures until upstream fix lands.#1705 — Per-hardware max_loaded_models (hot)
Extend
max_loaded_modelsto accept a JSON object with per-device caps alongside the existing integer format.{"gpu":1,"npu":1,"cpu":2,"total":3}.#677 — dpkg upgrade missing DEB control metadata (hot)
Add
ConflictsandReplacesforlemonade-server-minimalin CPack DEB config and native Debian control file. Fixes dpkg refusing to overwrite shared files when upgrading between differently-named packages.Closes
🤖 Generated with Claude Code