Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
7fa0192
Add ROCm detection to install.sh and expand shell tests
edamamez Mar 31, 2026
f3cc758
Add ROCm torch reinstall support to install_python_stack.py
GoldenGrapeGentleman Mar 31, 2026
062e25f
Add ROCm support to llama.cpp prebuilt installer
edamamez Mar 31, 2026
450f5de
Add IS_ROCM hardware flag and fix AMD error message
GoldenGrapeGentleman Mar 31, 2026
f6c2eb8
Add comprehensive ROCm support test suite (68 tests)
danielhanchen Mar 31, 2026
7290199
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2026
2cb0b52
Harden ROCm support: probe error handling, version cap, validation
danielhanchen Mar 31, 2026
56098e5
Clean up rocm_paths list construction in detect_host()
danielhanchen Mar 31, 2026
9e33c25
Require actual AMD GPU presence before selecting ROCm paths
danielhanchen Mar 31, 2026
4286525
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2026
7d6ac65
Harden hipconfig version parsing and torch probe compatibility
danielhanchen Mar 31, 2026
fd43235
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2026
10ec0cd
Strengthen AMD GPU detection and add NVIDIA precedence guard
danielhanchen Mar 31, 2026
c22312b
Merge branch 'main' into feature/rocm-support-v2
danielhanchen Mar 31, 2026
726fab1
Add Windows-specific ROCm/HIP detection in detect_host()
danielhanchen Mar 31, 2026
f17e007
Add AMD ROCm gaps: Mamba/SSM source builds, GPU monitoring, Windows m…
danielhanchen Mar 31, 2026
1482326
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2026
134638d
Harden ROCm detection, fix VRAM heuristic, and expand RDNA2 coverage
danielhanchen Mar 31, 2026
4dc1dca
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2026
3881539
Add HIP_VISIBLE_DEVICES support, unit-aware VRAM parsing, Windows GPU…
danielhanchen Mar 31, 2026
326a971
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2026
2d55e77
Fix HIP_VISIBLE_DEVICES empty-string handling in GPU visibility spec
danielhanchen Mar 31, 2026
478bc7f
Fix IS_ROCM test assertion for multi-line formatting
danielhanchen Mar 31, 2026
a067110
Cap torchvision/torchaudio versions, remove amdhip64.dll fallback, fi…
danielhanchen Mar 31, 2026
9484014
Merge branch 'main' into feature/rocm-support-v2
danielhanchen Mar 31, 2026
d1e3858
Attribute is_rdna() RDNA2/3/3.5/4 expansion to PR #4428
danielhanchen Apr 1, 2026
d1de729
Support AMD Radeon for studio (#4770)
iswaryaalex Apr 3, 2026
f4d64ff
Merge main into feature/rocm-support-v2, resolve install.sh conflict
danielhanchen Apr 3, 2026
f9a738a
Remove ROCm test files from main PR
danielhanchen Apr 3, 2026
4148591
Fix installer and hardware detection issues for PR #4720
danielhanchen Apr 3, 2026
ae0f9af
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 3, 2026
ec12f9b
Fix GPU detection false positives and add missing health groups
danielhanchen Apr 3, 2026
848c92a
Fix _ensure_rocm_torch and Windows AMD warning false positives
danielhanchen Apr 3, 2026
86735ff
Fix amd-smi GPU detection for GPU[N] output format
danielhanchen Apr 3, 2026
96ba872
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 3, 2026
3caaf30
Harden AMD GPU detection against false positives
danielhanchen Apr 3, 2026
263470e
Remove duplicate comment from pre-commit merge
danielhanchen Apr 3, 2026
1b98f6d
Refactor: deduplicate AMD detection, consolidate bitsandbytes, clean …
danielhanchen Apr 5, 2026
9d7c2e7
Merge main into feature/rocm-support-v2
danielhanchen Apr 5, 2026
b37f7e6
Fix VRAM parsing for string values and GB/GiB consistency
danielhanchen Apr 5, 2026
543e721
Add --no-cache to uv for ROCm HIP source builds
danielhanchen Apr 5, 2026
84a9c55
Fix critical: initialize _amd_gpu_radeon before case block
danielhanchen Apr 5, 2026
c6f5b3a
Fix Windows AMD: route has_rocm hosts to HIP prebuilt path
danielhanchen Apr 5, 2026
810b833
Harden ROCm detection, Radeon wheel fallback, and HIP visibility
danielhanchen Apr 8, 2026
8636fa6
Fix round 2 regressions: ROCm validate_server and Windows HIP routing
danielhanchen Apr 8, 2026
5341e46
Fix round 3 findings: x86_64 guard, ROCm version clip, Radeon deps
danielhanchen Apr 8, 2026
5305c31
Fix round 4 findings: apply_gpu_ids env inheritance, Radeon X.Y, bits…
danielhanchen Apr 8, 2026
7d27b2e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 8, 2026
f98aaef
Fix gemini findings: amd-smi metric envelope validation and dict-wrap…
danielhanchen Apr 8, 2026
37432b6
Fix gemini round 2 findings: explicit length guard on ROCm version fi…
danielhanchen Apr 8, 2026
c12e8b7
Fix gemini round 3: include has_rocm in validate_server fallback path
danielhanchen Apr 8, 2026
d25c570
Fix gemini round 4: remove risky bytes-vs-MB heuristic in _parse_memo…
danielhanchen Apr 8, 2026
b3627bc
Fix gemini round 5: POSIX compliance and leading-comma visibility par…
danielhanchen Apr 8, 2026
5211328
Fix 20-reviewer.py findings: base drift, Radeon %2B, dpkg/rpm fallbac…
danielhanchen Apr 9, 2026
1d387d6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 9, 2026
7effb3a
Fix gemini round 6 + URL audit: amd.py defensive checks, rocm6.5+ cli…
danielhanchen Apr 9, 2026
bae2421
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 9, 2026
a24b27e
Fix reviewer.py round 2: tokenizer AMD multi-GPU, --no-torch bnb, mai…
danielhanchen Apr 9, 2026
659c19c
Merge branch 'main' into feature/rocm-support-v2
danielhanchen Apr 9, 2026
83567f5
fix: prevent bitsandbytes from overwriting ROCm torch with CUDA wheels
danielhanchen Apr 9, 2026
7cbc51d
fix: ROCm inference fallback -- skip Unsloth patching and bnb 4-bit o…
danielhanchen Apr 9, 2026
cb7dbd4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 9, 2026
e54ed86
Merge branch 'main' into feature/rocm-support-v2
danielhanchen Apr 9, 2026
98f0215
fix: guard audio/vision inference on ROCm, remove unused import
danielhanchen Apr 9, 2026
4a87946
fix: amd-smi parsing for newer output format (gpu_data wrapper, mem_u…
danielhanchen Apr 9, 2026
1c58bb9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 9, 2026
9e98390
Bug fix detecting radeon (#4940)
iswaryaalex Apr 10, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 66 additions & 2 deletions install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,58 @@ get_torch_index_url() {
elif [ -x "/usr/bin/nvidia-smi" ]; then
_smi="/usr/bin/nvidia-smi"
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Probe NVIDIA usability before skipping ROCm detection

This branch only checks whether nvidia-smi exists, not whether a usable NVIDIA GPU is present. On ROCm hosts that still have nvidia-smi on PATH (stale driver packages or mixed tool images), the function bypasses ROCm probing and later defaults to a CUDA wheel index when CUDA parsing fails, which breaks AMD installs instead of using the ROCm index. Gate the NVIDIA path on a successful device probe before short-circuiting the ROCm fallback.

Useful? React with 👍 / 👎.

if [ -z "$_smi" ]; then echo "$_base/cpu"; return; fi
if [ -z "$_smi" ]; then
# No NVIDIA GPU -- check for AMD ROCm GPU
# First confirm an actual AMD GPU is present (not just ROCm tools installed)
_has_rocm_gpu=false
if command -v rocminfo >/dev/null 2>&1 && \
rocminfo 2>/dev/null | awk '/Name:[[:space:]]*gfx[0-9]/{found=1} END{exit !found}'; then
_has_rocm_gpu=true
elif command -v amd-smi >/dev/null 2>&1 && \
amd-smi list 2>/dev/null | awk 'NR>1 && NF{found=1} END{exit !found}'; then
_has_rocm_gpu=true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require concrete GPU rows from amd-smi list

In get_torch_index_url(), the AMD probe treats any non-empty line after the first from amd-smi list as proof of a GPU. Informational/header output (including common "no GPU" messages on line 2+) can satisfy NR>1 && NF, so CPU-only hosts with ROCm tools can be misclassified as AMD GPU machines. In that case the installer selects a ROCm wheel index instead of CPU and can fail dependency installation on non-AMD systems.

Useful? React with 👍 / 👎.

fi
if [ "$_has_rocm_gpu" != true ]; then
echo "$_base/cpu"; return
fi
# AMD GPU confirmed -- detect ROCm version
_rocm_tag=""
_rocm_tag=$({ command -v amd-smi >/dev/null 2>&1 && \
amd-smi version 2>/dev/null | awk -F'ROCm version: ' \
'NF>1{gsub(/[^0-9.]/, "", $2); split($2,a,"."); print "rocm"a[1]"."a[2]; ok=1; exit} END{exit !ok}'; } || \
Comment on lines +1047 to +1048

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Continue ROCm fallback when amd-smi version is malformed

The amd-smi probe currently returns success as soon as it sees a ROCm version: field, even when the extracted version is empty/non-numeric (for example N/A), so the || fallback chain never reaches /opt/rocm/.info/version, hipconfig, or package metadata. In that case _rocm_tag is later cleared by validation and the installer incorrectly falls back to CPU on ROCm hosts that have other valid version sources.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The awk script for parsing amd-smi version might fail if the version string contains unexpected characters or if the split results in fewer than two elements. While the validation guard at line 1017 handles malformed tags, ensuring that a[1] and a[2] are populated before printing would be more robust.

Comment on lines +1046 to +1048

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The ROCm version parsing from amd-smi version can be simplified and made more robust by using a character class in split to handle both dots and dashes, and by providing a default for the minor version to avoid malformed tags like rocm6. which would be rejected by the validation guard at line 1062.

Suggested change
_rocm_tag=$({ command -v amd-smi >/dev/null 2>&1 && \
amd-smi version 2>/dev/null | awk -F'ROCm version: ' \
'NF>1{gsub(/[^0-9.]/, "", $2); split($2,a,"."); print "rocm"a[1]"."a[2]; ok=1; exit} END{exit !ok}'; } || \
_rocm_tag=$({ command -v amd-smi >/dev/null 2>&1 && \
amd-smi version 2>/dev/null | awk -F'ROCm version: ' \
'NF>1{split($2,a,"[.-]"); print "rocm"a[1]"."(a[2]==""?"0":a[2]); ok=1; exit} END{exit !ok}'; } || \

{ [ -r /opt/rocm/.info/version ] && \
awk -F. '{print "rocm"$1"."$2; exit}' /opt/rocm/.info/version; } || \
Comment on lines +1046 to +1050

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prevent ROCm version probe from aborting install.sh

Because install.sh runs with set -e, this _rocm_tag=$(...) assignment will terminate the script when every version probe in the || chain fails. That happens on real ROCm hosts when GPU detection succeeds (via rocminfo) but version sources are unavailable/unreadable (for example custom ROCm layouts where /opt/rocm/.info/version, hipconfig, and package metadata are missing). In that case the installer exits before reaching the intended CPU fallback path.

Useful? React with 👍 / 👎.

{ command -v hipconfig >/dev/null 2>&1 && \
hipconfig --version 2>/dev/null | awk 'NR==1{split($1,a,"."); if(a[1]+0>0) print "rocm"a[1]"."a[2]}'; } || \
{ command -v dpkg-query >/dev/null 2>&1 && \
ver="$(dpkg-query -W -f='${Version}\n' rocm-core 2>/dev/null)" && \
[ -n "$ver" ] && \
printf '%s\n' "$ver" | sed 's/^[0-9]*://' | awk -F'[.-]' '{print "rocm"$1"."$2; exit}'; } || \
{ command -v rpm >/dev/null 2>&1 && \
ver="$(rpm -q --qf '%{VERSION}\n' rocm-core 2>/dev/null)" && \
[ -n "$ver" ] && \
printf '%s\n' "$ver" | awk -F'[.-]' '{print "rocm"$1"."$2; exit}'; }) 2>/dev/null
Comment on lines +1046 to +1060

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The command substitution for _rocm_tag is complex and fragile. If amd-smi is not installed, it falls back to several other methods. It is better to extract this detection logic into a dedicated function to improve readability and maintainability.

# Validate _rocm_tag: must match "rocmX.Y" with major >= 1
case "$_rocm_tag" in
rocm[1-9]*.[0-9]*) : ;; # valid (major >= 1)
*) _rocm_tag="" ;; # reject malformed (empty, garbled, or major=0)
esac
if [ -n "$_rocm_tag" ]; then
# ROCm 7.2 only has torch 2.11.0 which exceeds current bounds (<2.11.0).
# Fall back to rocm7.1 index which has torch 2.10.0.
# TODO: uncomment the next line when torch upper bound is bumped to >=2.11.0
# echo "$_base/$_rocm_tag"; return
case "$_rocm_tag" in
rocm6.*|rocm7.0*|rocm7.1*)
echo "$_base/$_rocm_tag" ;;
*)
# ROCm 7.2+ (including future 10.x+): cap to rocm7.1
echo "$_base/rocm7.1" ;;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict ROCm tag remap to supported versions

This remap logic treats every detected ROCm tag outside rocm6.*, rocm7.0*, and rocm7.1* as rocm7.1, so older stacks like rocm5.x are incorrectly forced onto the 7.1 wheel index; at the same time, rocm7.1* also matches rocm7.10, which emits a likely non-existent .../rocm7.10 index. In both cases the installer can request incompatible/unavailable wheels and fail instead of falling back cleanly. The mapping should only pass through explicitly supported tags and only cap versions >= 7.2 to rocm7.1.

Useful? React with 👍 / 👎.

esac
return
fi
echo "$_base/cpu"; return
fi
# Parse CUDA version from nvidia-smi output (POSIX-safe, no grep -P)
_cuda_ver=$(LC_ALL=C $_smi 2>/dev/null \
| sed -n 's/.*CUDA Version:[[:space:]]*\([0-9][0-9]*\.[0-9][0-9]*\).*/\1/p' \
Expand All @@ -1007,13 +1058,19 @@ case "$TORCH_INDEX_URL" in
*/cpu)
if [ "$SKIP_TORCH" = false ] && [ "$OS" != "macos" ]; then
echo ""
echo " NOTE: No NVIDIA GPU detected (nvidia-smi not found)."
echo " NOTE: No GPU detected (nvidia-smi and ROCm not found)."
echo " Installing CPU-only PyTorch. If you only need GGUF chat/inference,"
echo " re-run with --no-torch for a faster, lighter install:"
echo " curl -fsSL https://unsloth.ai/install.sh | sh -s -- --no-torch"
echo " AMD ROCm users: see https://docs.unsloth.ai/get-started/install-and-update/amd"
echo ""
fi
;;
*/rocm*)
echo ""
echo " AMD ROCm detected -- installing ROCm-enabled PyTorch ($TORCH_INDEX_URL)"
echo ""
;;
esac

# ── Install unsloth directly into the venv (no activation needed) ──
Expand Down Expand Up @@ -1051,6 +1108,13 @@ elif [ -n "$TORCH_INDEX_URL" ]; then
substep "installing PyTorch ($TORCH_INDEX_URL)..."
run_install_cmd "install PyTorch" uv pip install --python "$_VENV_PY" "torch>=2.4,<2.11.0" torchvision torchaudio \
--index-url "$TORCH_INDEX_URL"
# AMD ROCm: install bitsandbytes with AMD support
case "$TORCH_INDEX_URL" in
*/rocm*)
substep "installing bitsandbytes for AMD ROCm..."
run_install_cmd "install bitsandbytes (AMD)" uv pip install --python "$_VENV_PY" "bitsandbytes>=0.49.1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The version constraint bitsandbytes>=0.49.1 appears to be a typo. As of current releases, bitsandbytes is in the 0.45.x range. ROCm support was officially added in 0.44.0. Using a non-existent version will cause the installation to fail. Please verify the intended version (likely 0.45.1 or similar).

