Studio: GPU memory configuration for GGUF models#6414
Conversation
# Conflicts: # studio/backend/core/inference/llama_cpp.py
…L, loadedGpuMemoryFields helper)
…l leak, validate, types)
… floor, comment accuracy)
…ooltip, shorten fit dropdown label
…or-nothing toggle
# Conflicts: # studio/frontend/src/features/chat/chat-settings-sheet.tsx # studio/frontend/src/features/chat/stores/chat-runtime-store.ts
Resolve two conflicts: - chat-runtime-store.ts: keep both the unslothai#7152 HF-token imports and the new GPU-memory persistence helpers; drop the orphaned notifyHfTokenChanged (its call sites and logic moved to features/hub in unslothai#7152). - routes/inference.py: keep the unslothai#7209 gguf_load_in_flight download-safety and the unload-before-GGUF step; drop the inline inherited-extra-args block, which is superseded by the _resolve_inherited_extra_args helper called earlier in _load_model_impl (the helper adds tensor-split/offload stripping).
The name is used only in llama_server_args.py, routes/inference.py, and tests, not in llama_cpp.py; the unused hoisted import trips the import-hoist verifier in the source-lint CI job.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66e6b3e8aa
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # A changed GPU pick must reload (compare order-insensitively; None/[] | ||
| # both mean automatic). | ||
| if (self._gpu_ids or None) != (sorted(gpu_ids) if gpu_ids else None): | ||
| return False |
There was a problem hiding this comment.
Compare diffusion GPU picks against the runner's single device
When a Diffusion GGUF is loaded with a multi-GPU gpu_ids request (which the picker permits), _start_diffusion_server intentionally records only the lowest selected GPU because the runner is single-device, but this comparison still expects the entire original list. Consequently a repeated /load or settings Apply with the same multi-GPU request cannot deduplicate and unloads/restarts the diffusion server each time. Normalize the requested pick to its effective single diffusion GPU before comparing (and in the route-level matcher).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 958ccb1. The diffusion runner records only its single lowest device (self._gpu_ids = [sorted(gpu_ids)[0]]), so both _already_in_target_state and the route _request_matches_loaded_settings now collapse the requested pick to [sorted(gpu_ids)[0]] for a loaded diffusion model before comparing. A repeated multi-GPU request that resolves to the same device now deduplicates instead of restarting the server. Covered by test_gpu_ids_reload_detection_collapses_diffusion_to_single_device and test_route_matches_loaded_settings_collapses_diffusion_gpu_ids.
| if single_device_gpu is not None: | ||
| try: | ||
| selected_gpu = int(single_device_gpu) | ||
| if selected_gpu < 0: | ||
| raise ValueError | ||
| except (TypeError, ValueError): | ||
| return False, { | ||
| "mode": "single_device", | ||
| "reason": "unresolved_gpu_id", | ||
| } |
There was a problem hiding this comment.
Handle nonnumeric diffusion device tokens in the training guard
When training is active and DG_GPU is configured with a supported nonnumeric token such as a CUDA UUID (or is empty to force CPU), _diffusion_gpu_arg forwards that token here, but can_load_chat_during_training converts it with int() and returns unresolved_gpu_id. The guard therefore rejects every such diffusion load with 409 before the runner starts, even though the runner itself receives and uses the same DG_GPU token. Resolve UUID/CPU tokens against visible devices or fall back to the existing GGUF guard instead of treating them as an invalid integer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 958ccb1. can_load_chat_during_training no longer calls int() blindly on the single-device token: an empty token (a CPU-only runner such as a CPU diffusion GGUF) is allowed outright since it uses no GPU VRAM, and a non-numeric token such as a CUDA UUID or MIG handle falls back to sizing against the whole visible pool like the GGUF guard instead of returning unresolved_gpu_id. Covered by test_single_device_unresolved_token_sizes_against_pool, test_single_device_unresolved_token_refuses_when_pool_full, and test_single_device_cpu_token_allows.
The diffusion runner drives only its single lowest device and the backend records that one device (self._gpu_ids = [sorted(gpu_ids)[0]]), but the reload dedupe compared it against the full requested list, so a multi-GPU pick that resolves to the same device forced a needless reload. Normalize the request the same way for a loaded diffusion model in both _already_in_target_state and the route _request_matches_loaded_settings. The chat-during-training coexistence guard called int() on the single-device token and hard-rejected when it could not parse. A non-numeric token (a CUDA UUID / MIG handle) now sizes against the whole visible pool like the GGUF guard instead of falsely blocking the load, and an empty token (a CPU-only runner such as a CPU diffusion GGUF) is allowed outright since it uses no GPU VRAM.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
… TP reset, tensor_split validation - Training coexistence guard: a single-device runner pinned through an unresolvable UUID/MIG token was sized against the aggregate visible-VRAM pool, so a load could pass on capacity it cannot use and then OOM active training. Size against the worst-case visible device (min free) instead, keeping the guard's documented default-deny contract. The empty-token (CPU-only runner) allow path is unchanged. - Diffusion startup: _start_diffusion_server now resets self._tensor_parallel to False alongside the other placement resets. A prior tensor-parallel chat load (process killed but not fully unload-reset) otherwise left /status misreporting tensor parallelism and made an identical diffusion re-Apply reload against the stale state. - tensor_split: reject negative / non-finite / all-zero splits up front. They were dropped at launch but still compared raw in the reload dedupe, so an identical Apply reloaded indefinitely. - Tests: the shared httpx stub was incomplete and, installed via setdefault before real httpx loaded, broke a combined pytest run (collection errors on httpx.Response). Import the real installed httpx instead.
|
I ran an independent multi-reviewer pass over this PR and triaged the results against the code. Pushing fixes for the safe, high-value items; noting the rest so you can decide. Fixed in this push:
Noted, not changed here (lower severity or your design call):
Checked and dismissed as false positives:
All backend suites pass per-file, plus the combined run is now clean. Happy to send any of the "noted" items as a follow-up if you'd like. |
for more information, see https://pre-commit.ci
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…location (#7252) #6414 moved the llama_extra_args inheritance out of the GGUF branch in _load_model_impl into _guard_chat_load_against_training, which runs before the branch, so 'if request.llama_extra_args is None' is no longer inside the gguf_branch slice that test_load_marker_precedes_hub_guard_and_unload checks. The assertion failed on that now-missing landmark even though the guarantee it protects (the gguf_load_in_flight marker is entered before the hub-download guard and the unload) is intact. Drop the relocated landmark from the ordering so the test matches the current structure. Co-authored-by: danielhanchen <unslothshared@gmail.com>
* Studio: GPU memory dropdown — llama.cpp --fit on and manual gpu-layers/cpu-moe * Studio: simplify GPU memory changes (reuse ParamSlider, GPU_LAYERS_ALL, loadedGpuMemoryFields helper) * Studio: GPU picker — choose which GPUs a GGUF model loads on (gpu_ids) * Studio: simplify GPU picker (share /api/system fetch, validate gpu_ids) * Studio: GPU picker review fixes (gate relative indices, no cross-model leak, validate, types) * Studio: group GPU controls under a collapsible GPU section * Studio: GPU feature review fixes (fix fit-ctx test, behavior-test the floor, comment accuracy) * Studio: make GPU a top-level settings section (not nested under Model) * Studio: flatten GPU controls into the Model section, group by GPU/context/generation * Studio: move GPU Memory to the bottom of Model with its dependent controls beneath it * Studio: move GPU Memory below Tensor Parallelism and GPUs below GPU Memory * Studio: tighten GPU Memory and GPU Layers tooltip copy * Studio: fix fit-mode context slider track-click, restore GPU Memory tooltip, shorten fit dropdown label * Studio: GPU Memory tooltip one mode per line, briefer * Studio: note HIP_VISIBLE_DEVICES (ROCm) in the GPUs picker tooltip * Studio: narrow the GPU Memory dropdown to fit the shortened label * Studio: use 'llama.cpp --fit' in the GPU Memory tooltip for consistency * Studio: allow Tensor Parallelism in Manual GPU mode * Studio: graduated MoE-on-CPU offload (--n-cpu-moe) replacing the all-or-nothing toggle * Studio: size the MoE-offload slider for staged (deferred-load) models * Studio: share one GGUF header walk for the context-length and MoE-count readers * Studio: size the GPU Layers slider for staged models (one staged-header read) * Studio: move Tensor Parallelism below the GPUs picker * Studio: GPU split (--tensor-split) per-GPU model share in Manual mode * Studio: tolerate whitespace in GPU split input, move it below GPU Layers * Studio: rename the GPU split control to "Split ratio" * Studio: Split ratio sends explicit even input; fix blank=free-VRAM (not even) copy * Studio: tighten llama.cpp --fit VRAM margin with --fit-target 512 * Studio: GPU memory review fixes (rollback re-baseline, single-GPU TP gate, accurate copy) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: move Split ratio below MoE Layers on CPU * Studio: address PR review (fix GPU-info hydration race, share fit context-length across load paths) * Studio: address codex review (manual single-GPU TP guard, GPU-aware spec defaults in fit/manual, GGUF-only context/preference) * Studio: address codex review round 2 (gpu_present seed, single-GPU tensor-split guard, staged manual-knob reset, strip inherited offload flags) * Studio: address codex review round 3 (strip inherited --n-cpu-moe, CPU-fallback warning in Manual mode) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: address codex review round 4 (preserve pinned fit context across a later Apply) * Studio: address codex review round 5 (honor GPU picker for diffusion GGUFs, clear fit pin on cross-model switch) * Studio: preserve the pending GPU Memory mode when staging a model * Studio: pin diffusion GPU device order and reset GPU-memory state for diffusion loads * Studio: address codex review round 6 (fit-Auto rollback context, preserve manual non-tensor split modes, persist GPU mode on load not select) * Studio: persist the applied GPU Memory mode, not the requested one (skip diffusion loads) * Studio: replace Manual-mode split-ratio field with per-GPU layer sliders * Studio: clarify per-GPU layer split hint for tensor-parallel mode * Studio: address codex review round 7 (allow GGUF gpu_ids past the legacy guard, replay GPU-memory fields on respawn) * Studio: address codex review round 8 (size the validate preflight like the load in fit mode, across both load paths) * Studio: skip the training-OOM guard for llama.cpp --fit GGUF loads (they spill to RAM) * Studio: drop the now-redundant compare-path validate sizing (the --fit guard skip makes it moot) * Studio: address codex review round 9 (keep the training guard for fit loads, forward gpu_ids to validate, strip inherited manual tensor-split) * Studio: address codex review round 10 (gate GPU-memory adoption on is_gguf, record manual knobs only in Manual mode) * Studio: handle diffusion GGUFs symmetrically in the GPU Memory controls (preserve the standing mode preference, hide the inapplicable mode/TP controls) * Studio: remember the GPU Memory settings per model * Studio: consolidate --fit mode and Manual mode into a single Manual mode * Studio: preserve the per-GPU layer split across GPU Layers changes * Studio: trim overly long GPU Memory comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * address GPU memory config review comments * trim redundant GPU memory tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reconcile manual-mode TP drops with the unslothai#6659 drop-site invariants * Preserve quantized KV in manual --fit, charge GGUF companions in full, reconcile GPU pick on load * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Clear stale GPU baseline on non-GGUF loads so it can't read as dirty * Fix no-context-shift test for the conditional -c flag * Credit manual GPU-layer offload for cached HF GGUFs * Reset per-model load knobs on GGUF quant switch * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Strip inherited tensor-split when manual ratio is cleared * Match auto-load validation to safetensors placement * Reset editable manual knobs after Auto GGUF loads * Record a single device for diffusion GPU picks * Reset per-model GPU knobs before applying saved settings * Address review comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Guard manual tensor splits and keep remembered context on auto-load * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Snapshot compare knobs, seed splits from free VRAM, flag zero-offload loads * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Exempt CPU-only loads from the guard floor and harden compare and reseed paths * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reach full offload from the layers slider and charge extras drafters in the guard * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Warm the GPU device cache before pick reconciles and disable staged GPU controls * Align the training guard with inherited extras, spec mode, and compare targets * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Hide GPUs from companion-less zero-offload loads * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Size diffusion picks per device, own manual offload flags, reject XPU picks * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Drop tensor flags at zero layers and exempt CPU-pinned drafters * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Allowlist the zero-layer tensor parallel drop site * Keep validate and load guards on the same extras and refresh stale baselines * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Drop mismatched manual tensor splits before launch * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Gate XPU picks on the real backend field and harden split and hydration paths * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Weight full GPUs as zero, clamp split shares, and refine the zero-layer mask gate * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Carry fit context across mode changes and align drafter and picker gates * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Catch variant switches, uncached diffusion repos, and text-only mmproj skips * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Check companions on the first device and size native and remote zero-layer loads * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Replace the training guard's precise VRAM modeling with a conservative bound * Baseline context pins on non-GGUF hydration and reprobe list-seeded staged GGUFs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Size manual splits by their largest share and preserve resolved context from Default * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Default-deny unsized required companions and price KV at the effective cache dtype * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reserve MTP draft KV and MLA target-copy in the training guard * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Size tensor-parallel loads per device and show GPU controls for native GGUFs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reserve MTP overhead for uncached remote GGUFs and the mmproj runtime factor * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Drop the training-coexistence VRAM estimation this PR added * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Gate remembered load settings to GGUF picks * Lock the remaining load-time controls during a staged load * Clear the stale native-path token on compare loads * Drop a stale guard reference from the zero-offload masking comment * Seed GPU baselines from the rollback response and drop never-emitted offload flags * Match validate's training guard to load and keep the native reload token * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim verbose GPU-memory comments * Thread the variants header walk off the event loop, honor device pins on zero-offload, and hold staged GPU edits * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honor manual placement and classify pinned zero-offload loads * Close diffusion admission and status hydration gaps * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Check the actual diffusion GPU during training * Align staged baselines and manual reload dedupe * Fix GGUF placement and rollback state * Harden manual GGUF placement boundaries * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Remove unused resolve_tensor_parallel import in llama_cpp.py The name is used only in llama_server_args.py, routes/inference.py, and tests, not in llama_cpp.py; the unused hoisted import trips the import-hoist verifier in the source-lint CI job. * Fix diffusion GPU dedup and training guard for non-numeric device tokens The diffusion runner drives only its single lowest device and the backend records that one device (self._gpu_ids = [sorted(gpu_ids)[0]]), but the reload dedupe compared it against the full requested list, so a multi-GPU pick that resolves to the same device forced a needless reload. Normalize the request the same way for a loaded diffusion model in both _already_in_target_state and the route _request_matches_loaded_settings. The chat-during-training coexistence guard called int() on the single-device token and hard-rejected when it could not parse. A non-numeric token (a CUDA UUID / MIG handle) now sizes against the whole visible pool like the GGUF guard instead of falsely blocking the load, and an empty token (a CPU-only runner such as a CPU diffusion GGUF) is allowed outright since it uses no GPU VRAM. * Tighten comments added by the GPU memory config changes * Harden GGUF placement from independent review: VRAM sizing, diffusion TP reset, tensor_split validation - Training coexistence guard: a single-device runner pinned through an unresolvable UUID/MIG token was sized against the aggregate visible-VRAM pool, so a load could pass on capacity it cannot use and then OOM active training. Size against the worst-case visible device (min free) instead, keeping the guard's documented default-deny contract. The empty-token (CPU-only runner) allow path is unchanged. - Diffusion startup: _start_diffusion_server now resets self._tensor_parallel to False alongside the other placement resets. A prior tensor-parallel chat load (process killed but not fully unload-reset) otherwise left /status misreporting tensor parallelism and made an identical diffusion re-Apply reload against the stale state. - tensor_split: reject negative / non-finite / all-zero splits up front. They were dropped at launch but still compared raw in the reload dedupe, so an identical Apply reloaded indefinitely. - Tests: the shared httpx stub was incomplete and, installed via setdefault before real httpx loaded, broke a combined pytest run (collection errors on httpx.Response). Import the real installed httpx instead. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com>
…location (unslothai#7252) unslothai#6414 moved the llama_extra_args inheritance out of the GGUF branch in _load_model_impl into _guard_chat_load_against_training, which runs before the branch, so 'if request.llama_extra_args is None' is no longer inside the gguf_branch slice that test_load_marker_precedes_hub_guard_and_unload checks. The assertion failed on that now-missing landmark even though the guarantee it protects (the gguf_load_in_flight marker is entered before the hub-download guard and the unload) is intact. Drop the relocated landmark from the ordering so the test matches the current structure. Co-authored-by: danielhanchen <unslothshared@gmail.com>
Studio currently decides how a GGUF model is placed on the GPU(s), with no way to override it: you cannot choose which GPUs to use, how many layers go on the GPU, or how MoE experts are offloaded. This PR adds those controls.
Closes #4870
Closes #5025
Closes #6320
Closes #6530
Controls
The main addition is a GPU Memory dropdown:
--fit on. Studio floors an automatically sized context at 8192 tokens and targets a 512 MiB margin per GPU. Fitting prioritizes dense tensors, so overflow including MoE experts can spill to RAM. If you select a context length,--fitkeeps that length and adjusts placement around it.--fit offand is attempted as requested. Studio does not resize the context or layer count to rescue a configuration that does not fit.--tensor-split: the values are layer counts in layer-split mode and relative shares under Tensor Parallelism.A GPUs checkbox group chooses which GPUs llama.cpp may use. Leaving every GPU checked keeps the automatic device set. The picker is hidden when Studio cannot map the reported devices to physical GPU indices.
Tensor Parallelism remains available in Default mode and for explicit Manual placement with at least two selected GPUs. It is hidden with GPU Layers on Auto because llama.cpp's
--fitis incompatible with tensor split mode.The existing Remember settings option now stores GPU Memory mode, GPU Layers, MoE offload, and the GPU selection per GGUF quant, alongside the other load settings. The per-GPU split is deliberately recomputed for the current GPU set rather than persisted.
Behavior boundaries
--fit; explicit layers are loaded as requested, and a configuration that does not load remains the user's responsibility.Verification
Local verification passes: 161 targeted backend tests covering GPU memory mode, active-training loads, GGUF reload inheritance, and tensor-parallel vision behavior; Ruff on the changed backend files; targeted frontend ESLint; and the frontend production build.