[staging CI] unslothai/unsloth#9858 - #1242
Closed
danielhanchen wants to merge 57 commits into
Closed
danielhanchen wants to merge 57 commits into
danielhanchen wants to merge 57 commits into
Conversation
The in-app updater runs `unsloth studio update`, which runs studio/setup.ps1, which runs studio/install_python_stack.py. install.ps1 is never on that path, and install.ps1 holds the only PyTorch flavor repair there has ever been. Meanwhile the dependency steps in install_python_stack.py install with deps and without an --index-url, so the resolver's default source is PyPI, whose Windows torch is 2.11.0+cpu, and constraints.txt names no torch to stop it. Both Windows repair passes are disabled there: _ensure_cuda_torch returns early on Windows, and the step 13 repair block is gated on `not IS_WINDOWS`, even though its own comment says the steps above can pull torch from PyPI. The result is a cu124 venv that comes out of an update as 2.11.0+cpu with the update reporting success. The app then runs everything on CPU, Settings shows no GPU, and the bundled CUDA llama-server prints "Available devices: (none)" because a +cpu venv ships no cudart64_*.dll for it to load. Add a Windows torch flavor invariant as step 13w. It resolves the flavor the install actually selected (env var from setup.ps1, then the install manifest, then a live probe), compares it against what is in the venv, and reinstalls the bounded trio from the correct index on a mismatch. It prints install.ps1's line verbatim so a support log from either path reads the same. If torch is still CPU-only afterwards on a host that expected a GPU build, the install now fails rather than reporting "dependencies up to date". It is conservative by construction: no-torch mode, a deliberate UNSLOTH_TORCH_BACKEND, a non-CUDA or absent expectation, a hidden GPU via CUDA_VISIBLE_DEVICES, and a torch that is missing or unimportable all no-op. Also stop the update path deleting a healthy venv. setup.ps1 collapses the expected tag to "cpu" whenever its bounded nvidia-smi probe comes back empty. On a direct update none of the existing escapes apply, so a working cu124 venv was removed and setup then aborted telling the user to run install.ps1, with no rollback copy because only install.ps1 keeps one.
for more information, see https://pre-commit.ci
The publication block relied on `$env:X = ""` deleting the entry. That held on Windows PowerShell 5.1 and on 7.0-7.4, but 7.5 picked up .NET 9's change: an empty string is now a real value and only $null removes the name. The comment asserted the old behaviour as fact. Both spellings still read as unset on the Python side, which tests truthiness rather than presence, so nothing was broken today. The point is that a setup script should not publish a value whose meaning depends on which PowerShell the user happens to have. Remove-Item behaves the same on every version. Deleting rather than blanking matters on its own too: a value inherited from the caller's shell must not survive a run that decided it cannot name this host's flavor, or the stack enforces a stale expectation against a fresh venv. Found by running the block under a real PowerShell 7.6 over the host matrix.
Three separate holes in the invariant added earlier in this PR, all reachable. Do not route around an explicit index pin. When a pin's leaf names no flavor (a corporate /simple mirror), setup.ps1 publishes nothing, so the expectation fell back to the manifest, which records whatever was installed BEFORE the pin was set. Repairing off that stale tag rejects the pinned URL, because its leaf does not match, and reinstalls from the public pytorch index: it overrides an administrator's deliberate package source and fails outright on a network-restricted host. Decline instead, on the same test _ensure_cuda_torch already uses. Enforce the published XPU flavor, not just CUDA. setup.ps1 publishes "xpu" for an Arc host and installs the XPU trio before handing over, so treating that expectation as a no-op left the invariant carrying an answer it refused to act on. The exposure is identical: the dependency steps re-resolve torch from PyPI, and _ensure_xpu_torch cannot clean up afterwards because step 13's whole repair set is gated off Windows. Repairs with the XPU trio, whose floor is 2.6 rather than 2.4, since unsloth raises at import for an XPU device below that and a 2.5 wheel would satisfy the CUDA range and be kept. ROCm stays with the path that owns it: its wheels come from a per-architecture repo.amd.com index that cannot be reconstructed from a generic "rocm" tag. Classify a CUDA venv from disk when the torch probe hangs. A wedged display driver faults or hangs `import torch` on CUDA exactly as a faulted HIP runtime does on AMD. XPU and ROCm both rescue that venv by reading torch/version.py, but CUDA had no such arm, so the probe-failure chain fell through with a null tag, the no-wipe escape could not see a cu* wheel to preserve, and a direct update deleted a healthy CUDA environment before aborting. Keeps the family rather than a flat "cuda", because the stale comparison it feeds is cu126-vs-cu128, and matches with [regex]::Match so $Matches in the caller's scope is untouched. One existing test asserted the XPU no-op and has been updated: it was pinning the behaviour above, not guarding it.
The invariant enforced cu* and xpu but treated a published "rocm" expectation as a successful no-op. That left the AMD Windows path with the same hole the whole change exists to close: _ensure_rocm_torch runs at step 2b, which is BEFORE the dependency steps that can resolve torch back to PyPI's CPU wheel, and nothing ran it afterwards, so the update could still write a successful CPU-only manifest. The reason given for excluding it was that AMD's Windows wheels live on a per-architecture repo.amd.com index that a generic "rocm" tag cannot name. That much is true, and it is why this pass still does not build a ROCm URL of its own: setup.ps1 hands over $TorchInstallIndexUrl, which on the AMD path still points at /cpu. But it is not a reason to leave the flavor unchecked, because _ensure_rocm_torch already owns the whole problem -- it detects the arch, maps it to an index, honours an explicit ROCm pin, and re-probes the stale UNSLOTH_ROCM_TORCH_INSTALLED marker rather than trusting it. So the ROCm arm delegates to it and then applies the same verdict as the other two: still CPU-only afterwards fails the update rather than reporting success. The warning block is now a helper shared by both exits, so the three flavors cannot drift apart in what they tell the user.
…family
The two gates at the top of _ensure_expected_torch_flavor both fired on a host
that had pinned XPU or ROCm on purpose, which is the opposite of what the XPU
and ROCm enforcement added just before this was for.
_TORCH_BACKEND is not only install.sh's export: when it is empty, this module
derives it at import from UNSLOTH_TORCH_INDEX_URL / _FAMILY, so a pinned xpu or
rocm index sets it to "xpu" or "rocm" and the `not in ("", "cuda")` gate then
skipped the only post-dependency Windows invariant there is. Those two values
now fall through and are re-tested once the expectation is known: a GPU backend
that DISAGREES with what setup.ps1 published is still a no-op, because the pin
is the newer and more specific instruction, while one that agrees is enforced.
"cpu" and unrecognised values still exit before the expectation is resolved, so
that path costs no GPU probe.
The second gate was a flat veto on _explicit_unknown_family_torch_index_url().
That helper's known set is rocm*/gfx*/cpu/cuXXX -- it predates XPU and does not
count "xpu" as a family -- so an explicit XPU pin read as "unknown" and skipped
the very host it was set for. It is now compared against the expectation
instead: a pin whose leaf IS the family being enforced is not an unknown pin,
and it is also the correct index to repair from. Widening the helper itself
would have changed _ensure_cuda_torch and _ensure_rocm_torch too and drifted
from install.sh / setup.ps1 / install.ps1, which it is documented to match.
The parametrized deliberate-backend test still passed throughout, because it
pairs a rocm/xpu backend with a cu124 handover, which is the disagreement case.
It is split so the two reasons are stated separately rather than one standing in
for the other.
The manifest records what a previous install left behind; an index pin is the instruction for the run happening now. Resolving the manifest first meant a freshly set UNSLOTH_TORCH_INDEX_FAMILY=cu128 over a cu124 manifest resolved to cu124, and _expected_torch_index_url then rejected the cu128 pin as a family mismatch and repaired from the public cu124 index, undoing both the family and the source the user had just selected. The pin now resolves second, after the setup script handover and ahead of the manifest. AMD leaves collapse to "rocm" so the tag matches the installed flavor vocabulary, and a leaf naming no family still falls through to the manifest, so the unknown-pin escape added earlier is unchanged.
…repair No win_arm64 torchaudio wheel is published on any index. setup.ps1 drops it from all four of its install trios, but the new final repair asked for it unconditionally, so a cu* expectation reaching that path on Windows on ARM turned a repairable venv into a failed install. The repair now drops torchaudio on the same condition, for the CUDA and XPU trios alike, using the _is_windows_arm64 helper the script already carries.
for more information, see https://pre-commit.ci
Two ways the invariant could report success over an environment that never got what it asked for. The ROCm arm accepted any GPU build. _torch_build_is_gpu is deliberately family-blind, so a transient repo.amd.com failure inside _ensure_rocm_torch, which is non-fatal there, left the old cu124 or xpu wheel in place, passed the check, and the manifest recorded rocm over it. The arm now re-probes the flavor and requires the family it asked for, with a distinct warning for a wrong family and the existing CPU-only warning kept for a venv that came back CPU. A venv nothing could read still passes, because ambiguity must not fail an update by itself. An expectation of cpu was never enforceable, but the unpinned dependency steps honour UV_TORCH_BACKEND, so a venv deliberately built against /cpu can come out of them holding a GPU wheel. A pinned CPU index is now enforced, and only a pin: setup.ps1 also publishes cpu when its nvidia-smi probe returns nothing, which is the wedged-driver host whose healthy cu124 venv the no-wipe escape in that file exists to protect, and acting on that handover would force it down to CPU. The verification for that case compares the family rather than asking whether a GPU build is present, which is the opposite question.
…ase pins Four ways the invariant could still finish over the wrong environment. An untagged wheel under a CPU pin. _torch_flavor_tag reads every untagged version as cpu, which is right for PyPI and wrong for a private index serving an untagged CUDA or ROCm build: that wheel compared equal to a cpu expectation, skipped the repair, and was recorded as cpu. The runtime probe already carries the markers _ensure_cpu_torch uses to tell them apart, so the comparison consults them. The non-ROCm tail accepted any GPU build. A misconfigured mirror can answer a /cu128 request with a cached cu124, rocm or xpu wheel, and _torch_build_is_gpu is family-blind, so the update exited 0 and recorded the requested tag over a build that never arrived. The tail now re-probes the flavor and requires the family it asked for, as the ROCm arm already does. A CPU-only outcome keeps its own warning, since the remedy differs, and an unreadable venv still passes. torchao and xFormers are re-settled, but only when the repair actually moved the torch release, since the common case restores the wheel the venv already had. torchao's cpp extensions are release-specific and step 4 chose its pin from the torch this repair replaced, typically 0.17.0 for a 2.11 that the <2.11.0 spec takes down to 2.10, whose matched build is 0.16.0. xFormers is stricter: its _C.pyd is linked against one exact torch and CUDA pair, and beside any other pair the load is downgraded to a log line, so the import succeeds with memory-efficient attention, SwiGLU and the sparse ops silently gone. This script never installs xFormers and cannot reconstruct install.ps1's wheel selection, so a mismatched one is removed, which is what the backend's own resolver concludes as well: torch SDPA beats an extension that cannot load. Neither half can fail the update, both being secondary to the defect the repair has just fixed.
torchao depends on torch, so the --force-reinstall I added last round resolved dependencies and could re-resolve torch from the unpinned default index, which on Windows is the 2.11.0+cpu wheel the repair had just removed, reinstalled moments after removing it and after the verification had already passed. The update would then exit 0 and record a GPU tag over a CPU-only venv, which is the exact defect this whole change exists to close, reintroduced by its own follow-up step. The reinstall now passes --no-deps, which is not an optimisation here but the safety of the call: torch and the rest are already present and correct at that point. The helper additionally reports whether it ran anything that could touch torch, and the caller re-verifies the flavor when it did, so a regression cannot be silent even if the flag is ever wrong. The xFormers half was gated on the release moving, but its _C.pyd is linked against the exact torch and CUDA pair, so 2.10.0+cu124 to 2.10.0+cu128 after an index family change keeps the release and still leaves an extension that cannot load. The two are now gated separately: torchao on the release, xFormers on the full build label.
_select_torchao_spec branches on cuda>=13 as well as on the release, so a repair that takes a venv from cu124 to cu130 at the same torch release changes the matched build while the release comparison alone reports nothing happened. The 0.16.0 extension cannot load beside a cu130 wheel, which is the same silent drop to the slow fallback the release gate was added for. The gate now fires on either. A flavor move within one CUDA major, cu124 to cu128, still leaves torchao alone, since nothing about its matched build changed there.
Every GPU field in the System tab starts from get_device(), which is torch-only. The moment torch.cuda.is_available() returns False the whole chain goes quiet: get_backend_visible_gpu_info() and get_visible_gpu_utilization() both short circuit on their DeviceType gate, so the nvidia-smi probe that would have listed the cards correctly is never reached, and the UI says "No visible GPU" on a host with two working A4000s. CHAT_ONLY_REASON is set to "no_gpu", which the sidebar renders as "needs an NVIDIA or AMD GPU" - the opposite of the truth. This is the Windows shape of unslothai#8473 (reported there on AMD/Linux, also unslothai#7485 and unslothai#7307): a managed venv whose torch was replaced by PyPI's 2.11.0+cpu, with nvidia-smi listing every card throughout. Ask the OS as well as PyTorch: - get_physical_gpu_inventory() probes nvidia-smi, and on Windows the DirectX adapter registry that _rocm_windows_per_device_vram already trusts. It runs regardless of DEVICE, is bounded, never raises, and is cached with a TTL because a driver restart or an eGPU can change the answer. - classify_torch_build() separates a CPU-only wheel from an accelerator wheel whose runtime will not initialise. The advice differs, and telling someone with a healthy cu124 wheel to reinstall torch sends them the wrong way. - The CPU fallback in _detect_hardware_locked now reports "torch_cpu_build" or "torch_cuda_unavailable" instead of "no_gpu" when the host does have GPUs. An emptied CUDA_VISIBLE_DEVICES / HIP_VISIBLE_DEVICES / ROCR_VISIBLE_DEVICES is a deliberate hide, not a mismatch, and is excluded. The inventory is display-only. physical_devices and mismatch sit BESIDE devices in the payload and are never merged into it: devices is the runtime-usable list that the model-fit estimate budgets against and that the training device picker pins from, so a card torch cannot open must not become selectable. Also adds a diagnostic for the second half of the same report. The CUDA llama.cpp prebuilt takes cudart64_*.dll / cublas64_*.dll from the managed venv, so a CPU-only torch leaves it with nothing to load and llama-server prints "Available devices: (none)" while the prebuilt marker still says backend cuda. That was entirely silent; it now logs once per binary and changes nothing else. Finally, the repair the banner points at. Both affected users were fixed by rerunning the installer, and one asked for a button. start_managed_repair gains force_installer, which skips the update attempt: an update reuses the environment it finds, so a venv on a CPU-only wheel survives any number of successful updates. Settings > General grows a "Repair installation" row next to the existing reset row, wired through a small context to the same startRepair the startup screen uses, so it renders the same repairing UI and restarts the backend afterwards.
Extended CPU wheel tags. PyTorch publishes CPU builds whose local version carries a suffix, such as 2.8.0+cpu.cxx11.abi. An exact match on "cpu" classified those as accelerator wheels whose runtime failed to start, so the UI pointed the user at a driver instead of at the reinstall that actually fixes it. nvidia-smi off PATH. A driver install can leave nvidia-smi.exe in the NVSMI directory or the driver store without putting either on PATH, and the bare name then raises FileNotFoundError, leaving the physical inventory empty on exactly the host this inventory exists for. Resolution now falls back to the same two locations setup.ps1 already tries. Export and Video capability. Both render their message verbatim, and neither knew the new reasons, so a host with two working A4000s was told no supported accelerator was found, contradicting the inventory the same server had just published. Both now carry a reason-specific message that names the installed build and points at the repair, with the driver case worded differently from the CPU-build case. The llama.cpp CUDA runtime warning. It scanned only the directories prepended to the search path, but the inherited PATH is appended immediately afterwards and Windows resolves from it, so a hand installed CUDA toolkit was told to repair a working installation. The check now runs against the full search path. Also hides the Settings repair row for an externally started backend: start_managed_repair refuses that mutation, but startRepair swaps the shell to the repairing screen before the refusal arrives, leaving a connected user on the repair-error screen.
…icts Linux AMD had no physical inventory at all. nvidia-smi contributes nothing on that host and the AMD branch was Windows-only, so a ROCm wheel replaced by a CPU one left the inventory empty, the verdict at no_gpu, and the System tab still claiming there is no GPU, which is the AMD/Linux shape of the report this feature exists for. Added a sysfs walk of /sys/class/drm: vendor 0x1002 and a byte-valued mem_info_vram_total are both documented amdgpu interfaces, and the kernel driver publishes them whenever a card is bound. sysfs rather than amd-smi deliberately, because amd-smi is separate ROCm userspace and this is the host whose ROCm install is in question. No name is reported, since the kernel publishes none and borrowing an amd-smi row whose ordering is not guaranteed to match this walk would attach the wrong name to a card. Visibility masks are now applied only where they can hide something. Windows HIP has no ROCr layer, and this module's own resolver already ignores ROCR_VISIBLE_DEVICES there, so a stray empty one restored the no_gpu verdict on a Windows NVIDIA host with a CPU wheel. The HIP-layer variables are consulted only when an AMD card is actually in the inventory. An inventory that answered nothing keeps every variable, because unknown has to stay conservative. A deliberate CPU install is no longer reported as broken. Pinning /cpu on a machine that still has a GPU leaves no mask empty, so the wheel the user asked for was classified as a fault and the UI offered a repair whose only effect would be to replace it. An explicit CPU pin, or a cpu flavor recorded in the venv manifest, now suppresses the CPU-build reason. Only cpu counts: an absent or unknown record is not a choice. The dead-accelerator reason is untouched, since a wheel whose runtime will not start is a real fault whatever was recorded.
Three ways the deliberate-CPU and mismatch reporting could still be wrong. The manifest carried no flavor on any non-Windows install, because the tag was only ever resolved inside the Windows branch. A Linux GPU host installed with a transient explicit CPU pin therefore recorded nothing, and on the next launch, with that variable gone, the backend saw a CPU wheel beside a physical GPU and offered a repair that would reverse the choice. The tag is now resolved on the other platforms too, for the record only: nothing outside Windows enforces it, since step 13's repair set owns those hosts. A token-authenticated CPU pin was not recognised. Credentials in the query are supported and common, and a raw final-segment split saw cpu?token=... rather than cpu, so exactly the deliberate CPU install this was meant to protect was still reported as broken. The leaf is now read the way the installers read it, query and fragment stripped, trailing slashes off the path only. The chat-only verdict never followed the inventory. detect_hardware runs once at startup while the inventory refreshes on a 60 second TTL, so an eGPU attached after launch, or a driver that finished restarting, left the sidebar and the Export and Video pages insisting no accelerator exists while /api/system listed the card and published a mismatch. A card that goes away produced the mirror image. current_chat_only_verdict re-derives the three inventory-sensitive verdicts and leaves every other one exactly as detection set it, since a 60 second probe cannot change those and re-deriving them would fight detect_hardware rather than follow it.
… request path Intel was missing from both inventories. An Arc host whose XPU wheel was replaced by a CPU one is exactly the shape this reporting exists for, and nvidia-smi contributes nothing there either, so the inventory came back empty and the mismatch was discarded. The Windows DirectX registry scan now takes a vendor id, defaulting to AMD so every pre-existing caller is unchanged, and the Linux sysfs walk accepts 0x8086 beside 0x1002. Intel publishes no vram total on the discrete path, so an Arc card is reported with an unknown capacity rather than left out: knowing the GPU exists is the point. An unknown inventory is no longer collapsed into an empty one. nvidia-smi timing out or exiting non-zero comes back as a structured empty result, not an exception, so a transient failure read as the GPU having disappeared and retired a settled mismatch to no_gpu for a whole cache interval, handing the user the opposite advice. The probe now reports unknown when nothing was found AND some probe declined to answer, and the refreshed verdict keeps what it had in that case. The refreshed verdict no longer runs a subprocess on the request path. /api/health and /api/liveness both read it, the NVIDIA half shells out with a 10 second timeout, and the lock makes concurrent callers queue behind it, so a hung or restarting driver, which is the host this feature is for, could stall the event loop for ten seconds against a 2 second desktop timeout every time the TTL expired. Non-blocking callers now take the last known answer and hand the refresh to a daemon thread, single-flighted so a second caller does not queue a duplicate probe. Startup detection and /api/system still block, which is what keeps the cache warm for everyone else.
…etect on recovery Registry records are no longer trusted on their own. The DirectX registry outlives the hardware, which setup.ps1 states outright and works around by using these records only to relabel an adapter its live WMI scan also returned, never to add one. A CPU-only machine with a driver record left behind was being told it had an unusable GPU and offered a repair that cannot restore absent hardware. Each record is now joined against Win32_VideoController, the same live source setup.ps1 scans, by substring in either direction because the two spell the same card differently. A live scan that cannot answer contributes nothing and marks the inventory unknown, which keeps a settled verdict rather than inventing one. An ordinary Intel iGPU no longer establishes a mismatch. setup.sh does not autodetect Linux XPU at all and setup.ps1 limits automatic XPU detection to Arc and Data Center GPU by name, so an Intel UHD beside a CPU wheel is the correct state of that machine, and counting it reported a broken install and offered a repair that would reinstall the same CPU build. Intel now counts only when the name matches setup.ps1's rule, or an XPU flavor was pinned or recorded for this venv, or the installed torch carries an XPU runtime. The last two matter because the Linux sysfs walk publishes no name. The cards stay in the inventory either way, since the OS really does see them; what changed is what may be called a fault. A recovered accelerator now retires the cached verdict. The refresh added earlier updates only the reason and detail, so a driver that finished restarting, or a usable eGPU attached after startup, left DEVICE and CHAT_ONLY frozen at CPU while the reason flipped to no_gpu: Train and Export disabled and the UI insisting there is no GPU, until a restart. The refresh now invalidates the detection epoch instead and keeps the current answer until the next pass republishes, at most once per recovery so a poll cannot starve detection of a chance to settle.
…eter probes classify_torch_build asked only about CUDA, so an Intel host that started while its runtime was down and later recovered kept a healthy +xpu wheel classified as torch_cuda_unavailable forever. The recovery branch added last round was therefore never reached and the process stayed chat-only until restart. It now consults torch.xpu.is_available as well, with a probe that raises still counting as no answer rather than as a verdict. ZE_AFFINITY_MASK joins the deliberate-hide set. The rest of this module already reads an emptied one as hiding every Intel device, but it was missing here, so a deliberately masked XPU install was reported as broken and offered a repair for a configuration the user had chosen. Applied only where an Intel card is present or the inventory could not answer, matching the rule the HIP masks already follow. Registry records are claimed one to one. Two identical cards leave two identical records, and if one is removed WMI reports a single live adapter that the previous predicate matched to both, reporting a GPU that is gone and counting its VRAM twice. Each live entry is now consumed by at most one record. The live scan no longer swallows its own failure. -ErrorAction SilentlyContinue exits 0 with empty stdout, which is indistinguishable from no adapters, so the corroboration would have dropped every real card on a host with damaged WMI, recreating the false no-GPU result this change exists to remove. It now stops on error so the caller sees the documented unknown. A missing nvidia-smi is no longer a warning. That is the normal state of every CPU-only, AMD and Intel host, and the inventory runs on a 60 second refresh reached from the health and system polls, so it was a log line a minute on machines that are working correctly. A driver that IS installed and then hangs still warns.
Step 13w migrates torch on Windows for the same reason the two Linux repair passes do, and a direct `python install_python_stack.py` there has no setup.ps1 postlude to swap triton back afterwards. The platform guard test still asserted the old contract, where Windows was skipped unconditionally, so it covered the case the invariant exists for and called the fix a regression. Windows is now covered on both sides: it defers when setup.ps1 published the handover variable, and does the swap itself when nothing else will. The wiring tests count the third call site and check that it follows _ensure_expected_torch_flavor, which is that platform's torch migration.
… the sidebar keeps looking Five gaps from the last review, four of them in code I added earlier in this same review: - The recovery path called invalidate_detection() and stopped there, which retires the cached answer but schedules nothing to replace it, so DEVICE and CHAT_ONLY stayed frozen at CPU while the reason flipped. It now discards the detection under the lock and starts a fresh pass. - The mismatch report probed the inventory with block=True. /api/health and /api/liveness both reach it, and the NVIDIA half shells out with a 10 second timeout, so a hung driver -- the exact host this feature is for -- would have stalled liveness behind it. Non-blocking now. - classify_torch_build() gave up entirely when torch could not be imported, so a venv broken badly enough to matter reported nothing at all. It now reads the wheel's own version label off disk, with no interpreter started. - ZE_AFFINITY_MASK was missing from the visibility masks, so an Intel host that had masked its GPU away was told the install was broken. - The sidebar stopped polling at the first settled verdict, so a host that gained an accelerator after launch could not learn about it without a restart. It now keeps polling at the backend's own 60s inventory cadence, and only for the three verdicts a probe can actually move. A deliberate CPU, ROCm or XPU backend is also recorded as the expected flavor now, but only when the wheel on disk agrees with it: a backend is what the run asked for, and recording one that did not install would assert something untrue about the venv.
for more information, see https://pre-commit.ci
…e or untagged XPU build is classified correctly Three more from the review, all on the same function: - classify_torch_build() returned early on _has_torch(), which is False both for a torch that is absent and for one whose native runtime will not load. The second is the case this classifier exists for, so the on-disk fallback added last round was unreachable on precisely that host. The test forced _has_torch to True and hid it; it now asserts the real value instead. - An untagged build that sets torch.version.xpu is an XPU wheel whose runtime is down, not a CPU wheel. It read as torch_cpu_build and the UI offered a reinstall, which is the one remedy that cannot help, while _torch_reports_an_xpu_runtime() already knew better and let the card establish the mismatch. - The classification itself still ran inline on /api/health and /api/liveness. It imports torch and asks CUDA and XPU whether they are available, and both block while a driver is wedged, so making only the inventory lookup non-blocking left the stall in place. It is now cached with the same 60s TTL and single-flight background refresh the inventory uses: detection takes the one blocking pass, the request paths read the measurement, and a cold cache reports unknown rather than guessing that torch is fine. A recovery drops the measurement along with the detection, or the fresh pass would settle on the answer it just retired.
for more information, see https://pre-commit.ci
…runtime is a mismatch not a failure Four more, all genuine: - An emptied ZE_AFFINITY_MASK on a hybrid NVIDIA plus Arc host was cancelling the whole classification, so a CPU-only wheel went unreported for the NVIDIA card the user never masked. Masks are per vendor now: they suppress the classification only when they account for every accelerator the host has, and the cards they do hide drop out of the mismatch inventory instead. An inventory that cannot answer still stays conservative. - The on-disk classifier read only __version__, so an untagged conda or source CUDA build that failed to import was called a CPU wheel. It now reads the cuda/hip/xpu markers out of torch/version.py as well, which is what the importable path already looks at, parsed rather than executed. - UNSLOTH_TORCH_BACKEND was consulted after the manifest, so a reinstall that deliberately changes flavor re-recorded the previous install's tag: the next launch called the new CPU wheel a mismatch and a later unpinned update would have restored CUDA over the user's choice. The current run's backend now outranks the manifest, still only when the installed wheel agrees with it. - A wheel whose native runtime will not load took the detection_failed arm and never reached the classification, so the host this change exists for got a pointer to the server log rather than the repair. It is now classified from the wheel on disk, and detection_failed remains the answer when nothing on disk says otherwise or the OS found no card. The disk path deliberately avoids importing torch anywhere: that import is what failed, it takes seconds to fail on a real broken wheel, and a retry re-runs torch/__init__ against the partial module tree the first attempt left behind. Both hardware caches are also cleared between tests under their locks, or one suite's host answers for the next one within the TTL.
for more information, see https://pre-commit.ci
…es a failed torch import - HIP honours CUDA_VISIBLE_DEVICES alongside its own variables, which is why the visibility resolver in this module already reads all three together on an AMD host. Mapping it to NVIDIA alone meant an AMD-only box launched with CUDA_VISIBLE_DEVICES="" did not count as masked, so its cards still established a mismatch and Studio offered a repair for a mask its owner set on purpose. - The version label was still read with _torch_version_label(), which imports torch. On the broken-runtime host that import is exactly what fails, it can take seconds, and _has_torch() purges the partial module afterwards so every call genuinely re-runs the native load. /api/health, /api/liveness and GET /api/system all reach it, so the one place that seeded the classification from disk was being undone by the line that reported it. All three now read what detection published, or the label the wheel carries on disk, and only a host whose torch imports fine asks torch itself.
for more information, see https://pre-commit.ci
setup.sh ships a ROCm wheel only for RDNA 2 and newer, so a Polaris or RDNA 1 card is left on CPU torch on purpose. The DRM sysfs walk reports it as vendor amd like any other, and every non-Intel device was counted as evidence, so that host was told its installation was broken and offered a repair that cannot make the GPU usable. The arch is now consulted the same way the installer picks one: rocminfo, then amd-smi, read once inside the cached inventory probe rather than on any request path. The targets are attached to every AMD record rather than matched one to one, because setup.sh says in as many words that these orderings are not guaranteed to agree and a guessed pairing would label the wrong card. On Windows the driver already writes the arch into AdapterFamily, so that record answers directly. A host with no ROCm userspace names no arch, and a card with no arch is still counted: that is the conservative direction and the shape of the original report, an RX 7900 XT, which this stack does support. A ROCm expectation or a torch carrying a HIP runtime also outranks the table outright, since a wheel this install asked for is a fault whenever it is missing. The supported set is pinned against setup.sh's own table by a test, so the two cannot drift.
for more information, see https://pre-commit.ci
A visibility mask no longer vetoes an explicit expectation. An update inheriting CUDA_VISIBLE_DEVICES="" or -1 disabled the invariant outright, so the dependency steps could still put PyPI's CPU wheel over a cu124 venv and the update recorded the CUDA expectation over it, which is the whole defect this change exists to close. An emptied mask is a reason not to CONCLUDE a flavor from an nvidia-smi probe; it is not a reason to ignore one setup.ps1 published, one the user pinned, or one the last install recorded. The gate now applies only to the inferred case. The post-repair classification uses the same rule as the pre-repair one. Both compare through _torch_flavor_tag, which reads every untagged version as cpu, and only the pre-repair side consulted the runtime markers, so a private index answering a /cpu repair with an untagged CUDA wheel was accepted on a rule the check that triggered the repair had just rejected it on. The resync notices its own failures. pip_install_try and _uninstall_distribution report by return value rather than by raising, and both results were ignored, so an unreachable PyPI behind a reachable torch index, or an xFormers whose files were locked, left the wrong build in place while the update reported success. Both now warn. Neither fails the update, which stays deliberate. A direct Windows run now performs the XPU Triton swap. setup.ps1 does it after this script exits, so the helper skipped Windows entirely, but a direct invocation has no such postlude and the core package install pulls triton-windows over torch's XPU triton, leaving torch.compile unable to use the XPU. The handover variable gates it, since setup.ps1 sets it immediately before invoking this file and clears it otherwise.
…rch is not a hung one _ensure_rocm_torch's Windows branch asked for the full torch, torchvision and torchaudio trio unconditionally, so on win_arm64, where no torchaudio wheel exists on any index, the whole install is unresolvable. That is worse on the delegated path than on the plain one: the failure there is nonfatal, so the CPU build the repair existed to replace stays in place and the family verification then fails the update. The same _is_windows_arm64 exception the flavor repair and setup.ps1 apply now covers it. setup.ps1's disk-label rescue treated a definitive import failure exactly like a driver timeout. Both leave a +cu* version.py behind, and keeping the venv is right in both cases, since deleting one does not fix a driver and a faulted display or HIP runtime raises at DLL load rather than timing out, which is the bug the rescue was written for. But only a timeout says nothing about whether the installation is sound: a truncated torch took the same path, the family-matched install below then ran with bare requirements and no reinstall flag, and the run could write a completion manifest over a torch that still could not import. Invoke-BoundedPythonProbe now reports TimedOut, so the two are distinguishable, and a definitive failure keeps the venv but forces a same-family reinstall in place. Restricting the rescue itself to timeouts would have resurrected the original wipe, so it is the reinstall that is gated, not the rescue. One existing assertion in test_pr5940_followups.py looked for the three companion names inside the call itself; the trio is now built just above it so the ARM64 exception can drop torchaudio, and the assertion follows it there.
…PU choice is not a deliberate one - export_capability() and video_capability() asked _has_torch() before the mismatch branch, so on a host whose wheel will not import they reported pytorch_not_installed for a PyTorch that is installed, and retried the failing import on every request that renders them. The already-derived mismatch is consulted first now. - The NVIDIA inventory treated a missing nvidia-smi as no cards. _has_usable_nvidia_gpu() falls back to /proc/driver/nvidia/gpus, so the installer could select or repair a CUDA wheel on a host where the backend insisted there was none. Same fallback here, publishing no name and no capacity because procfs gives neither. - An AMD card nothing can name is eligible again when the KFD topology confirms it, which is the fallback _has_rocm_gpu() uses so `studio update` can repair a minimal host with no ROCm userspace. Positive evidence of an unsupported arch still excludes it; the NVIDIA open driver's own KFD nodes are rejected by vendor id, as in the installer. - A recorded CPU flavor is only treated as a deliberate choice when the manifest says someone named it. setup.ps1 selects /cpu automatically on a GPU-less host and records it identically, so the tag alone left a machine that later gained a card reporting no_gpu with no repair on offer. The installer now records whether the flavor was pinned, and a manifest predating the marker reads as not pinned: an offered repair can be declined, a silently CPU-only GPU host cannot. - The test fixture took the two hardware cache locks in the opposite order to the background refresh, which could deadlock the suite against a refresh a previous test left running.
for more information, see https://pre-commit.ci
…ived backend is not a pin - A vendor whose probe did not answer no longer has its unknown cancelled by some other vendor's card. On a hybrid Intel plus NVIDIA host an nvidia-smi timeout was cancelled by the Intel sysfs row, and since an ordinary iGPU cannot establish a mismatch the verdict then saw neither an eligible card nor an unanswered probe and downgraded a settled mismatch to no_gpu, hiding the repair while the NVIDIA probe was merely unavailable. - The registry-to-live adapter match now prefers an exact name. The registry outlives the hardware, and a stale "RX 7900 XT" record is a prefix of a live "RX 7900 XTX", so it could claim the card that is really installed and publish the removed one's name and VRAM. The prefix rule stays: the driver description and the WMI display name spell the same card differently. - install.sh derives UNSLOTH_TORCH_BACKEND from the index it resolved, which is "cpu" on any machine without a GPU whether or not anyone asked for it, so treating it as a pin recorded every ordinary Linux CPU install as a deliberate choice and left a later eGPU with no repair on offer. install.sh now marks the value as derived, and only an unmarked one -- the form setup.sh documents for a deliberate CPU install -- counts, alongside the index pin and family.
for more information, see https://pre-commit.ci
… and procfs for a broken CLI - The supported-arch set was built from setup.sh's marketing-name table, which is a subset of what the installers actually ship a wheel for: gfx1031, gfx1033, gfx1035, gfx1036, gfx908, gfx90a and the gfx906 ROCm 6.3 path were all missing, so those cards were reported as no_gpu with no repair. It is now pinned against install.sh's own arch-to-index-family map, which is the map that decides, and the drift test fails in both directions rather than only on an over-broad set. - An AMD host that names its devices in HIP_VISIBLE_DEVICES while inheriting an empty CUDA_VISIBLE_DEVICES was read as fully masked. HIP consults its own variables first and falls back to the alias only when neither is set, which is the precedence _get_parent_visible_gpu_spec already applies in this module. - The procfs fallback covered only an absent nvidia-smi, not one that hung or exited non-zero, though the kernel driver enumerates its cards either way. On a cold start there is no settled verdict for the resulting unknown to protect, so the host was reported as having no GPU for as long as the CLI stayed broken.
for more information, see https://pre-commit.ci
…time, and drop a stale flavor under a mirror pin - install.sh marked every backend it exported as derived, including one the caller had already stated. setup.sh documents UNSLOTH_TORCH_BACKEND=cpu as the way to keep a deliberate CPU install, and on a GPU-less host the resolved value is cpu too, so the two were indistinguishable. It now looks before it overwrites, and only a value it derived itself is marked. - The shared torch probe reported hip and cuda but not xpu, so an untagged source or conda XPU build read as having no GPU support at all -- the one verdict that fails an update outright -- while the backend classifier added here reads the same torch.version.xpu as a GPU build. The probe carries it now, kept beside the tuple rather than in it because thirteen call sites unpack five values. - With an explicit pin whose leaf names no family (a corporate /simple mirror), the manifest wrote the PREVIOUS install's flavor back and marked it pinned, since any explicit index counts as one. The wheel in the venv came from the mirror and the old record describes a venv that no longer exists, so a later unpinned run could have "repaired" the mirror's build to a public one on the strength of a flavor nobody verified. Nothing is recorded for that case now.
for more information, see https://pre-commit.ci
Comment-only pass over the eight files this branch touches: delete restatements of the code, section banners and narration, and collapse multi-line blocks that did not carry a distinct fact per line. Added comment lines 409 -> 140. No code or behaviour change. Installer suites still pass (3123 passed). (cherry picked from commit e25b0ca)
Comment-only pass over the 26 files this branch touches: delete restatements of the code, section banners and narration, and collapse multi-line blocks that did not carry a distinct fact per line. Added comment lines 1164 -> 447. No code or behaviour change. Installer and hardware suites pass (3238 passed), gpu-torch-mismatch.test.ts passes (11), cargo check clean.
for more information, see https://pre-commit.ci
The existing test pins the shape of the polling guard by regex. It would still pass if the sense of the condition were inverted, and that regression is expensive in exactly the wrong direction: every healthy install would start forcing a /api/system read a minute for the life of the session. Lift the guard and the reason set and run them, over both the hosts the inventory can still reclassify (torch_cpu_build, torch_cuda_unavailable, no_gpu, since an eGPU can arrive on a CPU-only box) and the hosts that must not poll at all (a healthy GPU host, an Intel Mac, a failed detection). The two pre-existing polls, the MLX self-heal and the unknown-verdict one, are asserted alongside them so neither can be dropped.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 213f7cecdc
ℹ️ 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".
| nohup unsloth studio -H 127.0.0.1 -p 8888 > studio.log 2>&1 & | ||
| for i in $(seq 1 60); do curl -sf http://127.0.0.1:8888/healthz && break; sleep 5; done | ||
| - name: Playwright smoke (studio_test_kit) | ||
| run: PYTHONPATH=.github/scripts python -m studio_test_kit._smoke_ui || true |
There was a problem hiding this comment.
In the inspected staging-9858-playwright.yml workflow, studio_test_kit._smoke_ui exits nonzero when any browser, screenshot, streaming, video, or MP4 assertion fails, but || true converts every such failure into success. This also leaves the workflow green when Studio never becomes healthy and the smoke cannot connect; the following artifact upload already uses if: always(), so failure propagation does not need to be suppressed to preserve diagnostics.
Useful? React with 👍 / 👎.
… 3.13 The test put an object in sys.modules whose every attribute access raises, then asserted _has_torch() reports False. Whether that holds depends on import machinery internals: reading __spec__ off an existing sys.modules entry changed in 3.13, so on 3.10 through 3.12 `import torch` handed the stub straight back untouched, _has_torch() returned True, and the premise silently inverted. The body then ran against a host that looked like it had a working torch, so the on-disk fallback the test exists to cover was never reached, and the assertion that would have caught it is the one that stopped holding. Install a meta-path finder whose loader raises OSError from exec_module instead. That is a real import that really fails, which is also closer to the case being described -- a wheel whose native library will not load -- and it records TORCH_IMPORT_ERROR the way the real failure does, which the old stub did not. Verified on 3.10, 3.11, 3.12, 3.13 and 3.14: _has_torch() is False on all five and the recorded error is the OSError. Before this commit it was True on the first three.
danielhanchen
force-pushed
the
pr-9858-xplat-ci
branch
from
August 31, 2026 08:05
213f7ce to
f70c3aa
Compare
for more information, see https://pre-commit.ci
setup.ps1: a torch that will not import now clears $SkipPythonDeps. Its only consumer, the CUDA --force-reinstall, lives inside `if (-not $SkipPythonDeps)`, so on a current core package with a valid manifest the fast path skipped the repair and the update reported dependencies up to date over a broken wheel. install_python_stack.py: the two "untagged wheel under a /cpu pin" checks now read torch.version.xpu alongside .hip and .cuda through _gpu_family_from_runtime_markers(), so an XPU build no longer compares equal to cpu. _expected_torch_flavor_was_pinned() takes the flavor being recorded and counts a pin only when its own family matches it, so a ROCm or XPU pin that setup.ps1 fell back to the CPU index for is no longer recorded as a deliberate CPU choice. hardware.py: a Windows AMD adapter the DirectX registry gave no AdapterFamily is now read through the installers' marketing-name arch table instead of falling to a Linux-only KFD probe; a registry read that failed is distinguishable from a vendor with no adapters and marks that vendor unanswered; an inventory refresh that could not answer carries the previous devices forward per unanswered vendor rather than caching the empty failure; the ROCm pin check uses the installer's exact family predicate; and a detection_failed that was classified from the wheel on disk can transition once the inventory recovers, without ever falling through to no_gpu. use-tauri-backend.ts: retry on a failed forced repair re-runs the forced repair. The installer is transactional, so the preflight would find the restored install and restart the same CPU-only backend the user pressed Repair about.
for more information, see https://pre-commit.ci
danielhanchen
force-pushed
the
pr-9858-xplat-ci
branch
2 times, most recently
from
August 31, 2026 09:19
bcd3ee0 to
64428b0
Compare
danielhanchen
force-pushed
the
pr-9858-xplat-ci
branch
from
August 31, 2026 09:20
bd2ab65 to
8bd0865
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4c725c564
ℹ️ 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".
| # The gfx targets this stack will actually install a ROCm wheel for: install.sh's | ||
| # _amd_arch_index_family_for_gfx, plus gfx906 from the ROCm 6.3 path. A card outside this | ||
| # set (Polaris gfx803, RDNA 1 gfx101x) is left on CPU torch ON PURPOSE. | ||
| _ROCM_SUPPORTED_GFX = frozenset( |
There was a problem hiding this comment.
On a Linux CDNA host such as an MI300 (gfx942) with a CPU-only PyTorch wheel, rocminfo adds gfx942 to the physical device's gfx_candidates, but this allowlist omits it, so _amd_device_can_establish_a_mismatch() rejects the GPU and the UI reports no_gpu instead of exposing the PyTorch mismatch and repair action. These architectures are supported by the generic ROCm indexes selected in install.sh when a ROCm version is detected; the set should therefore include generic-wheel targets such as gfx940/gfx941/gfx942 rather than only the per-architecture index map.
Useful? React with 👍 / 👎.
Owner
Author
|
Upstream PR unslothai#9858 is merged; staging replica no longer needed. |
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#9858. Do not merge; closed after CI.