AMD: Multi-GPU RX 9070 XT w/ RX 480 - GGUF OOM Labels & GPU Memory Controls#7356
AMD: Multi-GPU RX 9070 XT w/ RX 480 - GGUF OOM Labels & GPU Memory Controls#7356LeoBorcherding wants to merge 19 commits into
Conversation
…r-visible devices On a Vulkan llama.cpp build, llama-server can drive cards the torch backend cannot see (e.g. a pre-ROCm RX 480 next to a ROCm-only RX 9070 XT on Windows). Studio previously built its GPU list only from torch/ROCm, so such cards were invisible: TIGHT/OOM labels budgeted against half the real VRAM and the unslothai#6414 GPU Memory controls could not offer the second card. Backend: - _vulkan_probe.py prints a 5th column with ggml's device description. - LlamaCppBackend gains _run_vulkan_probe (shared parse, 4/5-column tolerant), vulkan_device_inventory (names + real totals for the UI), and validate_vulkan_gpu_ids (picks validated in ggml Vulkan ordinal space). - /api/system reports gguf_devices (the llama-server inventory, Vulkan builds) alongside the torch view, and gguf_gpu_ids_supported becomes true when the inventory is available: probe, picker, and --device Vulkan<i> pin all share one index space, so the old unconditional 400 on Vulkan picks is replaced with ordinal validation in /load and /validate. - The inventory rides its own 60s cache: /api/system pollers (floating monitor, Resources tab) keep expiring the 10s GPU cache, and the probe spawns a subprocess. Model loads keep their own fresh probe. Frontend: - GpuInfo gains ggufMemoryTotalGb (sum of llama-server-visible discrete VRAM, falling back to the torch total). GGUF fit surfaces (hub fit labels, variant expanders, fits-on-device filters) budget against it; training and safetensors estimates stay on the torch total, which training truly has. - The GPU picker lists gguf_devices (both cards, names, per-card VRAM) and allows picks on Vulkan builds. Tests: probe 5-column parse, inventory, ordinal validation, and the /api/system merge (torch view preserved, inventory surfaced and cached across GPU-cache refreshes, support flag).
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48dfa33e58
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- recommended-fit: only GGUF rows budget against the llama-server total; safetensors/MLX rows load through torch, which can't use Vulkan-only cards. - use-gpu-info: a reported llama-server inventory is authoritative even at zero discrete VRAM (all-iGPU case counts as system RAM, not torch dGPU VRAM); only a missing inventory falls back to the torch total. - Training guard: a Vulkan build's gpu_ids (ggml ordinals) are dropped before the physical-index free-VRAM sizing, so the load is guarded as unpinned instead of resolving the wrong card or skipping protection. - Persisted picks: remember the pick index space (physical vs vulkan) in localStorage; reconcilePersistedGpuIds clears saved picks after a backend swap flips the space, where a bare-number membership check would let a pick land on a different physical card. - _vulkan_probe: the name lookup binds in its own best-effort block so a ggml-base without ggml_backend_dev_description can't disable iGPU detection, and stdout is reconfigured to UTF-8 (reader decodes to match) so non-ASCII device names can't kill the probe on cp1252 consoles. Tests: guard drops/keeps gpu_ids by backend kind (test_chat_load_during_training).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40c1464d41
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
a12578f to
40c1464
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79eace774d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…keep fresh live picks Codex round-3 follow-ups on the Vulkan llama.cpp GPU-selection path: - can_load_chat_during_training: add worst_case_gpu_count so a Vulkan pick (ggml ordinals with no physical mapping) is sized against the N most- constrained visible cards instead of falling back to the whole-pool check, which could OK a load that lands on a busy selected card and OOMs training. - /load and /validate: reject gpu_ids for diffusion GGUFs on a Vulkan build with a 400 -- the diffusion runner cannot map ggml Vulkan ordinals. - reconcilePersistedGpuIds: gate the index-space-change clear behind a new fromPersisted flag so a freshly picked live Vulkan selection is not nulled for the whole first post-swap session. Persisted PerModelConfig restores pass fromPersisted:true; live store reads (send path, unload-restore) do not.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d65536268f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The pre-download /load and /validate Vulkan gates rejected gpu_ids whenever _classify_diffusion_gguf was not False, but None is the ordinary first-load case for an uncached Hub GGUF (no local header to classify yet). That 400'd first-time remote GGUF loads and made the new Vulkan GPU picker unusable unless the model was already cached. Reject only a CONFIRMED diffusion GGUF (is True) at the gates. For the rare uncached model that turns out to be diffusion after download, add a spawn-time backstop in load_model: on a Vulkan build, drop the unmappable gpu_ids pin before _start_diffusion_server (ggml Vulkan ordinals cannot be forwarded as the runner's CUDA/DG token) so it serves on the default device instead of the wrong card. Regression tests cover the relaxed gate and the spawn backstop.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9afb8011c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
applyPerModelConfigToRuntime runs synchronously on model selection, which can happen before any GPU hook has fetched /api/system. On a cold cache reconcilePersistedGpuIds can't tell whether the saved ids belong to the current llama.cpp backend's index space (physical CUDA/ROCm ids vs ggml Vulkan ordinals), so it passes them through. That value then launders into the store and the later load path reconciles it as a live pick (fromPersisted false), which never clears it -- a saved physical [1] gets sent as Vulkan ordinal 1 and pins the wrong card. When the pick is applied cold, warm the cache and re-reconcile with fromPersisted, overwriting only if the user has not replaced the pick since.
test_compare_load_uses_each_models_gpu_config asserted the exact single-arg
call reconcilePersistedGpuIds(ownConfig.selectedGpuIds); the per-model (persisted)
path now passes { fromPersisted: true } to drop a pick saved under the other
llama.cpp backend's GPU index space. Match the new call and assert the flag.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d13f25a76
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codex round on the fromPersisted marker: it was set on call sites that also
carry fresh, current-space picks, and the cold-cache revalidation raced the
load path.
- applyPerModelConfigToRuntime now takes { fromPersisted } (default false =
live). Only a config actually restored from storage passes true.
- chat-page load-on-selection passes fromPersisted only for the
rememberedConfigFor fallback, not an explicit selection.config (fresh Run
settings / picker edit). Prevents a freshly selected Vulkan ordinal from being
cleared and loaded on all GPUs.
- shared-composer compare pane gates fromPersisted on (!config && ownRemembered),
so an explicit sel.config pane pick stays live.
- Cold-cache race: instead of leaking the raw persisted ids into the store
(where the load path snapshots and sends them before the async revalidation
runs), park the pick at null (all GPUs, safe) and fill in the validated pick
once the cache warms. The load snapshot can no longer send a wrong-space pin.
Contract test updated + a regression guard for the storage-only invalidation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c85e763029
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The fromPersisted-by-call-site marker kept getting misapplied because those sites carry both storage restores and fresh live edits, and the cold-cache park/revalidate raced the load path. Replace it with per-value provenance: each GPU pick carries the index space it was made under, and reconcile drops it only when that stamp no longer matches the backend. - use-gpu-info: currentGpuIndexKind() (from /api/system gguf_devices); drop the session-global gpuIndexSpaceChanged marker machinery. - PerModelConfig gains selectedGpuIdsIndexKind (persisted); the runtime store gains selectedGpuIdsKind, stamped wherever a pick is set (live picker = current kind, load echo = current kind, config apply = the config's stamp). - reconcilePersistedGpuIds(ids, savedKind): drop when savedKind != current kind (both known); unstamped picks default to physical (pre-Vulkan). - Run settings stamps the current kind when the user edits the GPU picker, so an edited pick is never treated as a stale cross-space restore; an unedited stored pick keeps its saved kind. - No more park-null / async revalidate / fromPersisted: a same-backend refresh keeps the saved subset (kinds match), a cross-space pin is dropped, and the load snapshot carries the stamp so it can't send a wrong-space pin. Fixes the three Codex findings (cold-cache saved-pick loss, model-generation race, unedited run-settings treated as live). Contract tests updated; typecheck + build + tests/studio contracts green.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bdee5c7f8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…us kind Four Codex follow-ups on the Vulkan GPU-selection path: - /load + /validate: run validate_vulkan_gpu_ids off the event loop (asyncio.to_thread). It can spawn a blocking Vulkan probe subprocess, which would otherwise freeze status/progress/unload for up to the probe timeout. - /api/system, /load, /validate: check the Vulkan build BEFORE the XPU ban. A Vulkan pick uses ggml ordinals (--device Vulkan<i>), not torch-xpu ordinals, so an Intel/XPU host on a Vulkan build must still get the picker and accept gpu_ids instead of being rejected. - Active/status GPU-kind stamp: an active GGUF hydrated from /status before the GPU cache warms had no index-space stamp, so a later Reload/Remember treated its live Vulkan pick as a legacy physical one and reconciled it away. Carry selectedGpuIdsKind through useActiveModelConfig (via the reactive useCurrentGpuIndexKind) and fall back to the current kind in currentRuntimePerModelConfig when the store stamp is missing -- an active pick is by definition in the current backend's space.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1c1aa7fed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…an-6414 # Conflicts: # studio/backend/routes/inference.py # studio/backend/routes/training_vram.py # studio/backend/tests/test_gpu_memory_mode.py
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a96018358f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 715b20561f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… budget - gpuFieldsSignature now folds selectedGpuIdsIndexKind into the pick, so when the reactive fallback stamps a cold-hydrated active pick after the GPU cache warms, an open Run-settings panel remounts and re-snapshots with the new stamp instead of reloading/saving it as a legacy physical pick. - /api/system reports gguf_backend_is_vulkan so the frontend can tell an empty gguf_devices on a Vulkan build (probe failed/masked, budget unknown) from a non-Vulkan build. Empty Vulkan inventory now budgets GGUF at 0 instead of the torch VRAM total, which on a mixed host would let fit checks pass against VRAM /load cannot place.
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5d1646989
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| * /api/system cache has not populated yet (so callers cannot decide and must | ||
| * defer the index-space judgement to a later warm reconcile). */ | ||
| export function currentGpuIndexKind(): GpuIndexKind | null { | ||
| return systemGpuIndexKind(cachedSystem); |
There was a problem hiding this comment.
Refresh GPU kind after backend swaps
When the user updates or swaps the llama.cpp backend in the same browser session after /api/system has already populated, this returns the old cached index space forever because fetchSystemOnce never refetches once cachedSystem is set. The new selectedGpuIdsIndexKind reconciliation therefore keeps stale physical pins when the backend has become Vulkan, or drops valid Vulkan pins after a swap back, so loads can target the wrong ordinal space until a full page reload; the GPU cache needs an invalidation/refetch path when the llama runtime changes.
Useful? React with 👍 / 👎.
| # CUDA/DG_GPU token -- an ordinal there would target the wrong | ||
| # card. Drop the unmappable pin so the runner uses its default | ||
| # device selection instead (same as an unpinned diffusion load). | ||
| if gpu_ids and is_vulkan_backend: |
There was a problem hiding this comment.
Remove the earlier Vulkan diffusion raise
When a Vulkan-pinned GGUF reaches the diffusion branch after earlier classification missed it, this fallback never executes: the same if is_vulkan_backend and gpu_ids condition already raises a few lines above. As a result the load still hard-fails instead of clearing the unmappable ggml ordinal and starting the diffusion runner on its default device, so the new unclassified-GGUF backstop remains broken unless the earlier raise is removed or converted into this fallback.
Useful? React with 👍 / 👎.
| selectedGpuIdsIndexKind: | ||
| s.selectedGpuIds == null | ||
| ? undefined | ||
| : ((s.selectedGpuIdsKind ?? currentGpuIndexKind()) ?? undefined), |
There was a problem hiding this comment.
Avoid saving active Vulkan picks as unstamped
If /status hydrates an already-loaded Vulkan model before the /api/system GPU cache is warm, selectedGpuIdsKind is null and this snapshot still emits undefined when currentGpuIndexKind() is also null. Any immediate Remember/rollback path then treats that active Vulkan pick as a legacy physical pick (applyPerModelConfigToRuntime defaults missing stamps to physical), so the next warm reconcile can drop or mis-stamp a valid GPU selection; the snapshot path needs to warm/defer the kind before serializing a live pick.
Useful? React with 👍 / 👎.
Problem
On a Vulkan llama.cpp build, llama-server can drive GPUs the torch backend cannot see. A user report (dual AMD on Windows: RX 9070 XT on ROCm + RX 480, which is too old for ROCm, via the Vulkan prebuilt) shows llama-server using both cards for inference while Studio behaves as if only one exists:
Root cause:
/api/systembuilds its device list from torch/ROCm (get_backend_visible_gpu_info), and every fit label sums VRAM over that list. The report suggested pulling fromllama-server --list-devices; this PR does the equivalent through the ggml Vulkan probe Studio already ships, so no extra server spawn is needed.Backend
_vulkan_probe.pyprints a 5th column: ggml's device description (ggml_backend_dev_description), sanitized for the tab protocol.LlamaCppBackendgains_run_vulkan_probe(shared parse, tolerant of 4- or 5-column output),vulkan_device_inventory(names + real totals for the UI), andvalidate_vulkan_gpu_ids./api/systemreportsgguf_deviceson Vulkan builds: the llama-server inventory in ggml Vulkan ordinal space, alongside the unchanged torch view indevices. The inventory rides its own 60s cache so/api/systempollers don't respawn the probe subprocess every 10s GPU-cache expiry; model loads keep their own fresh probe.gpu_idspicks on Vulkan builds are now supported instead of hard-400ing: the probe, the picker, and the existing--device Vulkan<i>pin all share one index space, so/loadand/validatevalidate picks against the probed ordinals. XPU behavior is unchanged.Frontend
GpuInfogainsggufMemoryTotalGb: the sum of llama-server-visible discrete VRAM (iGPUs excluded, their memory is the system RAM already budgeted separately), falling back to the torch total on non-Vulkan builds. All GGUF fit surfaces (hub fit labels, GGUF variant expanders, fits-on-device filters) budget against it.Tensor parallelism is intentionally untouched; whether row-split TP is worthwhile on a mixed-generation pair is a separate question from device visibility.
Tests
/api/systemmerge: torch view preserved, inventory surfaced, support flag semantics, and no re-probe across GPU-cache refreshes.Verified live on a ROCm host:
devicesunchanged,gguf_devicesempty, picks still supported. The mixed-GPU Vulkan path is covered by the synthetic-probe tests; I don't have a pre-ROCm card to reproduce the reporter's exact hardware.