studio/setup.sh: guard empty CUDA arch detection in the source build (#5854)#6481
Conversation
PR unslothai#5826 hardened setup.sh for fresh CUDA toolkits, but the source build still set -DCMAKE_CUDA_ARCHITECTURES only when nvidia-smi reported a compute capability. When that query returns nothing the build proceeded with no explicit arch list, so llama.cpp built PTX only. On a driver older than the toolkit that binary fails at runtime with "the provided PTX was compiled with an unsupported toolchain" - the build succeeds, so neither the build-time check nor the CPU fallback caught it (issue unslothai#5854). Resolve the arch list before committing to a CUDA build. A new pure helper _resolve_cuda_archs parses and de-duplicates the nvidia-smi compute_cap output and honors an explicit UNSLOTH_LLAMA_CUDA_ARCHS override. When the result is empty, build CPU llama.cpp instead of a PTX-only binary, with a clear message pointing at the override - so the user still ends up with a working llama-server. The override also lets advanced users force a native build on hosts where nvidia-smi cannot report compute_cap. No behavior change when an arch is detected: -DGGML_CUDA=ON plus the arch, CUDA flags and NVCC_PREPEND_FLAGS are assembled exactly as before. Adds tests/sh/test_resolve_cuda_archs.sh (single/multi/dedup/empty/garbage/ whitespace/override cases), wired into tests/run_all.sh and the studio-backend-ci.yml shell-test loop.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function _resolve_cuda_archs in studio/setup.sh to safely resolve CUDA architectures, falling back to a CPU build if no architecture is detected to prevent runtime failures on older drivers. It also adds corresponding unit tests in tests/sh/test_resolve_cuda_archs.sh. Feedback was provided to improve the helper function by declaring variables as local to prevent scope pollution, adding an early return for empty input, and using pure Bash parameter expansion instead of spawning a subshell with tr for better performance.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| _resolve_cuda_archs() { | ||
| _raw_caps=$1 | ||
| _arch_override=$2 | ||
| if [ -n "$_arch_override" ]; then | ||
| printf '%s' "$_arch_override" | ||
| return 0 | ||
| fi | ||
| _archs="" | ||
| while IFS= read -r _cap; do | ||
| _cap=$(printf '%s' "$_cap" | tr -d '[:space:]') | ||
| if [[ "$_cap" =~ ^([0-9]+)\.([0-9]+)$ ]]; then | ||
| _arch="${BASH_REMATCH[1]}${BASH_REMATCH[2]}" | ||
| case ";$_archs;" in | ||
| *";$_arch;"*) ;; | ||
| *) _archs="${_archs:+$_archs;}$_arch" ;; | ||
| esac | ||
| fi | ||
| done <<< "$_raw_caps" | ||
| printf '%s' "$_archs" | ||
| } |
There was a problem hiding this comment.
Scope and Performance Improvements in _resolve_cuda_archs
- Scope Pollution: The variables
_raw_caps,_arch_override,_archs,_cap, and_archare currently declared as global variables. Declaring them aslocalprevents accidental scope pollution and potential side effects if the function is ever sourced or refactored. - Avoid Unnecessary Loop & Subshells: When
_raw_capsis empty, thewhileloop still executes once because<<<appends a newline. This triggers a subshell and executestrunnecessarily. Adding an early returnif [ -z "$_raw_caps" ]avoids this entirely. - Pure Bash Parameter Expansion: Spawning a subshell and running external commands like
tr(_cap=$(printf '%s' "$_cap" | tr -d '[:space:]')) inside a loop is slow, especially on Windows (Git Bash). We can use pure Bash parameter expansion${_cap//[[:space:]]/}to strip all whitespace in-process without spawning any external commands.
_resolve_cuda_archs() {
local _raw_caps=$1
local _arch_override=$2
if [ -n "$_arch_override" ]; then
printf '%s' "$_arch_override"
return 0
fi
if [ -z "$_raw_caps" ]; then
return 0
fi
local _archs=""
local _cap
local _arch
while IFS= read -r _cap; do
_cap="${_cap//[[:space:]]/}"
if [[ "$_cap" =~ ^([0-9]+)\\.([0-9]+)$ ]]; then
_arch="${BASH_REMATCH[1]}${BASH_REMATCH[2]}"
case ";$_archs;" in
*";$_arch;"*) ;;
*) _archs="${_archs:+$_archs;}$_arch" ;;
esac
fi
done <<< "$_raw_caps"
printf '%s' "$_archs"
}![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Lines 1490 to 1494 in c4544b9
Reuse the fallback nvidia-smi when resolving CUDA archs
When _setup_has_usable_nvidia_gpu classified the host through its /usr/bin/nvidia-smi fallback because nvidia-smi is not on PATH, this new arch resolution still leaves _raw_caps empty because it only probes command -v nvidia-smi. The empty-arch branch below now builds a CPU llama.cpp binary, so those NVIDIA source-build installs lose CUDA even though /usr/bin/nvidia-smi --query-gpu=compute_cap could have supplied the architecture; resolve the same smi path here before treating the arch as undetectable.
ℹ️ 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".
…tection Addresses review feedback on the empty-CUDA-arch guard: _setup_has_usable_nvidia_gpu classifies a host as NVIDIA-usable using nvidia-smi on PATH OR /usr/bin/nvidia-smi, but the new arch detection probed only `command -v nvidia-smi`. On a GPU host where nvidia-smi is off PATH (reachable only at /usr/bin), arch detection returned empty and the new empty-arch branch dropped the build to CPU, losing CUDA. Mirror the same PATH-then-/usr/bin resolution so those hosts still get a native CUDA build. Also scope _resolve_cuda_archs locals with `local` (no behavior change; it already runs under command substitution).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a6cacda4b
ℹ️ 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".
| _smi_bin="/usr/bin/nvidia-smi" | ||
| fi | ||
| if [ -n "$_smi_bin" ]; then | ||
| _raw_caps=$(_setup_run_smi "$_smi_bin" --query-gpu=compute_cap --format=csv,noheader 2>/dev/null || true) |
There was a problem hiding this comment.
In the repo CPU workflow (.github/workflows/studio-backend-ci.yml runs python -m pytest tests/ ...), tests/studio/install/test_gpu_detection_followups.py::TestSetupShHardening::test_compute_cap_probe_timeout_wrapped still searches for the literal _setup_run_smi nvidia-smi --query-gpu=compute_cap. This line now calls _setup_run_smi "$_smi_bin" ..., so the auto-discovered test fails (I reproduced with python -m pytest -q tests/studio/install/test_gpu_detection_followups.py::TestSetupShHardening::test_compute_cap_probe_timeout_wrapped). Please update the existing guard to accept the variable/fallback form, otherwise CI is red.
Useful? React with 👍 / 👎.
The nvidia-smi /usr/bin fallback parameterized the binary in the compute_cap probe (_setup_run_smi "$_smi_bin" ...), so the literal-string assertion in test_compute_cap_probe_timeout_wrapped no longer matched. Assert the probe is preceded by _setup_run_smi (timeout-wrapped) instead, scanning all occurrences so the comment mention is ignored. Same intent, binary-agnostic.
Collapse the backslash-continued assert onto one line and normalize slice spacing so the ruff-format pre-commit hook (0.6.9) is satisfied. Formatting only; no behavior change.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
# Conflicts: # tests/run_all.sh
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
|
Ran the shell tests (10/10) and exercised One follow-up, not blocking: the Windows path in LGTM for the Linux fix. |
…5854) The Windows source build added -DGGML_CUDA=ON unconditionally but only set -DCMAKE_CUDA_ARCHITECTURES when $CudaArch was detected. With no detectable compute capability that produced a PTX-only binary, the same hole the Linux fix closed. Build CPU llama.cpp in that case, and honor UNSLOTH_LLAMA_CUDA_ARCHS to force a CUDA build, matching setup.sh. Detected-arch builds are unchanged.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
The undetectable-arch CPU fallback adds an earlier -DGGML_CUDA=OFF, so the ordering check now anchors on -DGGML_CUDA=ON and the last -DGGML_CUDA=OFF instead of the first.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc34ef69e6
ℹ️ 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".
| # arch builds a PTX-only binary, so build CPU instead. Mirrors the | ||
| # Linux fix; set UNSLOTH_LLAMA_CUDA_ARCHS=120 to force a CUDA build. | ||
| substep "could not detect a CUDA compute capability; building CPU llama.cpp instead of a PTX-only binary (set UNSLOTH_LLAMA_CUDA_ARCHS=120 to force a CUDA build)." "Yellow" | ||
| $CmakeArgs += '-DGGML_CUDA=OFF' |
There was a problem hiding this comment.
When this no-arch fallback is taken on Windows, $HasNvidiaSmi stays true while the generated CMakeCache.txt records GGML_CUDA:BOOL=OFF. The existing source-build reuse check above treats exactly that combination as stale ($HasNvidiaSmi -and -not $cachedCuda), so users whose nvidia-smi cannot report compute_cap will rebuild llama.cpp on every subsequent setup run instead of reusing the working CPU fallback. Please record/special-case this fallback, or clear the GPU expectation, so the CPU build can be reused.
Useful? React with 👍 / 👎.
| if ($HasNvidiaSmi -and $NvccPath) { | ||
| $CmakeArgs += '-DGGML_CUDA=ON' | ||
| # Accept a host MSVC newer than nvcc's whitelist; a fresh toolkit | ||
| # (e.g. CUDA 13.3) otherwise aborts with "#error -- unsupported | ||
| # Microsoft Visual Studio version!". Mirrors the Linux fix. Via env | ||
| # (covers the configure probe + build), after Refresh-Environment, idempotent. | ||
| $nvccAllowFlag = '-allow-unsupported-compiler' | ||
| if ([string]::IsNullOrEmpty($env:NVCC_PREPEND_FLAGS)) { | ||
| $env:NVCC_PREPEND_FLAGS = $nvccAllowFlag | ||
| } elseif ($env:NVCC_PREPEND_FLAGS -notlike "*$nvccAllowFlag*") { | ||
| $env:NVCC_PREPEND_FLAGS = "$($env:NVCC_PREPEND_FLAGS) $nvccAllowFlag" | ||
| } | ||
| substep "NVCC_PREPEND_FLAGS = $env:NVCC_PREPEND_FLAGS" | ||
| $CmakeArgs += "-DCUDAToolkit_ROOT=$CudaToolkitRoot" | ||
| $CmakeArgs += "-DCUDA_TOOLKIT_ROOT_DIR=$CudaToolkitRoot" | ||
| $CmakeArgs += "-DCMAKE_CUDA_COMPILER=$NvccPath" | ||
| if ($CudaArch) { | ||
| # Validate nvcc actually supports this architecture | ||
| if (Test-NvccArchSupport -NvccExe $NvccPath -Arch $CudaArch) { | ||
| $CmakeArgs += "-DCMAKE_CUDA_ARCHITECTURES=$CudaArch" | ||
| } else { | ||
| # GPU arch too new for this toolkit -- fall back to highest supported. | ||
| # PTX forward-compatibility will JIT-compile for the actual GPU at runtime. | ||
| $maxArch = Get-NvccMaxArch -NvccExe $NvccPath | ||
| if ($maxArch) { | ||
| $CmakeArgs += "-DCMAKE_CUDA_ARCHITECTURES=$maxArch" | ||
| substep "GPU is sm_$CudaArch but nvcc only supports up to sm_$maxArch" "Yellow" | ||
| substep "Building with sm_$maxArch (PTX will JIT for your GPU at runtime)" "Yellow" | ||
| # UNSLOTH_LLAMA_CUDA_ARCHS (e.g. "120" or "89;86") forces the build | ||
| # arch and wins over detection, matching setup.sh. | ||
| $CudaArchOverride = if ($env:UNSLOTH_LLAMA_CUDA_ARCHS) { ($env:UNSLOTH_LLAMA_CUDA_ARCHS -replace '\s', '') } else { '' } | ||
| if ((-not $CudaArch) -and (-not $CudaArchOverride)) { |
There was a problem hiding this comment.
On Windows source builds where nvidia-smi -L detects a GPU but --query-gpu=compute_cap returns empty and no driver-compatible CUDA toolkit is installed, this CPU fallback is never reached: the build path already calls Resolve-CudaToolkit -RequireOrExit, which can install or exit before $NvccPath is available. Because the no-arch fallback is nested under $HasNvidiaSmi -and $NvccPath, the #5854 host still fails setup instead of building CPU llama.cpp; detect the missing arch before requiring nvcc, or let this fallback proceed without a toolkit.
Useful? React with 👍 / 👎.
* fix(studio): opt-in source-build GPU smoke validation (#5854) Gap 1 (empty CUDA arch -> CPU) already landed in #6481. Wire gap 2: after a GPU source build, optionally run the same staged llama-server smoke test as the prebuilt path, then CPU-fallback on failure. Gated by UNSLOTH_LLAMA_STAGED_VALIDATION (default off) to avoid Blackwell JIT stalls. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix(install): normalize staged validation env in setup.sh (#7322) Strip and lowercase UNSLOTH_LLAMA_STAGED_VALIDATION before the shell gate so values like True and surrounding whitespace match the Python staged_validation_enabled() helper. * Rebuild visual server after staged-validation CPU fallback (#5854) Mirror the primary source-build path by best-effort building llama-diffusion-gemma-visual-server after smoke-failure CPU fallback. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
3 participants
Summary
Addresses #5854 (gap 1): the source-build path could produce a PTX-only CUDA binary that fails at
runtime.
studio/setup.shset-DCMAKE_CUDA_ARCHITECTURESonly whennvidia-smi --query-gpu=compute_capreturned a value. When that query came back empty the build proceeded with no explicit arch list, so
llama.cpp built PTX only. On a driver older than the toolkit the binary then fails at first use with:
The build itself succeeds, so neither the build-time check nor the CPU fallback added in #5826 caught it.
Change
Resolve the arch list before committing to a CUDA build:
_resolve_cuda_archsparses and de-duplicates thecompute_capoutput and honors anexplicit
UNSLOTH_LLAMA_CUDA_ARCHSoverride (passed through verbatim, for example120or86;120).pointing at the override, so the user still ends up with a working
llama-server. The override also letsadvanced users force a native build on hosts where
nvidia-smicannot reportcompute_cap.No behavior change when an arch is detected:
-DGGML_CUDA=ON, the arch,-DCMAKE_CUDA_FLAGSandNVCC_PREPEND_FLAGSare assembled exactly as before (the happy-pathCMAKE_ARGSis byte-equivalent). Thedeferral of
-DGGML_CUDA=ONis safe because at that pointCMAKE_ARGSis still the clean CPU baselinecaptured for
CPU_FALLBACK_CMAKE_ARGS.On gap 2 (post-build GPU smoke test)
The issue also suggested running the prebuilt path's GPU smoke test after a source build. That validation
is currently gated off in
install_llama_prebuilt.py(_RUN_STAGED_PREBUILT_VALIDATION = False) becausethe
llama-serverGPU forward pass JIT-compiles CUDA kernels on first load and stalls every install byminutes on Blackwell. Adding an always-on equivalent to the source path would reintroduce that stall.
Gap 1 instead removes the only remaining way to silently produce a PTX-only binary: combined with the
toolkit-major-greater-than-driver fallback from #5826, the documented runtime PTX failure is now prevented
at build time rather than caught afterwards. If we later re-enable the staged validation, wiring the source
build into the same opt-in switch is the natural follow-up, so I have left the issue open for that.
Tests
tests/sh/test_resolve_cuda_archs.sh(single / multi / dedup / empty / garbage / whitespace / override /future-arch cases), wired into
tests/run_all.shand thestudio-backend-ci.ymlshell-test loop.Validated cross-platform on a staging runner: ubuntu-latest, macos-14 (bash 3.2) and windows-latest
(git-bash) all green, plus
bash -n studio/setup.shon each.Refs #5854, #5826.