[staging CI] unslothai/unsloth#8461 - #265
Open
danielhanchen wants to merge 10 commits into
Open
Conversation
…he installer
setup.ps1's WMI fallback built its adapter list with
$wmiGpus = if ($healthyGpus.Count -gt 0) { $healthyGpus } else { $amdGpus }
An unwrapped one-element branch unrolls to a bare WMI object on its way out of the
if, and a bare WMI object has no .Count in PS 5.1 (a string or hashtable does), so
`if ($wmiGpus.Count -gt 0)` never fired on a host with exactly one AMD adapter and
$ROCmGpuLabel stayed null. Setup reported "gpu none (chat-only / GGUF)", skipped the
name to gfx inference, and the stale-venv check then expected cpu torch against the
ROCm wheels install.ps1 had just placed:
Stale venv detected (torch rocm != required cpu).
[ERROR] The existing Unsloth environment needs repair.
The installer rolled back and the desktop app retried the same failure indefinitely.
Reproduced on a Radeon 8060S (gfx1151) Strix Halo laptop, where install.ps1 resolved
gfx1151 and setup.ps1 one second later saw no GPU at all.
Wrap the whole if in @(), the idiom the Intel scan below already documents.
install.ps1 also now exports the arch it resolved before invoking setup.ps1, so the
two never re-derive it independently. Setting UNSLOTH_ROCM_GFX_ARCH by hand was the
workaround for this bug, and doing it in the installer keeps any future divergence
between the two scans from turning into an unrecoverable rollback loop.
…g bug in the name list
Follow-up to the single-AMD-GPU scan fix in this branch, from reviewing what it
could do to hosts other than the one it was reported on.
install.ps1 forwarded its resolved arch as UNSLOTH_ROCM_GFX_ARCH. That name is
the documented operator override, and install_llama_prebuilt.py reads it back as
_manual to decide whether a forwarded --rocm-gfx outranks its own probe, so
publishing an auto-detected value there disarmed that safeguard. install.ps1's
scan is also the weaker of the two: it takes the first AMD adapter with no
visible-device mask and no shadowing-iGPU repick, both of which setup.ps1
applies. On a 780M + RX 9070 XT host setup resolves gfx1201 today; the forward
made it take the installer's gfx1103 verbatim and hand llama.cpp the iGPU
bundle. It was also never restored, so it outlived the install in the caller's
shell on the documented irm | iex path.
It now travels as _UNSLOTH_ROCM_GFX_ARCH_HANDOFF, matching the _UNSLOTH_ prefix
the neighbouring handoffs use, saved and restored in the same finally block, and
consumed by setup.ps1 only after its own probes and inference come up empty.
setup.ps1's gpu name list had the same unwrapped if as the adapter scan: it
wraps each branch but not the if, so a single adapter name unrolls to a bare
String and $gpuNames[$nameIdx] indexes the name and yields "A". The
$nameArches[0] rescue covers that unless a visible-device mask is set, so a
pinned single-GPU host still inferred no arch and looped the same way. Audited
every '= if (' site across the .ps1 files; this was the only other one.
The tests asserted on .Count, which pwsh answers as 1 for a scalar because
PowerShell/PowerShell#5745 added that fallback in 6.1 and Windows PowerShell 5.1
never got it, so they passed against the unfixed source and guarded nothing.
They now assert the shape of the value, re-run each case against stubs carrying
Count = $null to reproduce 5.1's behaviour, cover the mask and multi-adapter
paths and the handoff lifecycle, and pin their own failure against the merge
base.
for more information, see https://pre-commit.ci
The check resolved its pre-fix source through git merge-base against main. That holds only until this merges: after it, the merge base is a commit that already carries the fix, so the fixed source goes in as the before case and the three assertions fail on every host with pwsh. Reaching for an older revision at all also breaks in a shallow CI clone. It now undoes just the two @() wraps in the shipped source in memory, which is immutable, needs no git, and isolates the one thing under test since everything else about the two sources is identical by construction. Verified both ways: fed the fixed source in as before, the old assertion fails exactly as predicted; with the wraps undone it reports scalar and no label as it should.
The inference above deliberately leaves $pickedName unset when a mask is set and the selected adapter's name is not in the table, rather than borrowing a peer's arch. The handoff then took it anyway: install.ps1 scans without the masks and forwards the FIRST recognized adapter, so a host masking an unknown discrete card while a 780M is listed first resolved gfx1103 and installed wheels and prebuilts for the iGPU the mask hides. ROCR_VISIBLE_DEVICES filters below HIP, so masked devices never reach the runtime's enumeration at all, which makes targeting one strictly wrong rather than merely suboptimal. Setup now skips the handoff whenever any of the three masks is set, matching what its own inference and Resolve-ShadowingGfxPick already do. UNSLOTH_ROCM_GFX_ARCH stays the escape hatch and still wins. Confirmed both ways: with the guard removed the masked host resolves gfx1103, with it in place it resolves nothing, and the unmasked gap-filling case the handoff exists for is unchanged.
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: f03c9c5382
ℹ️ 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".
Comment on lines
+2445
to
+2446
| if (-not $script:ROCmGfxArch -and $env:_UNSLOTH_ROCM_GFX_ARCH_HANDOFF -and | ||
| -not (Test-VisibleDevicesPinned)) { |
There was a problem hiding this comment.
When setup's probes fail after install.ps1 successfully resolved a HIP/ROCR-pinned GPU, this blanket mask check discards the only correct arch and recreates the ROCm-versus-CPU mismatch and rollback loop the handoff is meant to prevent. The premise that the installer always ignores masks is false: its hipinfo and amd-smi paths select using HIP_VISIBLE_DEVICES/ROCR_VISIBLE_DEVICES at install.ps1:3239-3242 and install.ps1:3279-3284 before forwarding the result. Preserve the handoff's provenance or allow values produced by those mask-aware paths instead of suppressing every masked handoff.
Useful? React with 👍 / 👎.
install.ps1 took the first AMD adapter WMI listed with no health check, while setup.ps1 filters on ConfigManagerErrorCode. A disabled Radeon listed ahead of a healthy unsupported one therefore resolved that dead card's arch. Because a mapped arch installs ROCm wheels right there, the machine got wheels for a GPU it cannot use while the live card went unserved, and setup, which discards that adapter, disagreed and took the forwarded arch as its last resort. Fixed at the source rather than by teaching setup to distrust the handoff: rejecting it there would leave setup expecting cpu torch against the ROCm wheels install.ps1 had already placed, which is the stale-venv rollback loop this branch exists to end. Filtering here means the two scans start from the same healthy set, so a forwarded arch can only ever name an adapter setup also kept. Keeps setup's fallback for the case where the filter empties the list, since code 45 is routine on a muxless laptop with a parked dGPU. Confirmed both ways: before, the disabled card resolves gfx1201; after, the host resolves nothing and lands on CPU exactly as setup does.
danielhanchen
force-pushed
the
pr-8461-xplat-ci
branch
from
August 11, 2026 18:49
3ebf2b9 to
d96032e
Compare
for more information, see https://pre-commit.ci
#8398 landed the same $wmiGpus @() wrap from the same report, so the only conflict is the comment above an identical line. Took main's, which is the merged wording and cites #8335. What is left here on top of main: the second unwrapped if a block down ($gpuNames, still unwrapped on main), the ConfigManagerErrorCode filter in install.ps1's WMI fallback (still Select-Object -First 1 on main), the private arch handoff, and the tests.
danielhanchen
force-pushed
the
pr-8461-xplat-ci
branch
from
August 11, 2026 19:16
bce121f to
865023c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disposable CI run for unslothai/unsloth#8461. Do not merge; closed after CI.