Suggested change
run_install_cmd "install bitsandbytes (AMD)" uv pip install --python "$_VENV_PY" "bitsandbytes>=0.49.1"
run_install_cmd "install bitsandbytes (AMD)" uv pip install --python "$_VENV_PY" "bitsandbytes>=0.45.0"

;;
esac
fi
# Fresh: Step 2 - install unsloth, preserving pre-installed torch
substep "installing unsloth (this may take a few minutes)..."
Expand Down
2 changes: 2 additions & 0 deletions studio/backend/utils/hardware/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
DeviceType,
DEVICE,
CHAT_ONLY,
IS_ROCM,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Export IS_ROCM as a live value

IS_ROCM is imported by value from hardware.py, so utils.hardware.IS_ROCM becomes a one-time snapshot (typically False at import time). After detect_hardware() reassigns hardware.IS_ROCM, callers that read utils.hardware.IS_ROCM (or did from utils.hardware import IS_ROCM) still see the stale value, which can disable ROCm-specific logic on HIP hosts. Expose this flag via a live accessor/module attribute instead of binding the boolean in __init__.py.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Export IS_ROCM as a live value

IS_ROCM is imported by value from hardware.py, so utils.hardware.IS_ROCM becomes a one-time snapshot (typically False at import time). After detect_hardware() reassigns hardware.IS_ROCM, callers that read utils.hardware.IS_ROCM (or did from utils.hardware import IS_ROCM) still see the stale value, which can disable ROCm-specific logic on HIP hosts. Expose this flag via a live accessor/module attribute instead of binding the boolean in __init__.py.

