From ed41a254398bffbf721d0d9dd0b0c2bd33d52e76 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 12 Apr 2026 22:52:35 +0000 Subject: [PATCH] Trim verbose review-fix comments Shorten multi-line comments added during review iterations to 1-2 lines each. Remove redundant explanations where the code is self-evident. --- install.ps1 | 4 +--- studio/install_python_stack.py | 17 ++++++----------- studio/setup.ps1 | 31 +++++++++---------------------- 3 files changed, 16 insertions(+), 36 deletions(-) diff --git a/install.ps1 b/install.ps1 index c1dbb2ddbd..cebe7069c0 100644 --- a/install.ps1 +++ b/install.ps1 @@ -784,9 +784,7 @@ shell.Run cmd, 0, False # ── Install Python if no compatible version found ── # Find-CompatiblePython returns @{ Version = "3.13"; Path = "C:\...\python.exe" } or $null. - # AMD + torch path requires Python 3.12 specifically (Radeon only publishes - # cp312 wheels for Windows). AMD + --no-torch (GGUF-only) is fine with any - # 3.11-3.13, since the cp312 constraint only applies to the ROCm wheels. + # AMD + torch requires Python 3.12 (Radeon wheels are cp312 only). if ($HasAmdGpu -and -not $SkipTorch) { $PythonPreferred = @("3.12") $PythonVersion = "3.12" diff --git a/studio/install_python_stack.py b/studio/install_python_stack.py index a764b0f851..d84bc5a064 100644 --- a/studio/install_python_stack.py +++ b/studio/install_python_stack.py @@ -438,10 +438,8 @@ def _has_usable_nvidia_gpu() -> bool: """Return True only when nvidia-smi exists AND reports at least one GPU.""" exe = shutil.which("nvidia-smi") if not exe and IS_WINDOWS: - # nvidia-smi.exe is often absent from PATH on Windows even with a - # valid driver. Match the fallback paths used in install.ps1 / - # setup.ps1 so the NVIDIA-wins-on-mixed-systems rule is consistent - # between the PowerShell and Python install paths. + # nvidia-smi.exe is often not on PATH on Windows; match the + # fallback paths in install.ps1 / setup.ps1. _candidates = [ os.path.join( os.environ.get("ProgramFiles", r"C:\Program Files"), @@ -498,10 +496,8 @@ def _ensure_rocm_torch_windows() -> None: troubleshooting notes flag pip dep-resolver overwrite scenarios on this procedure. """ - # Cheap idempotency probe first -- no subprocess spawn needed when - # torch already links against ROCm (common at step 13 and on updates). - # Placed before the expensive GPU-detection calls so the happy-path - # (ROCm already installed) avoids two subprocess spawns entirely. + # Cheap idempotency probe first -- skip GPU detection when torch + # already links against ROCm (common at step 13 and on updates). try: _probe = subprocess.run( [ @@ -525,9 +521,8 @@ def _ensure_rocm_torch_windows() -> None: if not _has_rocm_gpu_windows(): return - # Radeon wheels are cp312 only. Hard-exit so the caller (setup.ps1 or - # install.ps1) sees a non-zero exit code instead of continuing with a - # CPU-only torch that silently reports success. + # Radeon wheels are cp312 only. Hard-exit so the caller sees a + # non-zero code instead of silently leaving CPU-only torch. if (sys.version_info.major, sys.version_info.minor) != (3, 12): _safe_print( _red( diff --git a/studio/setup.ps1 b/studio/setup.ps1 index 3290f0399d..8be29cb727 100644 --- a/studio/setup.ps1 +++ b/studio/setup.ps1 @@ -1510,14 +1510,9 @@ if (Test-Path $VenvDir -PathType Container) { $shouldRebuild = $true } - # --no-torch: skip ROCm torch repair and wheel install entirely. - # Must be defined here (before stale-venv detection) because the - # expectedTorchTag and $_NeedRocmRepair logic below depend on it. + # Must be defined before stale-venv detection (expectedTorchTag depends on it). $_NoTorch = $env:UNSLOTH_NO_TORCH -in @("1", "true", "True", "TRUE") - - # $_NeedRocmRepair is used later to override the version fast-path - # ($SkipPythonDeps) so the ROCm install block still runs even when - # the unsloth package is already at the latest version. + # Overrides $SkipPythonDeps so the ROCm block runs even when unsloth is current. $_NeedRocmRepair = $false if (-not $shouldRebuild) { @@ -1529,12 +1524,9 @@ if (Test-Path $VenvDir -PathType Container) { $expectedTorchTag = "cpu" } if ($installedTorchTag -and $installedTorchTag -ne $expectedTorchTag) { - # Existing Windows AMD users commonly have a CPU-only venv from - # pre-ROCm installs. Rebuilding from scratch would delete the - # venv and then exit with "Run install.ps1 first" because - # setup.ps1 cannot recreate the venv on its own. Instead, keep - # the venv and let the ROCm install block below repair torch - # in-place -- the end state is the same but no work is lost. + # Keep the venv and let the ROCm block below repair torch + # in-place; rebuilding would delete it and exit since + # setup.ps1 cannot recreate a venv on its own. if ($HasAmdGpu -and -not $_NoTorch -and $installedTorchTag -eq "cpu") { substep "CPU-only torch detected on AMD host; will repair to ROCm in place..." "Yellow" $_NeedRocmRepair = $true @@ -1671,8 +1663,7 @@ $env:TORCHINDUCTOR_CACHE_DIR = $TorchCacheDir [Environment]::SetEnvironmentVariable('TORCHINDUCTOR_CACHE_DIR', $TorchCacheDir, 'User') substep "TORCHINDUCTOR_CACHE_DIR set to $TorchCacheDir (avoids MAX_PATH issues)" -# $_NoTorch was already initialized earlier (before stale-venv detection) -# so it is available here for the $CuTag selection. +# $_NoTorch initialized earlier (before stale-venv detection). if ($HasNvidiaSmi) { $CuTag = Get-PytorchCudaTag @@ -1707,9 +1698,8 @@ if ($CuTag -eq "rocm") { exit 1 } - # Radeon wheels are cp312 only. Hard-stop when the venv's Python is a - # different minor version -- continuing would download multi-GB wheels - # that pip will reject with a confusing incompatible-wheel error. + # Radeon wheels are cp312 only. Hard-stop instead of downloading + # multi-GB wheels that pip will reject. try { $venvPyVer = (& python -c "import sys; print(f'{sys.version_info.major}.{sys.version_info.minor}')" 2>$null | Out-String).Trim() } catch { $venvPyVer = "" } @@ -1724,10 +1714,7 @@ if ($CuTag -eq "rocm") { exit 1 } - # Skip the expensive reinstall when ROCm torch is already healthy. - # Mirrors the idempotency guard in _ensure_rocm_torch_windows() -- - # without this, fresh installs (install.ps1 -> setup.ps1) and every - # `unsloth studio update` would re-download 2.1-3.9 GB unnecessarily. + # Skip reinstall when ROCm torch is already healthy (idempotency guard). $_existingHip = "" try { $_existingHip = (& python -c "import torch; print(getattr(torch.version,'hip','') or '')" 2>$null | Out-String).Trim()