Useful? React with 👍 / 👎.

detect_hardware,
get_device,
is_apple_silicon,
Expand Down Expand Up @@ -49,6 +50,7 @@
"DeviceType",
"DEVICE",
"CHAT_ONLY",
"IS_ROCM",
"detect_hardware",
"get_device",
"is_apple_silicon",
Expand Down
23 changes: 18 additions & 5 deletions studio/backend/utils/hardware/hardware.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class DeviceType(str, Enum):

DEVICE: Optional[DeviceType] = None
CHAT_ONLY: bool = True # No CUDA GPU -> GGUF chat only (Mac, CPU-only, etc.)
IS_ROCM: bool = False # True when running on AMD ROCm (HIP) -- display/logging only


# ========== Detection ==========
Expand Down Expand Up @@ -85,18 +86,28 @@ def detect_hardware() -> DeviceType:
2. MLX (Apple Silicon via MLX framework)
3. CPU (fallback)
"""
global DEVICE, CHAT_ONLY
CHAT_ONLY = True # reset -- only CUDA sets it to False
global DEVICE, CHAT_ONLY, IS_ROCM
CHAT_ONLY = True # reset -- only CUDA/ROCm sets it to False
IS_ROCM = False

# --- CUDA: try PyTorch ---
# --- CUDA / ROCm: try PyTorch ---
if _has_torch():
import torch

if torch.cuda.is_available():
DEVICE = DeviceType.CUDA
CHAT_ONLY = False
device_name = torch.cuda.get_device_properties(0).name
print(f"Hardware detected: CUDA — {device_name}")

# Distinguish AMD ROCm (HIP) from NVIDIA CUDA for display purposes.
# DeviceType stays CUDA since torch.cuda.* works on ROCm via HIP.
if getattr(torch.version, "hip", None) is not None:
IS_ROCM = True
print(
f"Hardware detected: ROCm (HIP {torch.version.hip}) -- {device_name}"
)
else:
print(f"Hardware detected: CUDA -- {device_name}")
return DEVICE

# --- XPU: Intel GPU ---
Expand Down Expand Up @@ -315,13 +326,15 @@ def get_package_versions() -> Dict[str, Optional[str]]:
except PackageNotFoundError:
versions[name] = None

# CUDA toolkit version bundled with torch
# GPU runtime version bundled with torch
try:
import torch

versions["cuda"] = getattr(torch.version, "cuda", None)
versions["rocm"] = getattr(torch.version, "hip", None)
except Exception:
versions["cuda"] = None
versions["rocm"] = None

return versions

Expand Down
71 changes: 69 additions & 2 deletions studio/install_llama_prebuilt.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class HostInfo:
visible_cuda_devices: str | None
has_physical_nvidia: bool
has_usable_nvidia: bool
has_rocm: bool = False


@dataclass
Expand Down Expand Up @@ -1430,6 +1431,25 @@ def detect_host() -> HostInfo:
except Exception:
pass
Comment on lines 2567 to 2568

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Broad exception handler detected. Per general rules, it is recommended to log the exception or add a descriptive comment instead of a silent pass, even for hardware probing logic.

References
  1. Avoid using broad, silent exception handlers like except Exception: pass. Instead, log the exception, even if at a debug level, to aid in future debugging.


# Detect AMD ROCm (HIP) -- require actual GPU, not just tools installed
has_rocm = False
if not is_macos:
for _cmd, _marker in (
(["rocminfo"], "gfx"),
(["amd-smi", "list"], "gpu"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require concrete AMD device markers in ROCm probe

The new ROCm detector uses ("amd-smi", "list") with marker "gpu" and then accepts any output containing that substring, which can misclassify non-AMD hosts because informational text (for example messages like “No GPUs found” or header-only output) still includes gpu. In environments without usable NVIDIA GPUs, that false positive routes selection toward HIP/ROCm prebuilts instead of CPU, leading to unnecessary validation failures and source-build fallback behavior.

Useful? React with 👍 / 👎.

):
_exe = shutil.which(_cmd[0])
if not _exe:
continue
try:
_result = run_capture([_exe, *_cmd[1:]], timeout = 10)
except Exception:
continue
if _result.returncode == 0 and _result.stdout.strip():
if _marker in _result.stdout.lower():
has_rocm = True
break
Comment on lines +2576 to +2593

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The ROCm detection logic in detect_host is likely to fail on native Windows systems. It relies on rocminfo (which is Linux-only) and amd-smi (which is often not in the system PATH on Windows). To improve detection on Windows, consider checking for the presence of amdhip64.dll in standard locations (e.g., C:\Windows\System32 or the ROCm installation directory) or using wmic to query the GPU vendor.


Comment on lines +2570 to +2613

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for detecting an AMD ROCm GPU on both Linux and Windows is duplicated in studio/install_python_stack.py (in _has_rocm_gpu and install_python_stack functions). This code repetition can lead to maintenance issues if the detection logic needs to be updated in the future.

Consider creating a shared utility module with a function like has_amd_gpu(platform: str) -> bool that both scripts can import and use. This would centralize the detection logic, ensuring consistency and making future updates easier. Since both scripts are part of the studio package, a shared utility seems appropriate.

return HostInfo(
system = system,
machine = machine,
Expand All @@ -1444,6 +1464,7 @@ def detect_host() -> HostInfo:
visible_cuda_devices = visible_cuda_devices,
has_physical_nvidia = has_physical_nvidia,
has_usable_nvidia = has_usable_nvidia,
has_rocm = has_rocm,
)


Expand Down Expand Up @@ -1724,6 +1745,32 @@ def resolve_linux_cuda_choice(
def resolve_upstream_asset_choice(host: HostInfo, llama_tag: str) -> AssetChoice:
upstream_assets = github_release_assets(UPSTREAM_REPO, llama_tag)
if host.is_linux and host.is_x86_64:
# AMD ROCm: try upstream ROCm prebuilt first, then fall back to source build.
# Source build (via setup.sh) compiles with -DGGML_HIP=ON and auto-detects
# the exact GPU target via rocminfo, which is more reliable for consumer
# GPUs (e.g. gfx1151) that may not be in the prebuilt.
if host.has_rocm and not host.has_usable_nvidia:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate ROCm branch on verified NVIDIA availability

This new guard blocks the ROCm path whenever host.has_usable_nvidia is true, but in detect_host() that flag can be set by nvidia-smi presence/banner without confirming an actual usable NVIDIA device. On AMD ROCm systems with stale NVIDIA tooling, this prevents ROCm asset selection and diverts installs away from HIP support. The guard should rely on a real NVIDIA device probe before suppressing the ROCm branch.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wire Linux ROCm selection into simple-policy flow

On Linux x86_64 AMD hosts without usable NVIDIA, this new ROCm branch is not reached in the default installer path because studio/setup.sh invokes install_llama_prebuilt.py with --simple-policy (lines 585-590), and resolve_simple_install_release_plans() routes unslothai/llama.cpp releases through direct_linux_release_plan(), which only appends CUDA or CPU attempts and never checks host.has_rocm. In practice that means normal setup still installs the CPU prebuilt on ROCm machines, so the added ROCm prebuilt/source-fallback logic here does not activate for production installs.

Useful? React with 👍 / 👎.

rocm_name = f"llama-{llama_tag}-bin-ubuntu-rocm-7.2-x64.tar.gz"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The ROCm version 7.2 is hardcoded in the prebuilt asset name. While the code includes a safe fallback to source builds if this specific version is missing or incompatible, hardcoding it limits the ability to automatically pick up newer prebuilt releases (e.g., ROCm 7.3) without manual code changes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The ROCm version 7.2 is hardcoded in the asset filename. This will cause prebuilt resolution to fail for any llama.cpp releases compiled against different ROCm versions (e.g., 7.1 or 6.2), forcing an unnecessary source build. Consider scanning the upstream_assets keys for a pattern matching rocm-*-x64.tar.gz to dynamically select the available ROCm prebuilt.

if rocm_name in upstream_assets:
log(f"AMD ROCm detected -- trying upstream prebuilt {rocm_name}")
log(
"Note: prebuilt is compiled for ROCm 7.2; if your ROCm version differs, "
"this may fail preflight and fall back to a source build (safe)"
)
return AssetChoice(
repo = UPSTREAM_REPO,
tag = llama_tag,
name = rocm_name,
url = upstream_assets[rocm_name],
source_label = "upstream",
install_kind = "linux-rocm",
)
# No ROCm prebuilt available -- fall back to source build
raise PrebuiltFallback(
"AMD ROCm detected but no upstream ROCm prebuilt found; "
"falling back to source build with HIP support"
)

upstream_name = f"llama-{llama_tag}-bin-ubuntu-x64.tar.gz"
if upstream_name not in upstream_assets:
raise PrebuiltFallback("upstream Linux CPU asset was not found")
Expand All @@ -1743,6 +1790,25 @@ def resolve_upstream_asset_choice(host: HostInfo, llama_tag: str) -> AssetChoice
return attempts[0]
raise PrebuiltFallback("no compatible Windows CUDA asset was found")

# AMD ROCm on Windows: try HIP prebuilt
if host.has_rocm:
hip_name = f"llama-{llama_tag}-bin-win-hip-radeon-x64.zip"
if hip_name in upstream_assets:
log(
Comment on lines +3175 to +3179

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Route Windows ROCm hosts through HIP candidate selection

The new host.has_rocm HIP branch in resolve_upstream_asset_choice() is effectively unreachable in normal installs because resolve_install_release_plans() routes Windows hosts through resolve_release_asset_choice(), and that function still picks published_asset_choice_for_kind(..., "windows-cpu") first for non-NVIDIA machines. When a published CPU artifact exists (the common case), AMD ROCm Windows hosts are locked to CPU bundles and never attempt the HIP prebuilt, so this commit’s Windows ROCm support does not actually activate.

Useful? React with 👍 / 👎.

Comment on lines +3176 to +3179

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wire HIP selection into simple-policy release planning

This new Windows HIP branch is bypassed on the default installer path: studio/setup.ps1 still invokes install_llama_prebuilt.py with --simple-policy (lines 1741-1746), and simple-policy resolution goes through resolve_simple_install_release_plans()direct_upstream_release_plan(), which only emits CUDA/CPU attempts for Windows and never a windows-hip attempt. As a result, AMD ROCm Windows hosts continue to get CPU bundles even though HIP support was added here.

Useful? React with 👍 / 👎.

f"AMD ROCm detected on Windows -- trying upstream HIP prebuilt {hip_name}"
)
return AssetChoice(
repo = UPSTREAM_REPO,
tag = llama_tag,
name = hip_name,
url = upstream_assets[hip_name],
source_label = "upstream",
install_kind = "windows-hip",
)
log(
"AMD ROCm detected on Windows but no HIP prebuilt found -- falling back to CPU"
)

upstream_name = f"llama-{llama_tag}-bin-win-cpu-x64.zip"
if upstream_name not in upstream_assets:
raise PrebuiltFallback("upstream Windows CPU asset was not found")
Expand Down Expand Up @@ -2121,7 +2187,7 @@ def overlay_directory_for_choice(


def runtime_patterns_for_choice(choice: AssetChoice) -> list[str]:
if choice.install_kind in {"linux-cpu", "linux-cuda"}:
if choice.install_kind in {"linux-cpu", "linux-cuda", "linux-rocm"}:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add ROCm runtime health checks before reusing cached install

This change introduces the linux-rocm install kind, but runtime_payload_health_groups() still has no branch for it, so it returns an empty group list and runtime_payload_is_healthy() treats the runtime as healthy without validating HIP libraries. In practice, a partially broken cached ROCm install (for example missing libggml-hip.so) can be incorrectly accepted and skipped, leading to later runtime failures instead of a reinstall.

Useful? React with 👍 / 👎.

return [
"llama-server",
"llama-quantize",
Expand All @@ -2131,11 +2197,12 @@ def runtime_patterns_for_choice(choice: AssetChoice) -> list[str]:
"libmtmd.so*",
"libggml-cpu-*.so*",
"libggml-cuda.so*",
"libggml-hip.so*",
"libggml-rpc.so*",
]
if choice.install_kind in {"macos-arm64", "macos-x64"}:
return ["llama-server", "llama-quantize", "lib*.dylib"]
if choice.install_kind in {"windows-cpu", "windows-cuda"}:
if choice.install_kind in {"windows-cpu", "windows-cuda", "windows-hip"}:
return ["*.exe", "*.dll"]
raise PrebuiltFallback(
f"unsupported install kind for runtime overlay: {choice.install_kind}"
Expand Down
Loading
Loading