mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-23 15:53:46 +00:00
140 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Daniel Han
|
2e3cc032f6
|
Record the venv own requirement digests, not the installers (#9263)
* Record the venv's own requirement digests, not the installer's
Every fresh desktop install on Linux and macOS came up ManagedStale and
repaired itself before it would run. From the app's own log:
05:44:22 Managed preflight: install probe result Stale { reason: "studio_install_requirements_changed" }
05:44:22 desktop_preflight completed disposition=ManagedStale
05:44:22 start_managed_repair command called
05:44:24 studio install incomplete -- forcing dependency pass to repair...
05:44:32 Managed preflight: install probe result Ready
The manifest recorded digests from REQ_ROOT, which is the requirements
directory next to whichever install_python_stack.py ran. A desktop bundle
carries its own copy. verify_install reads the INSTALLED package's copy,
because at verify time install_manifest.py is imported out of the venv.
Two different trees, compared to each other.
They agree until a tracked requirement file changes upstream, and then
every install performed by that bundle is stale for ever. v0.1.800-beta
was cut 2026-08-14 and installs unsloth 2026.8.18; #9148 pinned openai in
extras.txt in between. Windows was unaffected only by luck of layout.
The digests now describe the tree the verifier will read. A source or
editable install has no copy under site-packages and still uses the root
it was given, which is what keeps an edited studio.txt invalidating the
manifest on the --local path.
Also: the CI gate reported "saw: ManagedStale" and nothing else on all
four platforms. The reason is logged on its own line, and tail -60 scrolls
it away the moment the backend starts logging, so the one fact that
explained this had to be recovered from an uploaded artifact by hand. All
three log dumps now grep for it.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
|
||
Nilay
|
9d1dcfe58a
|
Studio: fail fast when the installed llama.cpp prebuilt has no kernels for this GPU (#8841)
* Studio: fail fast when the installed llama.cpp prebuilt has no kernels for this GPU * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: skip the CUDA SM gate for deliberate CPU-only loads * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: apply the CUDA SM gate at launch placement and fix its remedy * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix CUDA SM gate device order handling * Studio: gate CUDA SM on the oldest compiled arch, not exact SM overlap * Add the OS x GPU matrix and two installer invariants for PR #8841 The ROCm arch gate has test_gpu_arch_gate_os_matrix_7624.py asserting it is inert off its one supported host shape; the CUDA gate had no equivalent, and inertness is exactly what output alone cannot show since a gate that ran and passed returns None like one that never ran. Spy the marker reader and the nvidia-smi probe across [Windows, Linux, WSL, macOS] x [NVIDIA, AMD, CPU-only]. Also pins two properties the design rests on but nothing checked: supported_sms must stay out of expected_install_fingerprint, or every pre-existing install compares stale and reinstalls on upgrade, and a non-CUDA bundle must record an empty list, which is the only reason the call site needs no Vulkan guard. Covers the floor decision (sm_121 PTX cannot JIT down to sm_120, but sm_120 to sm_121 can), unreadable and failing probes, and the visibility masks: a numeric mask without PCI_BUS_ID ordering fails open, an empty one hides everything, and a UUID or MIG mask is dropped so the whole host is weighed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the new code comments for PR #8841 * Shorten the comments added by the CUDA SM gate --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> Co-authored-by: danielhanchen <unslothai@gmail.com> |
||
|
Daniel Han
|
6c124b6dd8
|
Run the repo CPU test suite on all four runner cores (#9019)
* Fix a 500 on dictation Unload before any backend is resident
stt_unload passes expected_model positionally:
_, unload_stt = _stt_lifecycle()
failed = await asyncio.to_thread(unload_stt, engines, model)
_stt_lifecycle() returns the orchestrator's unload_stt_model when a backend is
resident and stt_registry.unload when one is not. Only the first takes
expected_model positionally; on the registry it sits behind a `*`:
def unload(engines = None, *, wait = True, expected_model = None)
def unload_stt_model(self, engines = None, expected_model = None)
So with nothing loaded yet, which is what a fresh process is, Unload raises
TypeError and the route answers 500. Reproduced outside pytest:
peek_inference_backend() fresh process: None
stt_unload -> TypeError: unload() takes from 0 to 1 positional arguments
but 2 were given
Pass it by keyword, which both callables accept.
Found while profiling CI, not by the tests, and the tests are why: this file's
two unload tests passed only because an earlier test in the full suite had left
a backend resident. Standalone they failed, and in their own file they failed;
they went green only inside the full serial run. That order dependence hid a
live bug. The new test drives the no-backend path directly, so neither the
signature nor the call site can drift back.
before: 2 failed, 53 passed (file alone)
after: 56 passed
* Run the repo CPU test suite on all four runner cores
Backend CI is the most expensive workflow in the repo at 126.7 runner-minutes
per push. Install is cached at ~1.2 minutes, so almost all of it is pytest.
ubuntu-latest has 4 vCPUs and this suite is CPU-bound and GPU-free, so it was
running on one core of four.
On a real runner the step goes 17.75 minutes to 7.67, and the failure set is
unchanged: the same single pre-existing failure, serial and parallel, on the
same tree in the same staging repo. Locally, on the dependency shape the
workflow installs, 806.1s to 219.7s with an identical 2 failed / 8066 passed /
178 skipped / 40 subtests result.
The three small pytest steps in this job stay serial: the hardware-spoof step
exists because those files mutate hardware.py module globals, and all three
already measure under 0.1 minutes.
The four-interpreter matrix is deliberately left alone. It is the bigger prize
at ~79 runner-minutes, but measured under -n 4 on a real runner it fails tests
serial does not, in more than one way and not the same way twice, so it needs
its order dependencies found and fixed first rather than a flag.
* Pin the two unload signatures against the route's one call site
The bug fixed in the previous commit exists because _stt_lifecycle returns two
different callables and the route has a single call site:
def unload(engines = None, *, wait = True, expected_model = None) # stt_registry
def unload_stt_model(self, engines = None, expected_model = None) # orchestrator
A call that suits one is a TypeError on the other, and which one runs depends on
whether a backend happens to be resident, so the broken half only appears on a
fresh process. That will recur the next time either signature is edited.
This binds BOTH real signatures against the arguments the route actually passes.
It does not demand they be identical, only that one call site can serve both.
Checked against two mutations:
registry grows a keyword-only param -> passes (benign, and it should)
registry drops expected_model -> fails, "got an unexpected keyword
argument 'expected_model'"
Paired with the test beside it: that one covers the call site (route -> registry
on a cold process), this one covers the two callees staying compatible.
57 passed.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Give each node harness run its own script file
Reported on this PR, and reproduced before changing anything. Every _run in
test_chat_preset_builtin_invariants.py wrote the same TEMP/run.mts and then
executed "node run.mts" from TEMP. Under pytest-xdist's default load
distribution two of this file's nine tests can land on different workers, so one
worker executes the script the other just wrote over the top of it.
Pinned to four cores, running the file at -n 4:
before: 5 failed / 6 failed / 5 failed / 5 failed / 6 failed (of 9)
after: 9 passed, six runs out of six
Serial is unchanged at 9 passed either way, which is why the full-suite runs
that back this PR did not show it: at ~8000 tests across four workers these nine
rarely collide, so it is a latent flake rather than a reliable failure. That is
worse, not better, and enabling -n 4 is what would have started rolling the dice.
A unique NAME rather than a per-call directory, which is what
tests/studio/_node_harness.py::run_harness does for every other node harness
here. Those scripts reach the frontend sources by a relative path counted from
TEMP, so an extra directory level breaks every import with ERR_MODULE_NOT_FOUND
(tried it: 9 failed). register.mjs and loader.mjs stay shared because their
contents are fixed, so a concurrent rewrite writes identical bytes.
Swept the other five fixed TEMP roots under tests/studio: all of them already
allocate per call through tempfile.mkdtemp, directly or via run_harness. This
file was the only one rolling its own.
* Shorten the harness comment
* Stop two more shared-state races the parallel repo job would expose
Both reported on this PR, both real, both confirmed by reading the code.
1. tests/studio/test_chat_preset_builtin_invariants.py still rewrote the shared
register.mjs and loader.mjs on every _run. Only run.mts became unique in the
previous commit. write_text truncates before it writes, so a worker rewriting
one of those while another worker's node process is importing it can hand that
process an empty or partial module. The contents are constant, which is why I
wrongly called a concurrent rewrite harmless: identical bytes still arrive
after a truncation. They are now written through a temp file and os.replace,
so every reader sees one whole version.
2. tests/python/test_no_torch_filtering.py::TestRealRequirementsFiltering had an
autouse fixture that snapshotted the requirements directory and, at teardown,
deleted every filtered file that had appeared since. Those files land in the
REAL requirements directory, not tmp_path, so under xdist one test's teardown
removes a file another worker is still reading. It now records the paths it
creates and removes only those.
test_chat_preset_builtin_invariants.py: 9 passed serial, 9 passed on four -n 4 runs
test_no_torch_filtering.py: 65 passed serial, 65 passed on four -n 4 runs
no filtered files left behind
* Run the event-loop latency tests outside the parallel invocation
Reported on this PR. tests/studio/load_freeze asserts upper bounds on real
elapsed time -- a /health burst under 250 ms while a 600 ms blocking probe runs,
and a 100-request burst under 350 ms -- and those bounds ARE the contract, so
they cannot be loosened without the tests ceasing to test anything. A pytest
worker descheduled by the other three inflates them.
Measured before changing anything, under 3x CPU oversubscription (8 spinners
pinned to the same 4 cores): 8 ms against the 250 ms bound, 38 ms against the
350 ms one, 3 runs, no failures. So the margin is wide on this box. It is not
wide enough to leave alone: a runner core is several times slower, which puts
the 350 ms bound within reach of an unlucky schedule. The directory is now
ignored from the -n 4 run and runs in its own serial step, 20s against the
~10 minutes -n 4 saves on this job.
The ignore and the step are two edits held together by nothing, and losing the
step is silent -- the job stays green while 23 tests stop running. New guard
tests/studio/test_backend_ci_parallel_isolation.py fails if an isolated path is
missing from the parallel run's ignores, or ignored with no serial step running
it, for load_freeze and for the three hardware-spoof files that were already in
this shape. Deleting either half of the load_freeze change fails it; a third
test pins the command scan so it cannot pass by matching nothing.
load_freeze: 23 passed in 20.14s serial
new guard: 9 passed, and 1 failed under each of the two mutations
test_ci_shell_suite_coverage.py + test_xpu_spoof_pipeline.py: 58 passed
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Raise the matrix hang limit past the suite it was cancelling
Not a review item; found while triaging why every PR in this stack carries a red
check. The Backend CI matrix limit was 30 minutes, set when the step measured
14.1 to 15.0. It now measures 22.5 to 29.2, and it is cancelling: across the six
open PRs on this stack, five had exactly one leg cancelled at 30m9s while its
siblings passed at 25 to 29 minutes. Which leg loses is luck, so the red says
nothing about the PR, and a cancelled leg burns the full 30 minutes for no
signal.
45, which is what the Windows UI job already uses. This is a hang guard, not a
budget, and it costs nothing until a job needs it. The real fix is running this
matrix on all four cores, worth ~79 runner-minutes a push, and it stays blocked
on the order dependencies this stack is working through one at a time.
* Give each PowerShell probe its own script path
Reported on this PR, real, and reproduced directly. _run_capturing_bytes wrote to
tests/python/_{stem}_probe_{shape}.ps1, a name several tests share, so under
xdist one case unlinks the script another has written before that one's pwsh
child opens it. pwsh is installed on ubuntu-latest, so these do not skip there.
Reverting to the shared name and running -n 4 four times: 1 failed each time.
With a uuid in the name:
35 passed, 8 skipped serial
35 passed, 8 skipped on each of four -n 4 runs
no probe scripts left behind
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Stop the pin scanner reading a generated filtered file as a second pin source
Reported on this PR and reproduced. test_no_torch_filtering.py exercises
install_python_stack._filter_requirements, which writes
`.{stem}-filtered-XXXX.txt` BESIDE the source on purpose, so relative -r/-c
includes still resolve. test_diffusers_pin.py::test_only_the_pin_file_names_diffusers
scans REQ_ROOT.rglob("*.txt"), so under xdist it can see one worker's copy of
diffusers-pin.txt and report it as a second source of the pin.
Calling _filter_requirements on diffusers-pin.txt and then running that test
fails exactly as described. It is not only an xdist problem: the function passes
delete=False, so the file also survives a real install on any machine.
The scan now skips dotfiles. That matches what it means -- no CHECKED-IN
requirements file other than the pin may name diffusers -- rather than narrowing
it: a real second source still fails it (verified with a probe file), and the
production write path is untouched, since writing beside the source is the
behaviour that makes includes resolve.
5 passed with a filtered file present, 1 failed with a real second source
* Exempt only the generated filter files, not every dotfile
Reported on this PR and right: a checked-in hidden requirements file such as
.constraints.txt is a real place the pin could be overridden from, and the
blanket dotfile skip took it out of the scan.
Matched by the shape _filter_requirements actually writes now: a dot, the source
stem, "-filtered-", and NamedTemporaryFile's 8-character suffix.
generated filtered file present: 5 passed
checked-in .constraints.txt naming diffusers: 1 failed
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
|
||
|
Daniel Han
|
93225592c0
|
Tighten the comments on the versionless torch probe tests (#8811) | ||
|
Nilay
|
083d613b95
|
Exclude studio/backend/tests from the wheel (#8501)
* exclude studio/backend/tests from the wheel * Exclude nested studio backend test packages from the wheel * Actually keep the backend test suites out of the wheel Dropping studio.backend.tests from packages.find only stops setuptools treating it as an importable package. include-package-data then hands every tracked file to the nearest parent package that survived, so the 505 files under studio/backend/tests plus the 11 under studio/backend/hub/tests came straight back as package data of studio.backend and the wheel was byte for byte the size it was before (43.95 MB, 2708 entries, both refs identical). exclude-package-data has the highest precedence of the file selection options, so repeat the veto there. Measured: 43.95 MB / 2708 entries -> 40.71 MB / 2192 entries, and the 516 entries removed are exactly the two test trees, nothing else. The patterns are source layout relative, so the sdist is unchanged (3666 entries either way) and editable installs still see the suites. Extend test_package_discovery.py so this cannot regress: it now replays setuptools' nearest-parent-package rule over the tracked file list and asserts no backend test file would ship, and checks a built wheel directly when dist/ has one. --------- Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
Daniel Han
|
a0c1723bd1
|
Studio tests: pin the empty-version torch probe distinction at the repair paths (#8803)
* Studio tests: pin the empty-version torch probe distinction at the repair paths _probe_torch_runtime draws a deliberate line between "" and None: an empty __version__ is a torch the pins repair, while None is a probe that told us nothing and must leave the venv alone. TestProbeParsing covers that at the probe. Nothing covered it where it decides something. That gap matters because the per-path probes classified inside the subprocess, from torch.version.cuda and .hip as well as the version string, so a consumer that gates on the version alone silently stops repairing a wheel whose runtime fields say plainly what it is. Collapsing the distinction to `if not _version` makes three of these four fail: a CPU pin stops replacing a versionless CUDA or ROCm build, and an XPU pin stops repairing an unidentifiable one. The fourth records the other side, where the two readings agree. Test-only. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
cb07e02d7b
|
Studio tests: pin the torch probe invalidation contract to both installers (#8795)
* Studio tests: pin the torch probe invalidation contract to both installers #8779 made pip_install_try() drop the memoized torch classification, because it installs the Windows AMD ROCm trio and a memo it does not clear can answer for the build it just replaced. That fix shipped with no test: only pip_install() was covered, so reverting the pip_install_try() line kept the suite green. Three tests, each of which fails with that line removed: - pip_install_try() drops the memo, mirroring the pip_install() case - a real reinstall is really reclassified: two real torch packages on disk and a real probe subprocess either side of a real call into each installer, so the answer that replaces the memo is shown to come from the venv as it is now rather than from the mock that dropped it - every function that builds a pip or uv install command invalidates. Read out of the module with ast rather than listed here, so a third installer helper cannot be added without either invalidating or failing this test No production change. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Pin the torchao consumer to the reinstalled torch as well Folds in the one test #8796 had that this branch did not. The other two there duplicate what is already here, so #8796 closes in favour of this. _select_torchao_spec reads _probe_installed_torch_version() between the two repair points, which makes it the consumer that would actually read a memo left over from before a reinstall, and pin torchao against the torch that was just replaced. The existing cases assert on _probe_torch_runtime directly; this one goes through the accessor that the torchao path really calls. Fails with the _invalidate_torch_runtime_probe() line removed from pip_install_try, along with the three siblings, and passes with it. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
|
Daniel Han
|
7fd3eeba75
|
Studio: share one torch classification probe across the repair paths (#8779)
* Studio: share one torch classification probe across the repair paths _ensure_cuda_torch, _ensure_xpu_torch, _ensure_rocm_torch and _ensure_cpu_torch each spawned their own `import torch` subprocess to derive the same handful of facts about the installed wheel, and the installer runs all four at both the post-base-requirements repair point and the final one. A Linux update therefore started up to nine interpreters just to classify torch: four repair paths at each of the two points, plus the torchao version probe. Route them all through one memoized probe returning (ran, importable, version, hip, cuda). pip_install() invalidates it, since a pip operation is the only thing here that can change what is installed, so a repair that swaps torch is still followed by a fresh classification. The timeout is the part that actually bit: each probe was bounded at 90s independently, so a stalled GPU driver (exactly the host these repair paths exist to rescue) could hang an update for many minutes before the first on-disk fallback ran. It is now one 90s wait per repair point. `ran` and `importable` stay distinct so every caller keeps its existing behaviour: a probe that could not run falls back to the on-disk classifier, while torch that is present but un-importable still forces the reinstall. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments around the shared torch probe Same explanations, fewer lines. The four repair paths each restated the same 'un-importable means missing or broken' point, and the probe docstring retold the nine-interpreter story at length; each now makes its point once. Comments and docstrings only, AST-verified per file (3/3 code unchanged). 702 tests pass, ruff clean. * Add the AGPL-3.0 header to the new probe test Matches the SPDX two-line form the other new files in this tree use. * Keep the replacing decoder on the shared torch probe The five probes this consolidated each read their subprocess with errors="replace". The shared one kept text=True and lost that, which decodes strictly, and UnicodeDecodeError is a ValueError, so one undecodable byte in torch's import chatter escapes the except (OSError, TimeoutExpired) below and takes the installer down instead of falling back to the on-disk classifier. That fallback exists for exactly the hosts that reach this code. Reachable wherever the console code page and the child's output disagree, which is the Windows case this repo already carries an encoding suite for. The regression test runs a real subprocess rather than a mock, since which decoder was used is invisible to a mock. Reverting the one-line fix fails it with the UnicodeDecodeError. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden the shared torch probe against the awkward venvs Three fixes from simulating the shared probe against the pre-probe installer over [Linux, WSL, macOS, Windows] x [NVIDIA, AMD, Intel, CPU] x 23 installed-torch states, plus one memo-invalidation gap. All four are behaviour the per-path probes had and this one had lost. torch.version is reached through getattr now. A torch without that module made the child exit non-zero, which reads as "torch cannot import" and force-reinstalls a working venv. The old version probe only touched torch.__version__ and answered fine. The answer is printed behind a marker and parsed by that marker. "Last non-empty line" is only correct while nothing chatters AFTER the answer, and an atexit handler or a CUDA teardown notice does exactly that. version is None when no marked line came back, distinct from "" for an empty __version__. The XPU and CPU guards keyed off the empty string, so a venv with an unreadable version silently stopped being repaired where it used to be. pip_install_try() clears the memo too. It installs the Windows AMD ROCm trio, so "only pip_install changes what is installed" was not true, and a stale classification could answer for the previous build. Tests: the mocked probe stdout in test_cuda_repair.py and test_rocm_support.py carries the marker, and test_torch_probe_memoization.py gains the marker cases plus a real-subprocess one for the missing torch.version module, since that path crashes the installer rather than misrouting it. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Prove the classification is a translation, and drop a leftover parser Two things the hardening pass left behind. The old "last non-empty line" parser is still sitting after the return in _probe_torch_runtime, unreachable. Removed; asserted by a test so a future in-place replacement cannot leave one again. More importantly, nothing checked the part of this PR most able to be quietly wrong. Moving the five probes out of subprocess `-c` strings and into ordinary Python is meant to be a translation, but every existing test feeds the repair paths a MOCKED probe answer, so they exercise the new derivation only and would pass just as happily if it disagreed with the old one. test_torch_probe_classification_parity.py compares the two directly. The old expressions are reproduced verbatim from the merge base as reference implementations, cited by line. The new derivations are pulled out of the live module with ast rather than copied, so they cannot drift from what ships, and a rename fails extraction loudly instead of silently passing. 27 torch states, covering cu118 through cu130 tagged and untagged, rocm by hip and by version string, rocmsdk, +xpu in and out of range, CPU, empty, and hip plus cuda both set. Mutation-tested rather than assumed, since a parity test that cannot fail is worse than none. Dropping the rocm check from the CUDA marker, widening the XPU bound, dropping .lower() from the CPU predicate or the ROCm marker, and swapping the hip/cuda fields in the probe are each caught. The .lower() mutations initially passed. The matrix had no state where a substring test was the deciding factor, so the uppercase rows are there to make case actually matter rather than for completeness. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
|
Daniel Han
|
99bcfd3d93
|
Keep the #8577 AMD peer guards message-only, and fix the table drift they exposed (#8689)
* Keep the peer guards message-only, and stop the shell Polaris arms over-matching Adversarial verification of the "message-only" claim found two ways this branch had started changing what gets installed, both introduced by my own review fixes. studio/setup.ps1: widening the adapter scan gate from $HasROCm to the arch let CIM fill $script:ROCmGpuLabels on the amd-smi path. That variable feeds $gpuNames and so the arch inference, and the existing unpinned "borrow another adapter's arch" rule then resolved an arch where none was resolved before. On a host where amd-smi confirms a runtime but reports no gfx token, an RX 9070 XT went from CPU torch to gfx1201, and an RX 5700 beside an RX 7900 went to gfx1100, with --rocm-gfx forwarded to the llama.cpp and whisper installers. install.ps1: the Get-WmiObject to Get-CimInstance swap had the same effect on PowerShell 7, where the old call threw and the catch swallowed it. A host CIM can see but the ROCm tools cannot went from CPU torch to a repo.amd.com index. Both scans are now exactly what they were before this branch, byte for byte, and the peer names live in their own variable that only the uncovered-card verdict reads: $wmiAmdNames in install.ps1, $script:ROCmPeerLabels in studio/setup.ps1. Neither block writes a label or an arch. The pwsh suite asserts that through the AST rather than asserting the gate, which is the property that actually matters; appending a label or arch assignment to either block fails it. The PowerShell 7 Get-WmiObject defect is therefore still there. Fixing it changes what every pwsh 7 AMD host without a HIP SDK installs, so it belongs in its own PR. Separately, the five copies of the unsupported table were not pinned to each other, only the supported ones were, and they had already drifted: the regex copies carry (?!0) so "RX 5800" is not Polaris, while the shell globs matched it through "RX 580". The shell arms now carry the same guard, and a new test compares all five tables by EVALUATING them in their own languages over a shared name corpus. Adding a row to one file alone fails it. Also pinned: a blank UNSLOTH_TORCH_INDEX_URL / _FAMILY must not read as a pin in the Python path, which dropping the .strip() previously passed. * Yield the peer suppression to an AMD visible-device mask The peer check walked every adapter without looking at HIP_VISIBLE_DEVICES or ROCR_VISIBLE_DEVICES, so on a mixed host a masked-out supported card could silence the verdict about the card the user had actually selected. Both installers now skip the suppression under a mask, which is the rule studio/setup.ps1 already applies to its arch-borrowing branch: the mask names the card, so the verdict is about that one. HIP and ROCR only, deliberately, not Test-VisibleDevicesPinned's set: CUDA_VISIBLE_DEVICES masks NVIDIA devices and says nothing about which Radeon was chosen, and counting it fired the verdict beside a covered Radeon on every host that sets it, which two of the pwsh cases caught immediately. Three assertions per file pin it. Emptying the mask list fails one, adding CUDA_VISIBLE_DEVICES back fails another. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Scan adapters with CIM and drop the masked verdict when it may name another card for PR #8689 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
5a5bf64130
|
Reduce antivirus false positives in the desktop installers (#8586)
* Windows setup: install uv from a pinned release instead of running remote script text
studio/setup.ps1 piped astral's install.ps1 straight into Invoke-Expression. That
download-and-execute shape is the single construct AMSI providers and cloud ML
scanners score hardest, and install.ps1 already replaced it with a pinned-SHA-256
archive download. Port the same implementation across.
Progress goes to the pipeline rather than the console, so the quiet path swallows
it exactly as it swallowed astral's installer output and the printed lines around
the call site are unchanged.
* Windows: stop pairing a hidden window with a bypassed execution policy
The Studio shortcut launched launch-studio.ps1 with -WindowStyle Hidden and
-ExecutionPolicy Bypass on the same command line. That pair is what Microsoft's
own detections key on, and studio/src-tauri/src/install.rs already refuses it for
the app's own launch of install.ps1.
The installer writes launch-studio.ps1 itself, so the file carries no
mark-of-the-web and RemoteSigned loads it. The hidden window is unchanged, so the
shortcut behaves exactly as before. The generated launcher's own child launch
moves to RemoteSigned for the same reason: it runs an inline -Command against an
executable, where no script file is loaded and the two policies are equivalent.
Also refresh a stale comment in studio/setup.ps1 that attributed the PSModulePath
fix to astral's uv installer, which no longer runs in-process.
* Installers: keep download-and-run command lines out of the shipped script text
AMSI scans install.ps1 in full before a single line of it runs, and generic
script classifiers read install.sh the same way inside the Linux bundle. Both
headers rehearsed the piped web one-liner five times over, plus a scriptblock
form and an execution-policy bypass, none of which anything in the scripts reads
and all of which the README already documents.
Point at the README instead and reword the in-body comments that quoted the
one-liner as shorthand. Every printed line is untouched: the remediation text the
installers show users still spells out the command in full.
Same treatment for scripts/uninstall.ps1's header.
* Windows: resolve process image paths with one Win32_Process query
install.ps1's venv-holder probe opened a handle to every running PID through
inline C# compiled at runtime. Opening a handle per process is a shape AV
heuristics score hard, and it bought nothing: Win32_Process reports
ExecutablePath for exactly the processes those handles could be opened against,
and answers for all of them in a single query instead of once per PID.
The remaining file-canonicalisation imports stay -- handle-based resolution of
linked ancestors has no faithful Windows PowerShell 5.1 equivalent, and it runs
on security-relevant paths.
Falls back to the per-process .Path when the query is unavailable, so a degraded
WMI repository degrades exactly as the old code did on a process it could not
open.
* Desktop: say who blocked the install when AMSI stops the script
PowerShell hands the whole top-level script block to AMSI while compiling it, so
a security product's verdict arrives as a parse error over the entire file before
install.ps1 runs a statement: no [TAURI:ERROR] marker, no phase log, and a stderr
tail the user cannot act on. unsloth#8523 shows what that looks like in the UI --
"Installation failed: + FullyQualifiedErrorId : ScriptContainedMaliciousContent".
Recognise the two stable error ids on either stream and append what the user
actually needs: nothing was installed, nothing was changed, it is a false
positive, update definitions and retry, do not turn off endpoint protection. The
raw id stays in the message, because the diagnostics report and any vendor
submission both need it.
Matches the id, never the message text, which is localized, and tolerates the
cmdlet suffix the Invoke-Expression form carries.
* Desktop: ship each bundle only the installer it can run
resolve_install_script picks install.sh on unix and install.ps1 everywhere else,
but the shared Tauri config bundled both into every target. The Linux AppImage
therefore carried 280 KB of Windows PowerShell it can never execute -- and it is
the largest script body a generic classifier walking the squashfs reads, which is
where Microsoft's Trojan:Script/Wacatac.B!ml verdict on 0.1.701-beta landed.
Move the resource map into the per-platform configs. The clean-machine job
already fails when a Linux bundle ships no install.sh; it now also fails when one
ships install.ps1, so the split cannot silently regress in either direction.
The .deb scanned clean with the same payload, so this is surface reduction rather
than a proven fix for that verdict.
* POSIX installers: install uv from a pinned release before falling back
install.sh downloaded astral's install.sh to a temp file, ran it and deleted the
file; studio/setup.sh piped it straight into a shell. Both are, shape for shape,
what a dropper does, and generic ML script classifiers score them accordingly --
the 0.1.701-beta Linux AppImage came back Trojan:Script/Wacatac.B!ml while the
.deb carrying the same scripts came back clean.
Fetch the pinned release archive and verify a hardcoded SHA-256 instead, matching
what install.ps1 already does on Windows. Only the four mainstream targets are
pinned: musl, armv7 and any host without a digest tool keep the path they have
today, because guessing a target triple wrong would break the install outright
and that costs far more than the heuristic score of the fallback.
Destination, PATH handling and every printed line are unchanged, so a host that
takes either path ends up in the same state it did before.
* tests: pin the installer shapes antivirus heuristics score
One file collecting what was removed, so it cannot drift back: no remote script
run in-process, no encoded or base64 payload, no hidden window paired with a
bypassed execution policy, no handle opened against another process, and no new
runtime-compiled native import outside an allowlist that carries a reason for
each entry that stays.
The last test is the other half of the contract. Hardening must not change what a
user sees, so the remediation lines the installers print -- which still spell out
the web one-liner in full -- are asserted verbatim. Removing the one-liner from
comments is the point; removing it from what the user is told to run would be a
regression.
Runs on the existing discovery-based pytest step, no workflow list to update.
* release: emit a false-positive submission packet for whatever gets flagged
The build job assembles a Microsoft submission packet, but only for the Windows
-setup.exe. The detection that actually arrived on 0.1.701-beta was
Trojan:Script/Wacatac.B!ml on the Linux AppImage, so nothing was produced for the
one asset that needed it.
The VirusTotal job already knows which assets were flagged and by which engines,
so put the packet there: hash, size and both portals, for every flagged asset
whatever platform it came from, with a note that clearance is per hash and per
vendor. Engine names are not repeated -- they are third-party text and already
appear escaped under Flagging engines.
The gate stays advisory; this only makes acting on it take seconds.
* Revert "Windows: resolve process image paths with one Win32_Process query"
This reverts commit
|
||
|
Daniel Han
|
d5a2160ef8
|
Say ROCm does not cover RDNA 1 instead of advising a fix that cannot work (#8577)
* Say ROCm does not cover RDNA 1 instead of advising an impossible fix An RX 5700 XT (Navi 10, gfx1010, RDNA 1) correctly lands on CPU PyTorch: AMD publishes Windows torch indexes for gfx103X, gfx110X, gfx1150, gfx1151 and gfx120X, and there is no gfx101X index. Because the name-inference table covers only arches that have wheels, the arch stayed null and the installer fell into the "arch unknown" arm, which tells the user to install the HIP SDK or set UNSLOTH_ROCM_GFX_ARCH. Neither can work: UNSLOTH_ROCM_GFX_ARCH=gfx1010 lands on the unmapped-arch path and returns CPU anyway. Add a separate name lookup for AMD generations ROCm PyTorch does not cover, read only to word the report. Product names come from LLVM's AMDGPU GFX10.1 processor table. The lookup never sets the arch the installers route on, so CPU fallback is reached by exactly the same path as before. Mirrored across install.ps1, studio/setup.ps1, install.sh, studio/setup.sh and studio/install_python_stack.py so every install path agrees. * Point pre-RDNA 2 AMD users at the Vulkan llama.cpp path The previous commit stopped at "ROCm does not cover this GPU", which is true and still a dead end. There is a working path: llama.cpp's Vulkan bundle drives these cards, which is how #8458's reporter got an RX 580 running and how LM Studio drives the same hardware. Nothing routes these users there automatically. _should_auto_vulkan_for_amd_windows opens with `active = _active_rocm_gfx_target(host); if not active: return False`, and a pre-RDNA 2 card resolves to no gfx target at all, so the Windows auto-Vulkan fallback structurally cannot fire for exactly the cards that need it. The environment variable is their only route, so the message now names it. Two things about that advice are load-bearing and both are tested: - The current spelling, UNSLOTH_LLAMA_CPP_BACKEND=vulkan. The legacy UNSLOTH_FORCE_VULKAN still works but force_vulkan_requested() resolves the new variable first and consults the legacy one only when the new one is absent or unparseable, deliberately, so =hip stays a real opt-out that a stale legacy variable cannot overrule. New text must not spread the legacy name. - WHEN to set it. The variable picks the llama.cpp bundle at install/download time; nothing reads it at runtime to choose a binary. #8458's reporter set it after installing, saw no change, and only a clean reinstall worked. Advice that names the variable without naming the moment is worse than none. Also adds Polaris 10/20/30 (RX 470/480/570/580/590, gfx803) to the messaging-only table so #8458's card gets the right message. gfx803 stays out of _GFX_TO_AMD_INDEX_ARCH and every supported-arch table, and routing is untouched; a test pins that directly. Polaris 11/12 (RX 460/550/560) is left out because gfx803 vs gfx804 could not be confirmed for that die, and this table is only worth having while it never guesses. "RX 570" is a prefix of "RX 5700" and "RX 550" of "RX 5500". Python and PowerShell carry (?!0) lookahead guards; POSIX `case` has no lookahead, so in install.sh and studio/setup.sh correctness rests on arm order and the RDNA 1 arms come first. That order is now documented and asserted, and the shipped arms are evaluated in a real shell rather than checked by eye. README leads with the current spelling, since the installer now names a variable and the README is where users check it. Finally, test_cpu_index_note_respects_explicit_pin asserted a pin check appeared within a character window before a note. That is a budget on intervening source, not the ordering property it is for, and this work had already pushed it from 400 to 1400. It now walks the enclosing if/elif chain by indentation, so it tests order and has no distance left to re-tune. * Stop the HIP SDK arm outranking the pre-RDNA 2 message, and fix its advice An RDNA 1 or Polaris user who already installed the HIP SDK never saw the new message: the $HipSdkInstalled arm sits earlier in the chain and told them the ROCm compute driver was missing, which is the impossible remediation this change exists to remove, and those users installed the SDK because the old advice said to. Guard that arm, plus the matching CPU-hint arm in install.ps1, on the unsupported arch. The Vulkan setter was printed as UNSLOTH_LLAMA_CPP_BACKEND=vulkan by the two PowerShell installers and by the Windows-only branch in install_python_stack.py. PowerShell parses that as a command name, so a user who pastes it sets nothing and the next install picks the same CPU bundle. Print $env:UNSLOTH_LLAMA_CPP_BACKEND = "vulkan" there, as the README already does. The arms also said PyTorch training runs on CPU on these GPUs. It does not: with no CUDA or XPU accelerator, unsloth raises NotImplementedError at import, which is why studio/setup.sh already tells its other CPU-torch hosts that training and GPU inference are unavailable. Say the same thing here. README: Vega 20 (Radeon VII, MI50, gfx906) is older than RDNA 2 and does have a ROCm PyTorch path (install.sh routes it to rocm6.3), so name Polaris and RDNA 1 instead of every pre-RDNA 2 AMD GPU. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Guard the README gfx906 carve-out against any spelling of the cutoff The ban was on one exact literal, so "every AMD GPU older than RDNA 2" passed while contradicting the Vega 20 carve-out two sentences later. Match the phrase family instead, and assert the group is named by its members (Polaris, RDNA 1) rather than by a generation cutoff. * Tighten the comments added by this PR Comments only, no code or user-facing string touched. Every reason a guard exists is kept, just said in fewer lines. * Keep the note on why the rationale sits above the arm * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: initialise the unsupported-arch state outside the AMD detection block The ROCm summary reads $script:ROCmUnsupportedGfxArch unconditionally, but the assignment sat inside `if (-not $HasNvidiaSmi)`, so an NVIDIA host never defined it and a caller's Set-StrictMode turned the summary into an aborting undefined-variable error. ROCmGfxArch beside it was always initialised at top level; this one was not. The guard asks the PowerShell parser whether any assignment is unnested rather than comparing line numbers: the file has three -not $HasNvidiaSmi blocks, so an ordering check picks the wrong one and passes for the wrong reason. * Name the remaining RDNA 1 boards, diagnose the KFD-only host, and keep mixed AMD hosts on the arch-unknown advice Adds the Navi 10 / Navi 14 professional boards LLVM's processor table omits (Radeon Pro W5700/W5700X -> gfx1010, Pro W5500/W5500M/W5300M and RX 5300/5300M -> gfx1012) to all five copies of the unsupported-name table. Each mapping comes from libdrm data/amdgpu.ids read against pci.ids and the kernel amdgpu PCI table, not from a guess; the tables still route nothing. studio/setup.sh's KFD sysfs fallback detects the GPU without rocminfo or amd-smi, so it left the marketing name empty and the report fell through to a plain AMD ROCm line on a host with no ROCm. It now reads lspci for that report only, never writing it back into the name the supported table and --rocm-gfx key on. install.ps1's WMI fallback classifies adapter 0 only, so a host pairing an RX 5700 with an RX 7900 was told nothing could enable ROCm, which is false there. The verdict is now withheld when another adapter is covered, leaving the arch-unknown advice that does apply. install_python_stack.py and studio/setup.ps1 already scored every adapter. * Scope the uncovered-arch verdict to the card it names, and name the boards it was missing A host is not one GPU. On a box pairing an uncovered card with one that has wheels -- an RX 580 beside an RX 7900 XTX, or beside an Instinct MI210 -- "setting UNSLOTH_ROCM_GFX_ARCH will not enable ROCm PyTorch" was false: masking to the other card and pinning its arch installs them, and install.sh routes exactly that host to gfx110X-all a few lines earlier. Every advice site now says what is true of the card it just named and claims nothing beyond it. Deciding it at runtime was tried and dropped. Reading "an AMD adapter neither table names" as a working peer misfires on the Vega-class iGPU (Raven through Cezanne, Mendocino) that sits beside the dGPU on most Ryzen desktops and has no ROCm torch path of its own, which would trade a correct dead stop for the open-ended errand this change exists to remove. Reading only the supported table misses the Instinct and V620 parts that are routable and appear in no name table at all. Neither rule is right often enough to speak for a host. Four real boards are added to all five copies of the message-only table: Radeon Pro 5700 / 5700 XT (pci.ids 7319 and 731b, Navi 10, gfx1010) are the only Navi 10 retail parts whose name carries neither "RX 5700" nor a W prefix, and Radeon Pro WX 7100 / WX 5100 (Ellesmere, gfx803) carry no RX number at all, so both fell through to the generic advice. Provenance from pci.ids as before, and the shell case arms are matched case-sensitively, which is now stated where only the arm ordering was. Tests: - The absolute host-wide phrasings are banned from all five sources, with the scoped replacements required per site so deleting the sentence cannot pass. - test_unsupported_arch_routing_guards_8529.py drives the real index resolvers rather than asserting table shape: install.sh's get_torch_index_url and _amd_arch_index_family_for_gfx under sh, the .ps1 family maps under pwsh (including writes after the declaration, and the -contains list), the Python resolvers on both platforms, the Strix per-arch reroute, and studio/setup.sh's report-only lookup. Positive controls throughout. - _assert_guarded_by_pin_arm matches the pin arm exactly rather than by prefix, binds the message's own enclosing arm, requires the pin arm to still say its note, ignores comment lines, and detects inline and re-indented chain closes. Mutation tested: 13 mutants covering a post-declaration map write in each .ps1, a single-quoted arch in _rocmWheelArches, a bypass inside get_torch_index_url, the Strix arm, feeding the unsupported lookup into _setup_gfx directly and one hop later, an inline-closed pin chain, a re-indented fi, and removing each new table row. All killed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Say whose wheels are missing: AMD ships RDNA 1 PyTorch, Unsloth does not install it "No ROCm PyTorch wheels exist for that arch" is no longer true of RDNA 1. AMD's TheRock lists device-gfx1010, device-gfx1011 and device-gfx1012 as installable torch extras on its multi-arch index, and SUPPORTED_GPUS.md marks all three Build Passing, Sanity Tested and Release Ready. gfx803 is absent from that table entirely, and no GCN4 family appears at all, so the Polaris half stands. The claim these installers can honestly make is about their own routing, not about ROCm at large: repo.amd.com publishes gfx103X/110X/1150/1151/120X and nothing for gfx101X or gfx80X, so UNSLOTH_ROCM_GFX_ARCH=gfx1010 still lands on the unmapped path and still returns CPU. Every site now says Unsloth has no wheels for the arch rather than that none exist, and the tables carry a note saying why the wording is scoped. Routing, the CPU fallback and the Vulkan advice are all unchanged. Verified by running the merge base and this branch side by side under identical stubbed hardware, and diffing: - POSIX shell, 127 simulated hosts x 2 blocks = 254 rows, each run in both trees. Selected torch index URL differed in 0 rows, exit code in 0 rows, get_torch_index_url stdout in 0 rows. The 32 rows whose end-of-run summary text moved are all AMD arch-unknown hosts resolving to gfx1010/1011/1012/803. Covers linux/wsl/macos/aarch64, NVIDIA at five CUDA levels, 15 supported gfx arches, multi-GPU lspci mixes, every override, dash and bash, set -eu, and lspci absent/failing/hanging. Asserted separately that the new WARN lines go to stderr, so TORCH_INDEX_URL=$(get_torch_index_url) is never polluted: 0 of 127 index rows had anything but one URL on stdout. - PowerShell, 232 cells over 116 adapter inventories x install.ps1 and setup.ps1. Resolved arch, index URL, arch family, routing flag and the gfx handoff are identical in every cell. 80 cells changed text, all RDNA 1 or Polaris. An RDNA 1 card beside an RX 7900 keeps the old wording and still resolves gfx1100, which is the covered-peer guard doing its job. Under Set-StrictMode the new code passes only because of the variable initialisation added outside the detection block; removing it fails. - Python stack, 23034 value comparisons over 5 platforms x 4 GPU classes x 38 arch inputs x 99 adapter names x masks x overrides x 6 mirror configs. 0 value differences. No unsupported arch produced an AMD index URL by any path in either tree, and no supported arch changed URL. A negative control that adds gfx1010 to the routing map reports 83 differences, so the zero is real. - Existing installs: a manifest written by the old code verifies identically under the new code and vice versa, over 12 write/read combinations; a manifest poisoned with gfx_arch and index-url keys changes no verdict, because nothing arch-shaped is persisted. The legacy UNSLOTH_FORCE_VULKAN resolves identically across all 90 combinations with UNSLOTH_LLAMA_CPP_BACKEND, including falsey values and =hip overriding a stale truthy legacy value. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Declare the unsupported-arch variable where its readers can see it $ROCmUnsupportedGfxArch was declared inside `if (-not $HasNvidiaSmi)` in install.ps1, but the arms that read it sit outside that gate, so on an NVIDIA host the read is of a variable that was never assigned. Its five neighbours (HasROCm, HipSdkInstalled, ROCmGpuLabel, ROCmVersion, ROCmGfxArch) are all declared above the gate, and studio/setup.ps1 already hoists its own copy for exactly this reason, so this one was the odd one out. Harmless as shipped, because Install-UnslothStudio runs with Set-StrictMode off. Under a caller's `Set-StrictMode -Version Latest` it is a hard stop: driving the extracted blocks under pwsh, an NVIDIA host that lands on the /cpu leaf (a pre-CUDA-11 driver, or UNSLOTH_TORCH_INDEX_URL pinned to cpu) throws on the read. Found by running this branch and its merge base side by side over 240 Windows cells; every resolved arch, index URL, arch family, routing flag and gfx handoff matched, and this was the only asymmetry that was not message text. A test pins the declaration above the gate in install.ps1 and at script scope in setup.ps1. Moving it back inside the block fails the test. * Let an identified uncovered card outrank the generic ROCm report, and teach export Two review findings, both reproduced first. amd-smi can report a GPU with no gfx token anywhere in `list` or `static --asic` and only a market name. That sets $HasROCm with no arch, so the generic `} elseif ($HasROCm)` arm fired and called an RX 5700 XT "AMD ROCm (AMD Radeon RX 5700 XT)" while the wheel note in the same run said gfx1010 has none. Driving the real detection and step chain under pwsh with a stubbed amd-smi reproduces it exactly, and the host is not hypothetical: amd-smi is only probed when the HIP SDK is present, which is what the #8529 and #8458 reporters installed because the old message told them to. Both scripts now carry the same `-and -not $ROCmUnsupportedGfxArch` guard the HIP SDK arm below already had. A supported gfx1100 host and an unmapped Instinct MI210 host are unchanged on the same harness, since the guard is a no-op when no arch was identified. The POSIX advice said to `set UNSLOTH_LLAMA_CPP_BACKEND=vulkan` and re-run the installer. A bare assignment is a shell variable, not an environment entry, so the installer subprocess never sees it and the user gets the CPU bundle again: $ sh -c 'UNSLOTH_LLAMA_CPP_BACKEND=vulkan ./installer' -> installer sees: [<unset>] $ sh -c 'export UNSLOTH_LLAMA_CPP_BACKEND=vulkan ./installer' -> installer sees: [vulkan] That is the #8458 failure mode reintroduced by the fix for it. The README block has always used export; the three POSIX message sites now agree with it. The PowerShell sites already used `$env:`, which is the process environment, so they were correct and are untouched. Tests pin both: the emitted POSIX setter is now `export ...`, and each generic ROCm arm must carry the unsupported guard. Dropping either guard fails. * Guard the ROCm summary chain the same way its two siblings are The summary at studio/setup.ps1 opens with a bare `if ($HasROCm)` rather than an `} elseif`, so it was missed when the other two chains were guarded. Its own third arm names the uncovered card, and that arm is only reached when nothing outranks it. On a host where amd-smi enumerates an RDNA 1 card with no gfx token, the "ROCm x.y" arm wins and the arm written for that card never runs, so the same run reports ROCm here and no wheels below. The existing check only matched arms opening with `} elseif ($HasROCm`, which is why it walked past this one. The new test finds the chain by its own body. * Keep banning the bare POSIX setter in the PowerShell sources Requiring `export` in the .sh advice was done by redefining _POSIX_SETTER, which is also the needle two PowerShell bans read. With the export folded in, a .ps1 that printed a bare UNSLOTH_LLAMA_CPP_BACKEND=vulkan no longer matched either ban: a mutant adding exactly that line passed all 261 tests in the file. Split the two. _POSIX_ASSIGNMENT is the bare form the Windows sources must never print, and _POSIX_SETTER stays the exported form the POSIX ones must teach. The same mutant now fails, and reverting either export still fails. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments on the unsupported-arch path Final pass over the comments this branch added. The five copies of the table header each repeated the same four points at four different lengths, so they are now one block of the same wording everywhere, and the provenance and scoped-claim notes are folded into it rather than trailing it. Same for the report arms and the test rationales: no reason removed, fewer lines to read. Comments, docstrings and wrapping only. AST-checked with comment_tools.py (4/4 code-unchanged), `bash -n` on both POSIX scripts, and the PowerShell parser on all three .ps1 files. Suites re-run: 1036 passed, 1 skipped, and the pwsh behavioural suite green. * Blame the right card in the CPU summary, and stop promising macOS Vulkan Two more review findings, both reproduced first. The end-of-run CPU summary calls the lspci lookup unconditionally, so unlike the arm in get_torch_index_url it is not covered by the empty-probe gate. On a host pairing an RX 5700 with an RX 7900, a CPU fallback caused by the 7900's ROCm being older than 6.0 was attributed to the 5700, replacing the "upgrade ROCm" advice with advice that is false for the card that actually caused it. Running the shipped guard under sh with a stubbed lspci, only that host moves: lone RX 5700 uncovered-card message -> unchanged lone RX 580 uncovered-card message -> unchanged lone RX 7900 generic message -> unchanged lone MI210 generic message -> unchanged RX 5700 + RX 7900 uncovered-card message -> generic message The summary now asks _infer_linux_amd_gfx_arch, which scans every display adapter, and stays quiet when any of them is covered. Same shape as the peer guard install.ps1 already carries. The README's Vulkan paragraph sat under the combined "macOS, Linux, WSL" heading. macOS has no Vulkan llama.cpp bundle: install_llama_prebuilt.py logs that the variable is ignored and installs the Metal build, and upstream ships no macOS Vulkan asset either (Metal is the default there, and Vulkan on macOS only exists through MoltenVK, which you have to build yourself). An Intel Mac carrying one of these very cards, the 16-inch MacBook Pro shipped the Radeon Pro 5300M/5500M/5600M, would follow the command and get nothing. The paragraph now names Linux and WSL and macOS gets its own sentence. The installer itself needed no change: get_torch_index_url returns before any AMD probe on Darwin and the summary is already gated on it, so the advice was never emitted there. Both guards are pinned by tests that fail when the guard is dropped. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added in the last pass * Guard the Studio report's peer scan, explicit index pins, and Windows ARM64 Three more review findings, all reproduced first. studio/setup.sh had the same misattribution I fixed in install.sh last round, on the KFD path where neither rocminfo nor amd-smi answers and the lookup falls back to lspci. First match wins, so a host whose RX 5700 enumerates before an RX 7900 was told no override could help, which is false there. The supported name table is now a matcher, _setup_supported_gfx_from_name, with its arms byte-identical to before, so the scan can ask about a peer without touching $_setup_gfx. Same fixtures as the install.sh guard, run under sh: lone RX 5700 / RX 580 named -> unchanged RX 5700 + RX 7900, either order named -> quiet RX 580 + RX 7900 named -> quiet An explicit UNSLOTH_TORCH_INDEX_URL or _FAMILY reaches the ROCm install path for any gfx*/rocm* leaf, so "torch stays CPU-only and neither the HIP SDK nor UNSLOTH_ROCM_GFX_ARCH changes that" was false on a pinned run. install.sh's CPU note already skipped its guidance when pinned; install.ps1, studio/setup.ps1, studio/setup.sh and install_python_stack.py now agree with it. install.ps1's second site needed nothing, since it already sits behind -not $ROCmIndexUrl. studio/setup.ps1 throws on UNSLOTH_LLAMA_CPP_BACKEND=vulkan on Windows ARM64, where no Vulkan bundle is published, so the advice aborted the next update instead of enabling GGUF acceleration. Both PowerShell sites now branch on Get-HostMachineArch and point at a source build there. The advice-window test had to change with them: the claims now sit in if/else arms, and a fixed 8-line window either stopped mid-branch or spilled into the next arm, which is the failure its own docstring warns about. It walks to the end of the enclosing arm instead, capped. Both bash table-parity tests follow the matcher's new variable names and still compare the same rows. Each guard is pinned by a test that fails when it is dropped: the peer loop, the pin check, and the Get-HostMachineArch call were each mutated and killed. * Cut the comment lines that restated the message below them * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fill the Windows peer list on the amd-smi path, and three smaller corrections Four more review findings, all reproduced first. install.ps1's WMI scan sat behind `if (-not $HasROCm)`. amd-smi can report GPUs with no gfx token and only the first market name, which sets $HasROCm with no arch, so the scan was skipped and the peer guard added last round saw an empty list on exactly the multi-GPU host it exists for. It is now keyed on the arch, which is the condition under which the unsupported lookup can run at all: a host that already has an arch still does no WMI work here, and amd-smi's label still wins when it had one. The pwsh suite asserts the gate through the AST. studio/setup.sh returned the failure of a nonempty market name immediately, so a generic "AMD Radeon Graphics" from rocminfo ended the lookup before the lspci scan and the report fell back to the plain "AMD ROCm" line this change is meant to replace. It now returns only on a hit; a name that maps still short-circuits without touching lspci, and the peer guard still covers the mixed host. install_python_stack.py prints the same Vulkan advice as install.ps1 on the Windows WMI path, and the same ARM64 throw applies to it. Added _is_windows_arm64(), mirroring Get-HostMachineArch down to the PROCESSOR_ARCHITEW6432 case an emulated x64 Python needs. The setup.sh pin check treated a whitespace-only value as a pin, while get_torch_index_url trims both variables and treats a blank one as unset. It is trimmed the same way now. The variable also collided with the XPU block's _setup_pin, which is a global in POSIX sh, so it is renamed _setup_unsup_pin. One existing test asserted the behaviour the second finding calls a bug, that an unrecognised reported name claims nothing. Rewritten to the corrected intent, keeping a case that proves a covered card is still never claimed from lspci. test_windows_amd_gpu_scan_fallback.py extracts the WMI block by its literal gate, so its anchor follows the new condition. Each guard is pinned by a test that fails when it is dropped: all four were mutated and killed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Cover the Python emitter in the ARM64 Vulkan-offer guard * Fill the Studio peer list on the amd-smi path too studio/setup.ps1 carries the same WMI scan install.ps1 does, for its own name inference, and it had the same gate. amd-smi can report GPUs with no gfx token and only the first market name, which sets $HasROCm with no arch, so the scan was skipped, $script:ROCmGpuLabels held one name, $gpuNames was one entry, and a host pairing an RX 5700 with an RX 7900 was judged entirely on the 5700 even with HIP_VISIBLE_DEVICES=1 selecting the 7900. The scan is now gated on the arch, which is the condition the inference block below already runs under, so the two can no longer disagree about whether there is anything to infer from. A host that already has an arch still does no WMI work here, and amd-smi's label still wins when it had one: only the peer list is new on that path. The pwsh suite walks the AST from the $script:ROCmGpuLabels assignment to its enclosing if and asserts the condition names the arch and not $HasROCm, as it already does for install.ps1. Restoring the old gate fails it. tests/test_windows_amd_gpu_scan_fallback.py extracts that block with a regex anchored on the literal gate, so its pattern follows the new condition. * Run the covered-peer guard before the named hit too The guard added last round only covered setup.sh's lspci fallback, so a named hit still walked past it. amd-smi reports one market name, the first device's, so on a host whose RX 5700 precedes an RX 7900 the name IS the uncovered card and the false verdict came back through the other door. The scan now runs first, for both paths: lone RX 5700, named named -> unchanged lone RX 580, named named -> unchanged RX 5700 + RX 7900, named 5700 named -> quiet RX 5700 + RX 7900, no name quiet -> unchanged It must not become a silencer, so it applies only when lspci can answer: with no adapter list there is no peer to find, and the single-card host this report exists for still has to be told. A test drives the lookup with lspci off PATH and requires the verdict to survive. Putting the named hit back in front fails the two mixed-host cases while that no-lspci control still passes, so the ordering is what is pinned, not just the presence of the guard. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added since the last pass * Ask CIM for the adapter list, not the cmdlet PowerShell 7 dropped install.ps1:3331 was the only live Get-WmiObject call left in the file; every other WMI query in it already asks Get-CimInstance, and the script handles PSEdition Core explicitly. Get-WmiObject was superseded by Get-CimInstance in PowerShell 3.0 and removed outright in PowerShell 7, so on pwsh the call threw, the block's own catch swallowed it, and $wmiAmdNames came back empty. That is the peer list the guard added two rounds ago reads, so on pwsh the guard could never fire and the amd-smi path went back to blaming the uncovered card. Same class, same properties, and the CIM cmdlets ship with 5.1 as well, so the swap costs nothing downlevel and matches what studio/setup.ps1 already does. The harness stubbed Get-WmiObject, so it was answering for a cmdlet the installer no longer calls. It now stubs Get-CimInstance and defines Get-WmiObject to throw, so a revert fails loudly instead of quietly returning nothing through that catch. Reverting the call fails four tests. * Tighten the two comments from the last pass --------- Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> |
||
Leo Borcherding
|
5a3e9fc7a6
|
fix(amd): gate ROCm GPU selection and crash recovery on the build's arch coverage (#7670)
Some checks are pending
Unsloth export capability / capability (windows-latest) (push) Waiting to run
Frontend CI / Frontend build + bundle sanity (push) Waiting to run
Unsloth GGUF CI / OpenAI, Anthropic API tests (push) Waiting to run
Unsloth GGUF CI / Tool calling Tests (push) Waiting to run
Unsloth GGUF CI / JSON, images (push) Waiting to run
Unsloth load-orchestrator CI / test (push) Waiting to run
Mac Studio GGUF CI / GGUF inference smoke (API, tools, vision) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-26) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-15) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-15-intel) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-26-intel) (push) Waiting to run
Mac Studio UI + API + Update CI / Chat UI, API and Update Tests (push) Waiting to run
Unsloth Tauri CI / Tauri Linux debug build (no codesign) (push) Waiting to run
Unsloth Tauri CI / Rust unit tests (windows) (push) Waiting to run
Unsloth Tauri CI / Rust unit tests (macos) (push) Waiting to run
Unsloth UI CI / Chat UI Tests (push) Waiting to run
Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Windows Unsloth API CI / Unsloth API & Auth Tests (push) Waiting to run
Windows Unsloth GGUF CI / GGUF inference smoke (API, tools, vision) (push) Waiting to run
Windows Unsloth GGUF CI / Unsloth install + inference without Visual Studio (push) Waiting to run
Windows Unsloth GGUF CI / GPU prebuilt resolves without Visual Studio (push) Waiting to run
Windows Unsloth GGUF CI / setup.ps1 unit tests (VS 2026 / CMake guard) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2022) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2026) (push) Waiting to run
Windows Unsloth GGUF CI / VC++ runtime detect + install round-trip (windows-2025-vs2026) (push) Waiting to run
Windows Unsloth GGUF CI / VC++ runtime detect + install round-trip (windows-latest) (push) Waiting to run
Windows Unsloth UI CI / Chat UI Tests (push) Waiting to run
Windows Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Wheel CI / Wheel build + content sanity + import smoke (push) Waiting to run
Workflow trigger lint / workflow-trigger lint (pull_request_target / cache-poisoning) (push) Waiting to run
* fix(studio): gate ROCm GPU auto-selection on the installed prebuilt's built archs (#7624) The free-memory rank could pick a device the installed llama.cpp binary has no kernels for (e.g. an iGPU reporting shared system RAM outranking the dGPU), crashing llama-server with 'device kernel image is invalid'. - install_llama_prebuilt: record gfx_target + mapped_targets in UNSLOTH_PREBUILT_INFO.json for published ROCm bundles - llama_cpp: drop devices whose gcnArchName is not in the recorded list before ranking; unknown coverage or unknown device arch fails open * fix(studio): retry llama-server on remaining GPUs after a 'device kernel image is invalid' crash (#7624) Custom-linked llama.cpp builds have no mapped_targets manifest, so the proactive arch gate cannot see them. Catch ROCm's arch-mismatch crash on an auto-pinned device and respawn once with that device excluded, letting an arch-compatible sibling (e.g. the dGPU next to an unsupported iGPU) take the load. Explicit user device picks keep their error. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * refactor: reuse existing marker/arch helpers for the GPU arch gate - _installed_llama_gfx_archs now reads the install marker via llama_cpp_freshness.read_install_marker (walks up from the resolved binary), replacing a hand-rolled root list that missed custom studio homes and duplicated the freshness util - extract _rocm_arch_by_physical_id from _amd_apu_wants_unified_memory and share it with the enumeration gate instead of a second inline gcnArchName loop * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Run the formatter over the touched files * Studio: pin which callers of the ROCm GPU probe opt into the arch gate (#7624) * Studio: fail the ROCm arch gate open on non-concrete mapped_targets (#7624) mapped_targets is remote data: it comes from llama-prebuilt-manifest.json, which the release pipeline versions independently of this code. The gate is exact-set membership against the arch a device reports, so a token no device can ever report matches nothing and drops EVERY GPU, silently forcing CPU. That is worse than not gating at all, and it is reachable today from a malformed marker ([null] and [123] both produce such a set) as well as from a future publish recording a ROCm generic code object (gfx11-generic) or an umbrella family label (gfx110X). Treat an arch list that is not entirely concrete gfx tokens as unknown and keep every device. All-or-nothing on purpose: dropping just the odd token out of [gfx1100, gfx11-generic] would under-report what the build covers. Also pin the compatibility contract that had no coverage: a marker corpus (absent, empty, invalid, truncated, list-not-dict, null/string/dict/int targets, unreadable, directory-in-place-of-file, huge list) that must resolve to a sane arch set or None and never raise; that the new keys leave the install fingerprint byte-identical, so existing installs do not refresh on upgrade; that non-ROCm bundles record no archs; that the marker rewrite path preserves the fields; and that the family-label match still records the concrete list. * Studio: OS x GPU-vendor matrix tests for the ROCm arch gate (#7624) * Studio: degrade to CPU when the ROCm arch gate covers no GPU (#7624) The gate returning an empty pool left gpu_indices None, so the launch took the --fit on arm, the pin block never ran, and no HIP/ROCR/CUDA mask was written. The child still enumerated every unsupported card and died exactly as #7624 reports, and the reactive arch-crash retry could not help because its guard needs a truthy gpu_indices. Mask the child onto the CPU instead and say why, on auto placement on a ROCm host only. Also: - the arch-crash retry enumerates _detected_gpus, not gpus, matching what the region's own comments say to use; - the retry withdraws GGML_CUDA_ENABLE_UNIFIED_MEMORY when it moves to a discrete card, but only when this launch set it, so a user value stands; - test_single_gpu_host_has_no_retry counts probe calls instead of raising: the narrowing branch runs under except Exception, which swallowed the raising spy and left the test green with the guard removed; - correct the justification for gating automatic placement only. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: carry the ROCm arch gate into the embed launch and the crash retry (#7624) Two ways the gate stopped short of the process it was protecting. The embedding llama-server probe reduced the gated device list to a bool, so on a mixed host (unsupported iGPU plus supported dGPU) the gate passed on the dGPU and the child still enumerated every ROCm agent -- and that HSA enumeration is what dies on an arch the prebuilt has no kernels for. Pin the survivors with the ROCr-layer mask instead. Inert unless the install marker reports concrete mapped_targets AND the gate actually narrows the set, so NVIDIA, CPU-only, Vulkan and macOS write no mask and skip the probe entirely. The arch-crash retry masked devices out but respawned the planned argv verbatim. --tensor-split shares are positional over the child's visible devices (llama.cpp parses them by index, then copies the first n_devices), so the narrowed set re-indexes the survivors and hands them the crashed set's weights, which can overcommit a card. Drop the flag for the respawn and fall back to llama.cpp's free-VRAM split, which is correct for any device set. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: pin the arch gate's survivors when the fit owns placement (#7624) Two more places the gate stopped one step short of the child process. When the gate narrows a pool without emptying it, and the model is then too large for the planner, _select_gpus returns (None, True) and gpu_indices stays None. No branch in the env block writes a visibility mask on that arm, so the child enumerated the very card the gate had just dropped and died on it, and the reactive retry could not help either since its guard needs a truthy gpu_indices. That is the same reasoning the forced-CPU branch already spells out, one card short of forcing CPU. Mask the survivors in, last in the chain so a manual --tensor-split sized over the full visible count is not re-indexed under it. The probes behind it are lazy, so an ordinary pinned launch still pays for neither, and _arch_gate_survivors now backs the embed pin too. A forced-CPU launch also left _gpu_offload_active at None, because the gated probe leaves the detected list empty and the counted classifier answers None there. routes/training_vram.py spares a server only on 'is not False', so a server masked onto the CPU with no device visible at all was unloaded before training even though its death frees no VRAM. Record it as False, the contract the neighbouring comment already states. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: gate the manual-split launch and drop tensor mode on the CPU fallback (#7624) The manual per-GPU ratio took its own env branch, which re-emits the WHOLE visible GPU order, so the arch gate's answer was thrown away and the child got handed the card the build has no kernels for. Manual memory mode is not an explicit device pick: the probe already opts into the gate on 'not gpu_ids', and the ratio was sized from the full enumeration, not chosen per device. Fold that arm into the survivor branch: mask the survivors, and drop the ratio, which was sized over the full visible count and cannot survive the re-indexing that masking causes. Same trade the arch-crash retry already makes. A host the build fully covers keeps both its ratio and its PCI order pin. The forced-CPU branch could also be reached with --split-mode tensor still in the argv. Manual mode admits tensor parallelism on the full torch device count, which still counts the cards the gate dropped, so neither the <2-GPU manual guard nor the auto planner's zero-GPU guard fires. llama.cpp's llama_prepare_model_devices then fails the load with 'LLAMA_SPLIT_MODE_TENSOR needs >= 1 devices' and llama-server exits 1, turning the CPU fallback into a server that refuses to start. Strip the split flags there. Scoped to the gate: the zero-offload arm already drops Studio's own flags, and a user --split-mode there is deliberately overridden rather than stripped. * Studio: finish the gate's env handoff on the CPU and narrowed arms (#7624) The embed server's CPU arm only blanked CUDA_VISIBLE_DEVICES. HIP consults that variable only when HIP_VISIBLE_DEVICES is unset, and clr treats an empty value as no mask at all, so on ROCm the blank mask hid nothing and a load that had chosen the CPU still handed the child devices and the VRAM their contexts cost. Set the -1 sentinel at the HIP layer as well. An inherited ROCR mask is deliberately left alone: it hides agents below HIP, so clearing it would expose more of them to the HSA enumeration that dies on an uncovered arch. GGML_CUDA_ENABLE_UNIFIED_MEMORY is decided against gpu_indices, which is None on both arms the survivor mask serves, so an uncovered APU anywhere on the host turned it on and the narrowing then handed the child only discrete cards, where the same code calls the setting harmful. Withdraw it there, with the arch-crash retry's ownership check, so only a value this launch set is taken back and a deliberate user one stands. * Studio: finish the arch-crash retry's handoff onto the narrowed set (#7624) The respawn only handled one direction of each transition. A markerless mixed host can crash on the discrete card and land on the APU, where the first launch correctly left GGML_CUDA_ENABLE_UNIFIED_MEMORY unset, so the retry now sets it as well as withdrawing it. And narrowing to a single device makes --split-mode tensor a no-op that was still reported as active through the tensor_parallel property, which drives the UI and the MTP tensor watchdog. * Studio: re-run the APU RAM preflight for the arch-crash retry (#7624) The unified-memory RAM guard runs once, against the selection the first spawn used. On the mirror shape -- crash on the discrete card, retry on the unified-memory sibling -- that set is discrete, so the guard never fires, and the retry then switches GGML_CUDA_ENABLE_UNIFIED_MEMORY on for a load that now goes to system RAM. An oversized GGUF was OOM-killed mid-load instead of getting the actionable refusal the same host returns when the APU is picked first. Ask the same guard again for the set we are about to spawn on. * Studio: leave an unmappable GPU mask alone when the arch gate pins (#7624) * Studio: finish the proactive arch gate's handoff onto the survivors (#7624) The gate's survivor pin dropped only --tensor-split, so a manual load that kept tensor parallelism on the full device count ran one device under --split-mode tensor while /status still advertised the mode and the MTP tensor watchdog stayed armed. Drop the mode with the ratio, as the arch-crash retry already does. The APU RAM preflight runs with gpu_indices None on an unpinned launch, so an uncovered APU refused a load the surviving discrete card would have held. Re-ask it for the survivors, on the refusal branch only. The forced-CPU mask went through the default HIP arm, which clears an inherited ROCR mask. HIP -1 hides every device on its own, so the clear only re-exposed agents the parent hid to the HSA enumeration that dies on an uncovered arch. Keep the mask, matching the embedding CPU launch. * Studio: match HIP's other kernel-image error in the arch-crash retry (#7624) The reactive retry keyed on hipErrorInvalidImage ("device kernel image is invalid") alone, which is the wording both #7624 and #7669 field logs happened to show. hipErrorNoBinaryForGpu is the same arch mismatch and is the code whose documented cause is code compiled for a different GPU architecture, so a build that raises it fell through to the fit and flash-attn retries and landed back on the misleading GGUF/memory error. Match both wordings, case-folded: hipGetErrorString is lowercase but the layers that reprint it are not consistent about it. Three tests, including one that drives the whole recovery end to end on the new wording. * Studio: pin the two arch-gate shapes an existing AMD user would notice (#7624) Regression tests only, for the two "did this break my working setup" questions the gate raises and nothing pinned yet. HSA_OVERRIDE_GFX_VERSION is the long standing workaround for an arch the ROCm stack does not build for: the user sets it, ROCr reports the spoofed arch, and code compiled for that arch really does run on the card. The gate reads the arch through the same device properties HIP acts on, so it follows the override and the covered-by-spoofing card survives. Worth pinning because the raw silicon is uncovered and only the presented arch says otherwise. An install written before this PR has no mapped_targets in its marker, and the field is deliberately outside the install fingerprint, so it is never refreshed for this reason alone. Such a host must behave exactly as it did before: the probe keeps every device and the launch is not masked onto the CPU. #7669's mis-pick stays there until the install is refreshed, which is the intended trade, and the test says so rather than leaving it implied. Also three more crash strings on top of the ones the previous commit added: the same error raised during ggml's backend init, which prints through a different format string, and the CUDA spelling (cudaErrorNoKernelImageForDevice) since the retry is not ROCm-gated. And a correction to the comment on the "--alias -ts" case in test_a_command_without_a_split_reports_nothing_to_do: the scan is positional, so a VALUE spelled like the flag IS removed. Nothing Studio builds can reach it, so the limitation is now documented where it lives rather than described as behaviour that does not happen. * Studio: restore the Model Memory record for the arch-crash respawn (#7624) * Studio: backfill the bundle's built-arch list when an install is reused (#7624) write_prebuilt_metadata only runs on a real install, and mapped_targets is deliberately outside the install fingerprint so recording it never forces an existing install to reinstall. The two together meant the runtime arch gate read whatever the marker happened to hold, indefinitely. That is worst for the users this PR is for. An install made before the field existed never gains it, so on an already-up-to-date host the gate keeps failing open and #7624 / #7669 stay broken until an unrelated llama.cpp release happens to trigger a real reinstall. A manifest that corrects mapped_targets for an unchanged asset never lands either, and stale coverage is worse than none: too narrow forces a supported GPU to CPU, too wide leaves an unsupported one visible to crash. sync_marker_arch_coverage follows the ggml_tree backfill exactly, for exactly that reason, and is called from all three reuse paths beside the rocm_gfx sync, which has the same reach. A bundle that declares no targets (CUDA, Vulkan, CPU, source) leaves the marker alone rather than clearing it: reuse requires the fingerprint to match, so the asset is the one the marker already describes. It never raises, since the reuse path runs after the install is already valid and an exception there no longer falls back to a source build. Five tests, each confirmed by reverting the behaviour it claims. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: finish the forced-CPU launch's own bookkeeping (#7624) Two more places the arch gate's CPU-masked launch had to reach. holds_no_vram asks for manual mode with gpu_layers 0, so an arch-gated launch answered False even though it is the stronger case: the gate found the installed build has no kernels for any card here, the child was masked with the "-1" sentinel and cannot see a device at all. It arrives through an automatic request, which is exactly what the caller at routes/inference.py:8206 already documents as "recovery may turn an automatic GPU request into a zero-VRAM load". Left as it was, a server holding no VRAM kept the CHAT claim, blocked an image or video pipeline from coexisting, and could be unloaded mid-load by an owner it never competed with. The flag is per-load: set where the launch records its offload state, cleared on commit and on unload. llama.cpp reads LLAMA_ARG_DEVICE and LLAMA_ARG_MAIN_GPU as the env spelling of --device and --main-gpu (common/arg.cpp set_env), and neither the chat forced-CPU launch nor the embedding CPU launch passes those flags, so a value exported into the parent environment reaches the child. Hiding every device while leaving that pick in place is the one combination llama.cpp cannot serve: parse_device_list rejects a name that no longer enumerates, so the child exits instead of running on the CPU we just chose for it. Cleared with the "-1" sentinel and on the embedding CPU arm. This file already treats an inherited LLAMA_ARG_SPLIT_MODE and LLAMA_ARG_FIT as live input, so it is the same rule rather than a new one. Nine tests, each confirmed by reverting the behaviour it claims. * Studio: keep the gate's narrowed split load deduplicating (#7624) When manual mode supplies a per-GPU ratio across every visible card and the arch gate drops one of them, the launch strips --tensor-split from the argv and leaves self._tensor_split None. The request that asked for the ratio is unchanged, and the UI re-sends it verbatim on every Apply, so _runtime_matches_intent compared the live None against the requested tuple and read it as a different load every time. The result was a teardown and relaunch of the same already-normalized server on each identical Apply, forever, on the one host shape the gate exists for. Record the ratio the gate dropped and accept it in the comparator as this request normalized. The record is guarded on an actual drop rather than on None == None, so a launch that dropped nothing still cannot excuse a live split against a request that asks for none, and any other ratio still reloads. It is cleared unconditionally at the top of each load, so a drop earned by one launch cannot excuse a mismatch on the next. Seven tests, each confirmed by reverting the behaviour it claims. * Studio: stop the gate's state and env leaking past the launch that set them (#7624) Three misses, all in the arch gate's own new paths. The forced-CPU flag was only ever assigned True. load_model phase 1 kills the old process without running the unload reset, so the value outlived its launch: a host that gains coverage (a llama.cpp update, or just the next model) reported a VRAM-holding server as holding none, and the GPU arbiter left it unclaimed beside a competing workload. That is worse than before the flag existed. It is now published on every load, and cleared in the diffusion state block beside the sibling chat fields it already resets for the same reason. The reset I had put in _apply_cpu_fallback_state is gone with it: that runs only on the Vulkan CPU-fallback path, which is why it never covered this. Stripping --split-mode / --tensor-split from argv cannot remove LLAMA_ARG_SPLIT_MODE / LLAMA_ARG_TENSOR_SPLIT, and the existing tensor-to-layer reconciliation only clears the pair when an inherited mode is present and non-layer. So an inherited ratio survived a narrowing that re-indexes the survivors under it, and an inherited tensor mode walked straight back into the abort the forced-CPU strip exists to prevent ("LLAMA_SPLIT_MODE_TENSOR needs >= 1 devices"). Cleared at all three gate branches, and not gated on the argv strip having found anything, since the env twin is the hazard on its own. Residency is a property of the devices, and the arch-crash retry changes them. The canonical shape crashes on the discrete card and lands on the unified-memory APU, where the weights are host-backed after all, so the page-lock the first launch correctly skipped is the one the user asked for. The respawn ran unlocked and recorded _memory_mlock_applicable False, which reads the missing lock as deliberate and dedupes away the reload that would apply it. Recomputed through the same helpers as the first launch so the two cannot drift. Also collapses the device-placement env clear onto the existing _clear_device_placement_env instead of the near-duplicate helper I added last round. Eleven tests, each confirmed by reverting the behaviour it claims. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make the unmappable-mask test independent of torch being installed (#7624) Cross-platform staging CI failed this test on all three runners, and the cause was the test, not the code: it patches _torch_is_rocm but not the `import torch` that guards it. _active_gpu_visibility_mask reads the ROCr mask only inside that try, so on a runner whose dependency set omits torch the except arm sets is_rocm False, the mask is read from CUDA_VISIBLE_DEVICES instead, an unmappable ROCr value reads back as "no mask", and the gate pins the ordinals the test asserts it must not. So the assertion was exercising the CUDA branch and reporting it as the ROCm one. A fake torch module and an explicit CUDA_VISIBLE_DEVICES clear make it deterministic. Verified both ways: with torch importable, and under a meta_path finder that makes `import torch` raise, which is what the runners do and what reproduces the CI failure against the previous commit. * Studio: keep the arch-crash retry's memory recompute additive (#7624) The recompute swept every --mlock / --no-mmap / --load-mode off argv before re-appending its own. That dropped hand-written user memory flags the policy deliberately preserves, and since --mlock and --no-mmap take no value in llama.cpp's parser, the value-consuming scan also ate the argv entry that followed them. Restrict it to the lock-adding direction, which is the only one where argv carries no policy-emitted lock to take back off, and append the flag so llama.cpp's last-wins parse gives it the final say. The reverse direction leaves the crashed launch's lock alone; the child really does hold it, so the record stays truthful. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten the comments added by this PR (#7624) Comments and docstrings only; no code changes. * Studio: record the gate's tensor normalization and recheck placement-dependent state (#7624) Four review items, all the same shape as ones already fixed here. Record the dropped split MODE alongside the dropped ratio, and record the ratio on the reactive retry and the forced-CPU arm as the proactive gate already does. Without it the duplicate-load check reads a normalized server as a different one and tears down and reloads a multi-GB model on every Apply. Treat a forced-CPU launch as host-resident for Model Memory: the child runs entirely from host RAM, but _weights_in_host_memory answered for the original placement, so the page-lock was skipped and recorded as deliberate. Require real arch evidence before forcing CPU. _get_gpu_memory turns any probe error into an empty list, so "gated empty, ungated not" also describes a one-shot failure of the first probe on a host the gate never filters; the branch now re-derives the filter's own verdict instead. * Studio: cover the tensor-mode record and the forced-CPU rechecks with tests (#7624) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten the comments added by the last review round (#7624) Comments and docstrings only; no code changes. * Studio: stop the old-install probe test depending on the host's free RAM (#7624) test_the_probe_keeps_every_device asserted an exact APU figure of 11152MiB, which is the shared pool minus the host reserve. The pool is capped by the machine's real free system RAM unless _available_system_memory_mib is stubbed, so the expectation silently assumed the runner had more than 12176MiB free. It passed on the Linux, macOS and Windows runners and failed inside a WSL VM, which computed 7340->6316MiB. Stub the host RAM the way the sibling test in the same class already does. * Fix the arch-crash marker set and the survivor probe's torch import for PR #7670 Two follow-ups on the ROCm arch gate. _KERNEL_IMAGE_INVALID_MARKERS only matched hipErrorInvalidImage and hipErrorNoBinaryForGpu. clr raises hipErrorInvalidKernelFile and hipErrorInvalidDeviceFunction from the same fat-binary load in hip_fatbin.cpp, hip_platform.cpp propagates all of them out of the launch path, and ggml reprints hipGetErrorString verbatim, so a build that surfaces either of the missing two never armed the retry and was left on the misleading GGUF error. Both NVIDIA spellings match too, since the retry is not ROCm gated. _arch_gate_survivors called _host_torch_is_rocm before the coverage check, so a non-ROCm host paid a cold torch import (measured 1.35s and about 636MiB peak RSS) only to return []. Coverage is a cached marker read and a CUDA, Vulkan, CPU or macOS bundle records no mapped_targets, so checking it first answers [] without the import. Pure reordering, same result on every host. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten the comments added by the ROCm arch gate (#7624) Comment-only pass over the files this PR touches: the multi-line blocks and docstrings say the same things in fewer lines, with the reasoning that records a review finding kept intact. Preserved verbatim in shorter form: why _CONCRETE_GFX_ARCH fails open on non-concrete tokens and why its trailing hex is load-bearing (a digits-only pattern would reject gfx90a / gfx90c); why the crash markers cover both hipErrorInvalidImage and hipErrorNoBinaryForGpu and why the match is case-folded; why the -archfallback respawn restores the memory-policy snapshot; and why an unmappable visibility mask is left alone. Two comments were also corrected where they had drifted from the code: the marker block still said "two spellings" after the set grew to four, and the regex comment did not say that the trailing hex is what admits gfx90a / gfx90c. No code changed, verified with comment_tools.py check --strip-docstrings. * Studio: tighten the arch-gate test comments for PR #7670 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
oobabooga
|
5426a78c39
|
Studio: switch llama.cpp backends from the UI (#8520)
* Studio: add llama.cpp backend selector * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix llama.cpp backend selection edge cases * Fix llama.cpp backend selection review issues * Fix remaining llama.cpp backend review issues * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix llama.cpp backend switch consistency * Re-pair whisper after llama runtime changes * Re-pair whisper when llama runtime identity changes * Unify llama backend selection invariants * Preserve llama backend fallback constraints * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Publish llama planning job state * Handle llama frontend job transitions * Simplify the llama.cpp backend selection contracts * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address the open Codex findings on backend precedence and job ownership * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Drop the unused llama backend imports the hoist check rejects The Source lint job fails on studio/install_llama_prebuilt.py: scripts/verify_import_hoist.py reports INSTALL_KIND_BACKENDS and marker_backend as hoisted but unused. marker_backend is genuinely dead here, so it goes. INSTALL_KIND_BACKENDS is not: the installer is meant to share one vocabulary with the marker readers, and two tests assert that through this module. Give it a real use instead of a bare re-export by deriving VULKAN_INSTALL_KINDS from the map rather than spelling the same two names out again, which also removes a mirrored definition of the kind this file warns about elsewhere. * Treat an install marker that is not a JSON object as no marker read_install_marker returns whatever json.loads produced, so a marker holding [] or 123 reaches callers as something without .get. Every caller assumes a mapping, and the new backend picker adds one more: get_backend_status calls marker_backend(marker) and raises AttributeError, so GET /api/llama/backend answers 500 and Settings > System shows a load error for what is only a corrupt file. This is not new on this branch (get_update_status raises the same way on main), but the picker makes it reachable from a page users open. Guard it where the file is read, so the update planner, the picker and crash recovery all degrade to the source-build path together, exactly as they already do for unparseable JSON. * Disable Apply when the environment pins the backend The Select is disabled whenever env_backend is set, but Apply is gated only on dirtiness, and the two are computed independently. An automatic install whose detection has since drifted reports selection_applied false, so the row is dirty while the Select is disabled and Apply becomes the only live control. POST /api/llama/backend then refuses it with environment_override, which is correct, but the button should not have offered it. Also fills in the status shapes the payload tests did not reach: every unsupported reason, a macOS install reporting metal, and the terminal job states. * Run the setup.ps1 exit routing under pwsh instead of matching its text The Windows half of the fail-closed change was asserted by comparing source strings, which cannot catch a branch that reads the same and behaves differently. Extract the routing block the way the bash harness already does, run it under pwsh, and require the same decision from both: identical exit code, and identical answer on whether a source build was queued. Covers exit codes 0/1/2/3/4/5/137 against each explicit backend and both install states, 70 pairs, and pins the two branches the picker depends on: exit 5 fails closed everywhere, exit 2 stays the one automatic path allowed to fall back to a compile. Skipped where pwsh is absent, like the other PowerShell tests here. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Let a failed whisper re-pair be retried from the same selection The llama phase runs first and records the new backend, so a retryable whisper failure (a dropped download, an install that was busy) ends with llama.cpp on the requested backend and dictation still hardlinked to the old runtime. Retrying that selection is then already_selected, which skips the llama phase, and the whisper planner refused to run without one. The reported failure was unfixable except by switching away and back. Allow a repair-only job for that one refusal, gated on the pairing actually being stale so an ordinary already-selected request stays a refusal instead of becoming a no-op job that reports success. slim_pairing_is_stale exposes the comparison run_repair_phase already makes before it does any work. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
Daniel Han
|
8502cf84c2
|
Detect the Radeon AI PRO R9700 (gfx1201): it carries neither 9070 nor 9080, so name inference found nothing (#8573)
* Detect the Radeon AI PRO R9700 (gfx1201) in every GPU-name arch table The name-to-gfx tables matched RDNA 4 Navi 48 on 9070|9080 only. The workstation card is branded Radeon AI PRO R9700, which contains neither token, so the first-match-wins table returned nothing, no arch was inferred and the installer fell back to CPU torch. On a host with the HIP SDK present the gcnArchName probe answers first and the gap is invisible, which is why the R9700 reporters in #7624 and #7307 never hit it. On a plain Windows 11 box with a single R9700 and no SDK, name inference is the only path left and the card is simply reported as not detected. Add R9700 to all six copies of the table (install.sh twice, install.ps1, studio/setup.sh, studio/setup.ps1, studio/install_python_stack.py). The token is R9700 rather than a bare 9700 so the 2002 ATI Radeon 9700 PRO cannot pick up RDNA 4 wheels. * Tighten the R9700 detection comments * Run the AMD GPU-name arch table test in cross-platform CI --------- Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
|
Daniel Han
|
e468965b43
|
Pin the ROCm-on-WSL bootstrap to immutable refs (#8540)
* Pin the ROCm-on-WSL bootstrap to immutable refs
The WSL AMD bootstrap runs unattended and installs with sudo, so both pieces
of code it pulls in should be addressed by commit, not by a moving branch:
- install.sh fetched the helper from raw.githubusercontent .../unsloth/main,
so anything landing on main became root code on affected WSL hosts. It now
fetches a pinned commit SHA.
- the helper cloned ROCm/librocdxg at develop, then cmake/make/sudo make
install. It now builds tag v1.2.2 and verifies the clone resolves to that
tag's commit SHA before anything is built or installed.
v1.2.2 is what librocdxg develop points at today and the pinned helper commit
is the current main copy, so the installed result is unchanged. An explicit
UNSLOTH_LIBROCDXG_REF with no UNSLOTH_LIBROCDXG_SHA still builds whatever the
operator asked for.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Point the helper pin at current main and forward the librocdxg pin
Two follow-ups so WSL users keep getting the current helper and the pinned
third-party source applies immediately:
- _ROCM_WSL_HELPER_REF now points at current main (
|
||
|
Daniel Han
|
5714530f27
|
Route spoofed Strix Halo GPUs to the AMD per-gfx index (#8480)
* Route spoofed Strix Halo GPUs to the AMD per-gfx index (#7331) HSA_OVERRIDE_GFX_VERSION=11.0.0 is the widely circulated Strix Halo workaround, and ROCr applies it in userland, so rocminfo hands the spoofed gfx1100 to every consumer. The installer believed it: the correct gfx1151 inferred from the product name in /proc/cpuinfo was discarded, the Strix reroute intersected {gfx1151, gfx1150, gfx1152} against ["gfx1100"] and got nothing, and torch came from download.pytorch.org/whl/rocm6.3 as 2.9.1+rocm6.3. The first real allocation then ran gfx1100 kernels on gfx1151 silicon and segfaulted. The runtime-visible arch still outranks the product name everywhere it did before. The correction fires only when HSA_OVERRIDE_GFX_VERSION is set, the probe saw exactly one device, that device's arch differs from an inferred RDNA 3.5 APU arch, and a source the override cannot reach agrees with the product name: KFD topology sysfs first, since amdkfd writes gfx_target_version from the kernel's own table, then rocminfo re-run with the variable stripped, and only if neither can answer, the variable statically naming the arch that was reported. A mixed Strix APU plus discrete AMD GPU host stays on today's path. Two devices in the probe declines outright, and so does a kernel that sees a second GPU the spoofed probe had collapsed away, so the existing precedence test needed no change. install.sh carries the same three helpers and the same decision, and the tests execute both copies over the same eight host shapes rather than grepping for them. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Require corroboration before undoing an HSA_OVERRIDE_GFX_VERSION spoof Three problems with the correction as it stood. install.sh never fired. The Strix reroute passes `_gfx_all` in raw, and that is `rocminfo | grep -oE 'gfx[1-9][0-9a-z]{2,3}'` output, which repeats the token once per Name / ISA line. A single GPU therefore arrives as two or three lines, the device count read 2 or 3, and the helper returned early on every real host, #7331's own included. Count DISTINCT tokens instead. The Python side already splits on agent headers and was unaffected, so the two paths disagreed on exactly the host the fix exists for: `studio update` repaired the machine and a fresh `curl | sh` install re-broke it. The static fallback could fire when it should not. "The variable names the arch that was reported, so assume a spoof" is indistinguishable from a host telling the truth: a real RX 7900 XTX in a Ryzen AI Max chassis reports gfx1100, infers gfx1151 from the CPU product name (the inference reads /proc/cpuinfo and never looks at the GPU), and presents the identical fingerprint the moment its owner has the override set for an unrelated reason. Same for a correct HSA_OVERRIDE_GFX_VERSION=10.3.0 on an RDNA2 card. Both were rerouted to Strix wheels whenever KFD sysfs was unreadable. Drop the fallback: corroboration from the kernel or from an unspoofed re-probe is now required, and a re-probe that still answers the probed arch is read as evidence FOR the probe rather than as a failure to disprove. Rerouting a working machine to the wrong wheels is worse than the bug being fixed. The Python re-probe left the visible-device masks in place while install.sh cleared them, so a mask pinned to the dGPU hid the second GPU whose presence is the only thing that vetoes the correction on a mixed host. Clear all three in both, and add the arch the variable names as a required precondition rather than a sufficient one: ROCr can only rename an agent to the target the variable names, so any other reading is real silicon. Tests: the parity check now builds each side's probe input the way its own call site does instead of handing both a pre-shaped list, which is what let the install.sh defect through. Over the resulting matrix the previous code diverges on 91 of 3360 shapes with install.sh correcting none of them; it is clean now. Adds the 7900-XTX-in-a-Ryzen-AI-Max cases, executes the shell KFD parser against a fabricated topology tree rather than grepping it, and passes create = True where a helper is patched so a run against an older tree fails on the assertion instead of on AttributeError. * Say why the spoof check declined, and count arches without wc The correction printed "Checking whether the ISA is being spoofed." and then, on every path that decides there is no spoof, said nothing further. That is the CORRECT outcome for a real gfx1100 card in a Ryzen AI Max chassis, and it is the outcome a user is most likely to see, so silence reads as a failure. Report the conclusion in both implementations, naming the source that declined to corroborate and the arch being kept. Also count distinct probe tokens inside the existing awk instead of piping to `wc -l | tr -d`. wc was the one tool the change added to install.sh's dependency set (0 pre-existing uses); this drops it along with two process spawns per call. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Clear a corroborated HSA_OVERRIDE_GFX_VERSION spoof before Studio launches * Clear a contradicting HSA_OVERRIDE_GFX_VERSION at every Studio launch install.sh unsets the corroborated spoof for the one launch it performs itself, but that unset dies with the installer: studio/setup.sh runs install_python_stack.py as a child, so the `unsloth studio update` repair, the generated launch-studio.sh and a hand-typed `unsloth studio` all still start with the variable set. Clear it at the CLI chokepoint the exec, the Windows Popen and the in-process paths all pass through, keyed on the install rather than on a hardware probe: AMD's per-gfx index ships rocm_sdk_libraries_<arch> beside single-arch wheels, so an override naming a different arch is provably asking for kernels this install does not contain. A generic multi-arch index brings no such distribution and is left alone, since there the override is often the only thing making the GPU usable. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Read the active ROCm family, clear the spoof on every launch entry point * Pin the amd-smi re-probe parity between install.sh and the Python stack * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep the override when --no-torch installs no per-gfx wheels Clearing HSA_OVERRIDE_GFX_VERSION is only sound because native per-gfx wheels are going in on that branch. The guard checked only that the spoof had been corroborated, not that anything was actually being installed. --no-torch, and the Intel Mac auto-detection that sets the same SKIP_TORCH, reach the reroute and then install no torch at all. Clearing there left the host with the generic wheels it already had and no override, which is strictly worse than either alone: on a spoofed Strix host the override was the only source of usable kernels. The block's own comment already stated the invariant (native wheels are going in); it just did not enforce it. It does now. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Let torch's own requirements decide whether the rocm metadata is live The launch-time arbiter read the bare rocm meta-package's Requires-Dist to name the family the install carries kernels for. That is authoritative only while torch actually resolves through it. Switching from AMD's per-gfx index to a generic pytorch.org ROCm one does not uninstall it: the generic wheels vendor their own ROCm libraries and depend on no meta-package, so rocm is orphaned outright, and pip has no autoremove. The stale metadata then named the OLD family and cleared an override that the generic wheels may be the only reason the GPU works at all, which is the opposite of what this code is for. torch's Requires-Dist is now the first discriminator. An unknown shape answers no, so a tree this cannot read is left alone rather than arbitrated on a guess. The venv fixture grows the same dependency edge, since one that never carried it could not tell a live meta-package from an orphaned one. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Condense comments in the HSA_OVERRIDE_GFX_VERSION spoof detection --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
|
Daniel Han
|
e696d3328a
|
Windows installer: fix single-AMD-GPU detection and the unrecoverable "needs repair" loop (#8398)
* Windows installer: fix single-AMD-GPU detection and the "needs repair" loop Two defects that met on the same host and made the installer unable to converge on a single-Radeon Windows box (#8335). The WMI fallback picked between the healthy adapters and the full adapter list with a bare if-expression, which unrolls a one-element array to a scalar. A scalar's .Count is $null under Windows PowerShell 5.1, so a host with exactly one AMD GPU read as having none: no $script:ROCmGpuLabels, no inferred gfx arch, "gpu none" in the hardware report, and the installed +rocm venv judged stale against a required "cpu". Wrapped in @(), matching the Intel scan in the same file and in install.ps1. The stale-venv branch under $InstallerManagedSetup then aborted with "re-run install.ps1 so it can replace the environment safely with rollback". install.ps1 is the caller on that path and had already done exactly that earlier in the same run, and its failure path moves the previous environment straight back, so every attempt ended on the state it started from and the next attempt reached the same verdict. That abort has now been reported from four unrelated triggers. It is replaced by the in-place torch repair the index-pin and CUDA-family changes already use, which needs no delete: install.ps1 invokes setup through the venv's own unsloth.exe, so python.exe is locked by the process running the script. Also: - Test-VenvTorchIsRocm, the AMD counterpart of Test-VenvTorchIsXpu. A faulted Adrenalin or HIP runtime makes `import torch` raise at the DLL load or hang, and the venv then read as "torch could not be imported" and got deleted. version.py on disk still names the wheel, so trust it and point at the driver. - Invoke-BoundedPythonProbe keeps the stderr it used to drain and discard, in both installers, and the stale-venv message prints it. A dead driver, a half-written wheel and a missing torch were previously one sentence. tests/studio/test_amd_venv_repair_loop.ps1 covers the ROCm disk read, the probe error plumbing against a real child process in both installers, and the source shapes. pwsh 7 answers 1 to a scalar .Count, so it cannot reproduce the 5.1 half of #8335 and does not claim to; that leg is the windows-latest row of cross-platform-parity-ci. Closes #8335. * Windows installer: correct the 5.1 diagnosis, make the test reproduce it, and close the one path the repair loop fix made worse Validation follow-up to the previous commit. The stated root cause was over-general and, as written, wrong. "A scalar's .Count is $null under Windows PowerShell 5.1" is not true: a String or an Int32 answers 1 on 5.1, exactly as on 7, so anyone checking the claim that way concludes there is no bug. $null comes back only for objects whose PSObject carries no Count of its own -- [pscustomobject], which Microsoft documents, and CimInstance, which it does not. Get-CimInstance returns the second kind, which is what the WMI fallback assigns. Measured on windows-latest, PowerShell 5.1.26100.33158 against a real CIM instance, with pwsh 7.6.4 on the same runner: 'a' 5.1: 1 7: 1 [pscustomobject]@{...} 5.1: $null 7: 1 CimInstance (one instance) 5.1: $null 7: 1 @(if (...) { ... }) 5.1: 1 7: 1 The conclusion and the fix are unchanged; the reason is now the right one. That also means the test could always have been real, rather than a source shape apologising for pwsh. Win32_OperatingSystem returns exactly one instance on any Windows host, which reproduces "the machine has exactly one AMD GPU" with no AMD GPU present. The suite now adapts to its host: under 5.1 it reproduces #8335 and regression-tests the @() wrap for real, under pwsh the unroll still runs and the consequence is reported as a shape. 93 checks, green on both, verified on a windows-latest runner under powershell.exe 5.1.26100.33158 and pwsh 7.6.4. The claim that the 5.1 leg is covered by cross-platform-parity-ci was wrong: every step in that workflow is shell: pwsh, which is PowerShell 7. A shell: powershell step needs adding beside the pwsh one -- 5.1 is the runtime the CLI actually launches setup with (unsloth_cli/commands/studio.py). Left out of this commit only because the push token has no workflow scope; the exact step is on the PR. The in-place repair had one path it made worse than the abort it replaced. A venv directory with no Scripts\python.exe is incomplete, not stale, and has no interpreter to force-reinstall torch through. The abort used to catch it by catching every stale verdict. Without it the run reaches the activation -- a dot-source, which is NON-terminating at the "Continue" the pip section runs at -- so setup carried on with the venv unactivated, resolved every later python and uv pip against PATH, installed the whole stack outside the environment, and could still exit 0. Narrow to reach, since install.ps1 launches setup through that venv's own unsloth.exe, but silent when reached. Now refused, before the activation, without deleting anything. Also: - The surfaced probe error no longer indexes [0] into a pipeline that a whitespace-only stderr empties; fatal under a caller's Set-StrictMode, and studio/setup.bat does not pass -NoProfile. Confirmed to throw on 5.1 too. - $script:PinChangedForceReinstall is hoisted beside $installedTorchTag. Four install arms read it to decide --force-reinstall and a fresh install never reaches the assignment. - Test-VenvTorchIsRocm's comment claimed AMD's Windows wheels are labelled +gfx1151. They are not. repo.amd.com/rocm/whl/<arch>/torch/ publishes torch-2.11.0+rocm7.13.0-cp312-cp312-win_amd64.whl and keeps the arch in the URL only -- the same filename is a different binary under gfx1151, gfx110X-all and gfx120X-all. download.pytorch.org publishes the two-component +rocm6.4, Linux only. +rocm7.13.0 is what the #8335 reporter ended up on, and is now a case. The +gfx arm stays as deliberate defence. * Run the AMD repair loop tests under Windows PowerShell 5.1 as well Every step in cross-platform-parity-ci is shell: pwsh, so the whole job runs under PowerShell 7. 7 returns .Count = 1 for the single CimInstance that 5.1 returns $null for, which is the entire mechanism of #8335, so the existing pwsh step passes just as happily against the unfixed code. The suite had no leg on the shell the CLI actually launches setup.ps1 with. Adds the same test file under shell: powershell on the Windows row only. * Windows installer: keep the installer's GPU wheel through an in-place repair, refuse a venv with no activation script, and bound the AMD fast-path torch probe * Windows installer: keep the installer's GPU wheel through a GPU-to-GPU rescan, not just a CPU one * Windows installer: verify the venv activation actually took effect, not just that Activate.ps1 exists A present Activate.ps1 is not an activated venv. The script prepends the venv to PATH in its last statement, so a copy truncated by an interrupted or out-of-disk python -m venv runs to its last complete statement and returns without raising anything at all, and an unparseable one is a ParserError, which is non-terminating at the Continue the pip section runs at. Either way the dot-source succeeds with the ambient interpreter still first on PATH, which on a real install is the system or Microsoft Store python because install.ps1 keeps the venv Scripts directory off PATH on purpose. Fast-Install hands exactly that to uv pip install --python, every Exit-SetupFailure after it keys off an exit code from the wrong interpreter, and setup exits 0, at which point install.ps1 commits over its rollback copy. Assert the post-condition instead of adding a third existence check: the python now in effect must live under $VenvDir. VIRTUAL_ENV would not do, since Activate.ps1 sets it before the line that matters. Both sides are normalised through Get-Item, the same call Activate.ps1 uses to build the PATH entry, so a short 8.3 path, a substituted drive, a junction or a differently cased drive letter cannot read as outside, and every branch that cannot prove the interpreter is wrong returns rather than refusing a working install. * Windows installer: only preserve the GPU wheel install.ps1 actually selected The guard that keeps an installer-placed GPU wheel through a rescan treated any non-cpu wheel under UNSLOTH_INSTALL_ROLLBACK_MANAGED as the installer's own choice. That is not true on an upgrade: install.ps1's migrated-venv arm installs unsloth alone and never touches torch, and its flavor repair no-ops whenever its own expected tag is cpu or unrecognised, so a legacy ~/.unsloth/studio/.venv can hand setup a +cu118 wheel left by a previous install on different hardware. Preserving that pinned the whole dependency pass onto a cu118 index, and on a mapped AMD host the kept cu* tag also blocked the ROCm reroute, which needs $CuTag -eq "cpu" -- so the Radeon never got a ROCm wheel and setup still exited 0. install.ps1 now reports the family it settled on in UNSLOTH_INSTALLER_TORCH_TAG and setup preserves only a wheel that matches it. Absent or blank means the installer did not say (an older cached install.ps1, or --no-torch) and falls back to preserving, so an older installer keeps exactly its previous behaviour. A wheel the installer did not choose repairs in place, as it did before the guard existed; nothing aborts. * Windows installer: tighten the comments added across the AMD repair-loop rounds Four review rounds each re-explained the same mechanisms, so the exit-0 hazard, the @() unroll and the preserve guard were each written out in several places. Keep the full reasoning at one site per mechanism and cut the restatements. Comments and whitespace only; no executable change. * Scope ErrorActionPreference around the corrupt-activation child process The Windows PowerShell 5.1 leg of the parity workflow failed at the corrupt activation script case. Every assertion passed, then the run ended with a NativeCommandError. 5.1 wraps a native command's stderr in a NativeCommandError and applies $ErrorActionPreference to it, so the parse error the corrupt script writes to stderr became terminating under the Stop set at the top of the file. PowerShell 7.1 stopped applying the preference to native stderr, which is why the same script passes under pwsh and failed only on 5.1. Set the preference to Continue for the duration of the child invocation. The exit code is what the case actually judges and it is still read and asserted. --------- Co-authored-by: danielhanchen <unslothai@gmail.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> |
||
|
Daniel Han
|
26a91bb65d
|
Studio: pin the remaining unpinned requirements (#8408)
* Studio: pin the remaining unpinned requirements Every entry in studio/backend/requirements that had no version specifier now carries an exact pin, resolved per interpreter where the newest release dropped an older Python. * Align trailing comments to the existing column, note the certifi trade-off * Fix review findings on the requirement pins - test_torchao_select asserted the verbatim pre-split einx line, so the einx split broke it. Assertion updated to both new lines. - pytorch_tokenizers is capped rather than pinned: 1.4.1 has no musllinux wheel and its arm64 wheel is macosx_14_0, so an exact pin pushed musl hosts and macOS 13 onto the one release with an sdist and a cmake build. - sentence-transformers in no-torch-runtime.txt now matches the 5.2.0 that extras-no-deps.txt installs later; the higher pin could never take effect. - Corrected the MeCab and numpy comments, and widened the certifi note to the rest of the TLS and HTTP request path. * Mirror the pins into the studio extra, and bump lxml off PYSEC-2026-87 The `studio` extra claims to mirror studio.txt but kept bare requirements, so `pip install "unsloth[studio]"` resolved pandas 3.x where the managed installer resolves 2.3.3. The existing sync test compared normalised names only, which is why the drift was invisible; it now also compares specifiers and markers, and fails if either side moves alone. lxml was pinned at 6.0.2, which pip-audit reports as PYSEC-2026-87 (fixed in 6.1.0). Pinning is what surfaced it: a bare entry always resolved to whatever was current, so the nightly audit never had a version to flag. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
510c69d35a
|
Studio: pair slim whisper with ROCm targets hipBLASLt has no kernels for (#8364) (#8400)
* Studio: pair slim whisper with ROCm targets hipBLASLt has no kernels for (#8364) * Pin the on-disk catalog check the marker relaxation must not touch Membership plus required replaced set equality on the marker's linked_runtime_directories, but the per-directory "exists and holds a file" check on disk is what makes a genuinely broken runtime fail closed. Nothing covered an install whose wired catalog went empty, so a future refactor could drop that loop and read a broken install as current, which is exactly the failure the guard exists to prevent. Adds the empty-catalog-on-disk case on both sides (existing_install_matches and the sidecar launch guard) for hipblaslt and rocblas alike, plus the missing runtime_wiring_version case: the version floor is a positive test, not a default. * Tighten the ROCm catalog pairing comments --------- Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> Co-authored-by: danielhanchen <unslothai@gmail.com> |
||
|
Daniel Han
|
3fc67dcad6
|
Linux: resolve the ROCm version from every source, not the first that answers (#8414)
* Linux: resolve ROCm version from every source, not the first that answers install.sh took the ROCm version from the first source that responded, in the order amd-smi, /opt/rocm/.info/version, hipconfig, dpkg rocm-core, rpm rocm-core. Debian 13 (and Linux Mint on top of it) packages hipconfig at 5.7.31921-0 next to a 6.1.x rocminfo/HSA runtime, so detection settled on rocm5.7, the "PyTorch ROCm wheels require ROCm 6.0+" gate fired, and a working RX 7900 XTX got torch 2.10.0+cpu. The same install log then reported "AMD ROCm (gfx1100) ROCm: /opt/rocm", so the installer held evidence against its own conclusion. Detection now reads every source and takes the highest reading. A source reporting lower than another on the same host is stale packaging, not a downgrade, so the highest is the one that describes the runtime the GPU uses. The inline || chain is split into one helper per source plus a highest-wins resolver; each helper returns 0 unconditionally, so the case where every source is missing still reaches the actionable no-version warning instead of dying under set -e. The sub-6.0 warning also names a way forward. It now says the reported version was the highest any source gave, and names UNSLOTH_TORCH_INDEX_FAMILY and UNSLOTH_TORCH_INDEX_URL, both of which return from get_torch_index_url before any GPU probing and are forwarded through the WSL reroute, so they are a real override for a host with split ROCm packaging. Tests: tests/sh/test_rocm_version_source_disagreement.sh covers the reported Debian 13 shape, a stale reading in each source position, a genuine ROCm 5.x host still falling back to CPU, every source missing not killing the installer under set -e, and the unchanged tag normalisation (patch levels, 6.5+ clip, 7.3+ cap). test_rocm_support.py's set -e guard is rewritten against the new shape and gains a per-position highest-wins check. * Linux: stop a removed-not-purged rocm-core from choosing the ROCm wheels Resolving the ROCm version by highest reading fixed the undershoot in #8402 and opened the symmetric question: a source reading HIGHER than the host now picks the wheel family instead of being shadowed by its position in the chain. Most of that is fine and was already bounded. PyTorch's ROCm wheels vendor their own ROCm userspace into torch/lib, so what they need from the host is the amdgpu/KFD driver, and AMD documents driver/userspace compatibility as +/- 2 releases, widening to about a year's span from 6.4 on. The normalisation below can only emit an index leaf PyTorch actually publishes. A host that reads high because it genuinely carries a newer ROCm tree is therefore usually right, or at worst harmless. What is not bounded is a reading taken off something that is not installed at all, and dpkg could produce one. `dpkg-query -W` lists every package in the status database except purged ones, so a rocm-core taken out with `apt remove` and never purged sits in state "deinstall ok config-files" and still reports the version it had. A host that ran ROCm 7.0, went back to 6.1 and never purged therefore offered a 7.0 reading off a package that is gone. Fourth in the old first-answer chain it was usually shadowed; under highest-wins it beat every live source. Detection now asks for ${Status} alongside ${Version} and takes the version only when the status word is "installed". ${Status} is a documented showformat field with no dpkg version floor, and dpkg-query renders an unrecognised field as empty rather than failing, so on a dpkg that lacked it this source would go silent rather than over-report. The other four were audited for the same class of state and none has one. rpm has no equivalent of config-files: `rpm -e` drops the header record, and `rpm -q` then exits 1, which the helper already treats as no answer. amd-smi resolves librocm-core.so and hipconfig reads its own tree's version file, so both report a ROCm that is installed. /opt/rocm/.info/version describes whatever tree /opt/rocm resolves to. What those three cannot do, on a host carrying several ROCm trees, is say which runtime a process will actually load; none of them knows, and neither does this installer. So disagreement is now on the record. When the sources do not agree, resolution names every reading and which one won. That is the cheap part of the answer to the multi-version case: when a too-high pick does bite it surfaces as an HSA error at the first HIP call, or as a GPU silently missing from torch, and neither points back at wheel selection without the readings in the install log. Agreeing sources stay silent. The call site stops discarding stderr, which only ever carried this line: each source's own noise is suppressed inside the resolver. Tests: the shell suite gains the config-files 7.0 entry on a 6.1 host, its installed twin at the same versions so the assertion cannot pass on the version alone, the other non-installed dpkg states, and a stale-high reading in each of the five source positions. Its dpkg-query mock now renders the showformat string it is handed, so the tests cover how install.sh asks for the version and not only how it parses the answer; test_rocm_support.py gains the same mock and the config-files/installed pair. Both new cases fail against the previous commit and pass here. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stop amd-smi reporting the amdgpu driver version as the ROCm version amd-smi prints one pipe-delimited line, so the ROCm field is followed on the same line by the amdgpu driver version. Stripping every non-digit from the field glued the two together, and when amd-smi cannot detect a ROCm userspace it prints "ROCm version: N/A", which left only the driver version behind: ... | ROCm version: N/A | amdgpu version: 6.10.10 | ... -> rocm6.10 While detection took the first source that answered this was mostly hidden, since a later real reading could not be reached anyway. Under highest-wins a fabricated reading competes, and this one wins: on a host with a genuine ROCm 6.1 the N/A line resolved rocm6.4 instead of rocm6.1. The field is now cut at the separator and required to start with digits, so an N/A or empty field contributes nothing instead of a wrong number. * Extract the ROCm version helpers in the no-version endpoint test The test builds a shell script out of get_torch_index_url plus the two gfx helpers, but the version chain now lives in its own helpers, so the call to _detect_rocm_version_tag hit a command that does not exist. The guarded assignment swallowed the 127 and the no-version endpoint was reached for the wrong reason, which made the test pass whatever the resolver did. Mutation confirms it: returning a real tag from _detect_rocm_version_tag left the test green before this change and fails it after. The /opt/rocm/.info/version rewrite moves onto the helper text for the same reason. The read moved out of get_torch_index_url, so rewriting the function body alone had quietly become a no-op. * Bound the rpm probe highest-wins made unconditional Resolving the ROCm version from every source instead of the first that answers moved `rpm -q rocm-core` from last in a first-answer-wins chain to a probe that runs on every AMD host. On RHEL/SLES with a normal ROCm install /opt/rocm/.info/version answered at position two, so rpm was never invoked; now it always is. rpm -q is not a lock-free read. On the BerkeleyDB backend (rpm < 4.16, so RHEL 8 and SLES 15, both supported ROCm platforms) a leftover /var/lib/rpm/__db.00* from any killed rpm or yum leaves plain queries stuck in futex until the files are removed, and rpm 6.0.x reintroduced a read lock that deadlocks rpm --query against a running dnf transaction. Route it through the existing _run_bounded helper so a wedged rpmdb makes the source decline to answer instead of hanging the installer. _run_bounded no-ops where timeout is absent, so this adds no new dependency. The other four sources are left alone: amd-smi ran first in the old chain so it is not newly reachable, the version file is a plain read, hipconfig --version reads a version file and is not linked against HIP or HSA, and dpkg-query takes no dpkg lock at all. * Tighten the ROCm version resolution comments --------- Co-authored-by: danielhanchen <unslothai@gmail.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
Lee Jackson
|
86e02ebc13
|
Fix Windows Whisper slim pairing on GPU llama runtimes (ROCm, CUDA, Vulkan) (#8379)
* Fix Windows ROCm Whisper slim pairing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Extend the Windows libomp exemption to CUDA and Vulkan for PR #8379 * Tighten the Windows libomp exemption comments for PR #8379 * Scope the Windows libomp exemption to GPU bundles and require ggml-hip.dll for PR #8379 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Decide the Windows libomp exemption from the runtime files, not bundle_profile, for PR #8379 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the Windows libomp exemption comments for PR #8379 * Pin the Windows ROCm module glob with a decoy-only regression test for PR #8379 * Let the whisper prebuilt tests run on Windows for PR #8379 * Cover the cpu backend on a GPU llama runtime and note the arm64 libomp dependency for PR #8379 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
Daniel Han
|
3a58fa5c41
|
Studio: apply base.txt on the install.sh and install.ps1 paths (#8195)
* Studio: apply base.txt on the install.sh and install.ps1 paths
install.sh and install.ps1 install unsloth and unsloth-zoo inline, then
export SKIP_STUDIO_BASE=1 so setup.sh / setup.ps1 do not install the same
two packages a second time. install_python_stack.py read that flag as
"skip base.txt" and short-circuited the whole step:
if skip_base:
pass
That was the same thing only for as long as base.txt held nothing but
those two names. Add a third, pinned entry to base.txt and it reaches no
fresh install on any platform: neither installer reads the file, and the
one branch that does was skipped. It would only land later, if the user
happened to run `unsloth studio update`.
Every install.sh and install.ps1 path was affected, on every platform:
CUDA, ROCm, XPU, CPU, macOS, local and non-local, fresh and migrated.
Keep skipping the two core packages, which is all the flag was ever
meant to avoid repeating, and apply whatever else base.txt asks for.
When base.txt holds only the core packages, as it does today, there is
nothing left to install and no extra subprocess runs. No-torch mode is
untouched: it has its own list in no-torch-runtime.txt, which the
installers do apply inline.
The core-package filter parses the project name rather than matching on
a prefix, so a future unsloth-<something> pin is not swallowed too.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Reconcile base requirements with current main
* Preserve relative requirements includes across filters
* Fix filtered requirements test cleanup
* Separate core and shared base requirements
* Preserve shared base requirement resolution
* Keep the filtered-requirements and uv alias paths from aborting an install
The adjacent temp copy raised PermissionError on a read-only requirements dir, and a symlink failure handed uv back the spaced path it cannot read. Fall back to the temp dir and to a copy respectively, and stop the real-extras tests leaving filtered files in the tree.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Count the MLX slot and survive an unusable base.txt
Simulating every install path showed two gaps. base_total never counted the Apple Silicon MLX step, so `studio update` there ran 13 steps out of a declared 12 and recorded the wrong steps_total. And the new base.txt read happens before the manifest is dropped, so a missing or unreadable file aborted with a traceback where the old code reached pip; a BOM also read as content and scheduled an empty step. Progress coverage now spans both core paths on all four platforms.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Make the unreadable base.txt case independent of the mode bits
chmod(0o000) denies nothing as root, which containerized test jobs run as, and Windows does not implement POSIX modes at all, so the case asserted None against a file it could still read. Raise from a patched read instead.
* Tighten the comments this PR adds
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com>
|
||
|
Daniel Han
|
88c54835a7
|
CLI: stop verify-install describing an unrelated venv around an editable checkout (#8308)
`unsloth studio verify-install` resolves which prefix owns
studio/install_manifest.py by walking up from the module file to the first
pyvenv.cfg. After `./install.sh --local` that module lives in the repo, so the
first pyvenv.cfg above it is whatever venv the clone happens to sit inside,
which is frequently unrelated to the managed install.
It then walked that venv's site-packages and reported every managed dependency
as missing, so a healthy install printed
Unsloth Studio install is incomplete (studio_install_incomplete).
missing packages: matplotlib, nest_asyncio, datasets, huggingface-hub, ...
repair with: unsloth studio update
and exited 1, even though all of those import fine in the managed venv.
setup.sh / setup.ps1 gate their "already up to date" fast path on that exit
code, so every install re-ran the full dependency pass, and the desktop
preflight reads the same reason string.
A prefix now only owns the module when the module actually lives in that
prefix's site-packages. An editable checkout resolves to no owner, which falls
back to sys.prefix: the venv the CLI is running in, which is the managed one.
|
||
|
oobabooga
|
495ba21ebd
|
Studio: add MiniMax H3 video generation (#7989)
* Add MiniMax H3 video generation * Improve MiniMax H3 memory routing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address MiniMax H3 review feedback * Keep H3's VAE off the CPU path, so low_vram stops aborting low_vram maps to the `model` policy, and offload_flags emits --vae-on-cpu for it unconditionally. On H3 that kills the process: ggml/src/ggml-cpu/ops.cpp:6321: GGML_ASSERT(src0->type == GGML_TYPE_F16) failed deterministically, SIGABRT, exit 134. Bisected on the flags: --vae-on-cpu with --audio-vae aborts, the same command without --audio-vae renders in 95.87s, and an fp16-converted audio VAE aborts too. So the trigger is the audio VAE, not the video one, and the F32 type is imposed inside stable-diffusion.cpp rather than by the file: ggml_conv_1d hardcodes an F16 im2col destination (ggml/src/ggml.c), ggml_compute_forward_im2col_f16 then asserts the KERNEL is F16, and audio_conv_weight_type (src/model/vae/ltx_audio_vae.hpp) maps only BF16 to F16 and lets F32 through. It cannot be fixed by shipping a different checkpoint. low_vram is the one mode a small-card user reaches for, so this drops the flag rather than the mode. offload_flags takes vae_on_cpu, defaulting True so no other family changes, and the H3 native path passes False. --offload-to-cpu and --clip-on-cpu still apply, which is where the saving actually is: the denoiser dominates, and with --offload-to-cpu the whole model peaks at 13.14 GiB. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Point the H3 GGUF pick at the unsloth mirror Main added test_curated_gguf_repos_are_unsloth_mirrors, which requires every curated video gguf_repo to live under unsloth/ so a one-click pick cannot 404 when a community repack is renamed or taken down. H3 was the one family still on a community repo. This was meant to be part of the merge commit but was left in the worktree, so CI on that commit still saw the old value. unsloth/MiniMax-H3-GGUF is still private and has to be made public before this merges, or the pick will 401. No CI check reads it. * Pin H3's native cfg-scale under test H3 is distilled and CFG-free: its empty unconditional prompt encodes to zero tokens, and the transposed tensor that produces trips GGML_ASSERT(!ggml_is_transposed(a)) in ggml.c. SIGABRT, exit 134. Measured: cfg 1.0 renders, cfg 1.5 and cfg 4.0 both abort. sd.cpp defaults cfg-scale to 7.0, so this is a crash a plausible refactor reintroduces by forwarding guidance the way every other family does. The native path already hardcodes 1.0 and the family sets supports_cfg = False, but nothing held either in place. supports_cfg only gates the diffusers path; the native path builds its own params. Checked the test fails when cfg_scale is changed to forward guidance, so it is not passing vacuously. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Pin diffusers by source archive instead of git, so macOS installs work The macos-15-intel leg fails deterministically, not flakily: process didn't exit successfully: `/usr/bin/git init` (exit status: 1) --- stderr xcode-select: note: No developer tools were found, requesting install. That runner has no Xcode, so /usr/bin/git is the developer-tools shim and exits 1 for everything. uv needs a working git to resolve a git+https dependency, so the diffusers pin this branch added cannot install there at all. Main is unaffected because it depends on plain diffusers. GitHub serves the same commit as a source archive, which uv installs with no git involved. Verified by stubbing git to fail exactly the way the macOS shim does: the git+ form reproduces the CI error, the archive form installs diffusers 0.40.0.dev0 from the same SHA with MiniMaxH3Transformer3DModel present and exported. Also drops a full clone of diffusers from every install. * Allow the diffusers source build on the clean-machine legs With the archive pin the macOS leg gets past `git init` and installs, but then trips the nobuild guard: built from source: diffusers -- these must resolve to wheels on a clean machine There is no wheel to resolve to. MiniMax-H3 support is not in any diffusers release, so this branch has to pin a commit, and neither a git URL nor a source archive can produce a wheel from an index. Added to the same allowlist that already carries triton-kernels for the same reason. Checked against the bar the comment there sets: diffusers builds with plain setuptools, declares no ext_modules, and its tree has zero .c/.cpp/.pyx/.rs/.cu files and no shipped binaries, so the PEP 517 build is a pure-Python copy step and needs no toolchain. The separate compiler-invocation check in both scripts is untouched and still fires if one is ever needed. Verified the allowlist logic still rejects a non-allowlisted source build (a log building both diffusers and numpy reports only numpy). Remove this entry once a diffusers release carries H3 and the requirement goes back to a version specifier. Noted in both scripts. * Baseline the hf-hub retry loop reopened by the 1.x upgrade `pip scan-packages :: hf-stack` fails on this branch with 1 unbaselined CRITICAL: C2 polling/beaconing loop detected Package: huggingface-hub File: huggingface_hub/utils/_http.py L461: while True: This is on us, not upstream drift. Main pins huggingface-hub==0.36.2; this branch needs >=1.23.0,<2.0 because diffusers at the pinned commit requires it, so the resolved version moves 0.36.2 -> 1.27.0. The baseline already carries this exact file and check at L462 and L298, from earlier versions. It did not carry over because the key hashes the matched code, not the line number, and the surrounding code changed across the major version. That is the guard behaving correctly: changed code in a baselined file reopens for review rather than staying suppressed. Reviewed it rather than just re-suppressing. L461 is the retry loop in `http_backoff`: bounded by `nb_tries > max_retries`, the URL comes from the caller, there is no hardcoded endpoint and nothing is exfiltrated. Same benign construct as the entries it replaces. Entry generated with the scanner's own _evidence_hash rather than hand-written, and inserted beside its siblings so the diff stays 8 lines. Verified: with the baseline the scan is exit 0 with 4 suppressed, without it exit 1, so the guard still bites. * Lift the macOS-arm huggingface-hub cap that this branch made unsatisfiable `mac macos-15 / trace / file` fails on this branch and passes on main. uv reports: No solution found when resolving dependencies: Because you require huggingface-hub>=0.34.0,<1.0 and huggingface-hub>=1.23.0,<2.0, your requirements are unsatisfiable. This branch moved base.txt, no-torch-runtime.txt, studio.txt and constraints.txt to hub >=1.23.0,<2.0, because diffusers at the pinned commit requires it, but left the flat <1.0 cap in overrides-darwin-arm64.txt. That file is macOS-arm only, which is why only the mac legs see it and Linux and Windows stayed green. The failure then presents as something else entirely: uv gives up, the installer falls back to pip, and the clean-machine trace fails on "installer invoked toolchain: rustc" rather than on the resolution. The cap's own comment explains it exists so the resolver can never pair hub 1.x with a pinned transformers 4.57.6 / hub 0.36.2 stack. That is still true below python 3.10 and still capped there. At 3.10 and above this branch is on transformers 5.5.0 and hub 1.x, so the premise is gone, and mlx-audio's own >=1.0 floor is satisfied by the 1.x window anyway. Checked by collecting every hub specifier that applies per Python version across all five files: py3.9 resolves to 0.36.2 as intended, py3.13 to 1.23.0/1.27.0. Before this change py3.13 resolved to nothing. * Make H3's native download use the repo the family advertises The curated-mirror test main added only inspects VideoFamily.gguf_repo. H3's native path does not read that field: video_minimax_h3.py has its own H3_GGUF_REPO constant, used for both the transformer and the Qwen3-VL encoder. So pointing the family at the unsloth mirror in the previous commit satisfied that test while the actual one-click download still came from a community repack, which is the exact failure the test exists to prevent. Pointed the constant at the same mirror and added test_the_h3_native_repo_matches_the_family_gguf_repo to pin the pair, so the two cannot drift apart again. Verified it fails when the constant is put back to leejet, so it is not passing vacuously. The mirror now carries the Qwen3-VL encoder quants alongside the denoisers, byte-identical in size to the community ones, so this repo alone satisfies both of h3_native_hub_files' hub entries. The encoder is part of MiniMaxAI/MiniMax-H3 itself (FL2VA/text_encoder) which we already mirror publicly at unsloth/MiniMax-H3 under the same licence, so shipping a quantization of it beside the denoisers is the same act. Also checked the encoder-tier routing survives the dynamic rung names: -UD-Q2_K_XL selects the Q2_K_M encoder and -UD-Q3_K_XL the Q4_K_M one, asserted in the new test. Updated the download-plan test, which hardcoded the old repo id. NOTE: unsloth/MiniMax-H3-GGUF is private. Unlike before, that now really does gate this: the native path downloads from it. It has to be public before this merges. * Pin H3's companion-checkpoint guard under test validate_h3_transformer_filename had no test. That mattered less when the denoisers lived alone; the mirror now ships the Qwen3-VL encoder quants in the same repo, so the picker lists both and a user can name either. Loading a 12-17 GB encoder as the transformer would fail deep inside sd-cli instead of at the boundary. The accept cases include the dynamic rung names on purpose. The guard is a prefix/suffix check and `-UD-Q2_K_XL` is a shape it had never seen when it was written; it happens to pass, and now that is asserted rather than assumed. Checked the test fails when the prefix check is dropped, so it is not vacuous. * Record why H3 drops --vae-on-cpu, now that the abort is fixed The comment justified the drop entirely by an sd.cpp abort. That abort is fixed in the Unsloth fork, which would have made this look like a stale workaround to revert once the fix reaches the pinned prebuilt. Measured on a build carrying the fix, 640x384, 25 frames, 4 steps, q4_K, with --offload-to-cpu --clip-on-cpu already applied: adding --vae-on-cpu moved peak VRAM 12.42 -> 12.42 GiB and wall time 20.9s -> 100.4s. Under --offload-to-cpu the peak is set by the streamed denoiser, so the flag saves nothing and costs 4.8x. It stays off on its own merits. * Pin the sd.cpp prebuilt that actually renders MiniMax-H3 The pin was master-812-ea7f0c8, a stock upstream build, and on a stock build H3 does not work: it aborts on the default --cfg-scale, aborts again on --vae-on-cpu, and its 1-D norms are quantized into an output uncorrelated with its own bf16 reference (LPIPS 0.981). The Studio side worked around the first by pinning cfg to 1.0 and the second by dropping the flag; the third had no workaround on the consumer side at all. All three are fixed in unslothai/stable-diffusion.cpp and open upstream as leejet/stable-diffusion.cpp#1861, #1862 and #1863. The mirror's prebuilt pipeline now applies them on top of the aged upstream tag it already builds, and marks such a build with a -u<id> suffix naming the patch set, so master-813-bfbef5b-u0665242 is upstream master-813-bfbef5b plus those three patches and nothing else. The patches are deleted once upstream releases them, at which point this pin goes back to a plain tag. Verified on the published Linux x86_64 asset, not on a local build: both new error strings are in the shipped sd-cli, and running it on a q4_K H3 denoiser without --mode vid_gen now exits 1 with the instruction instead of core dumping on a ggml assert. test_video_backend's fake engine returned the old tag as its version string, which read like a second pin; it only needs a non-None value, so it now says so. * Close two gaps the H3 mirror switch opened Both are consequences of the two preceding commits, found in review. The prebuilt pin is now mirror-only (master-813-bfbef5b-u0665242), and _resolve_with_fallback still asked leejet for it. That request is a guaranteed 404 by construction, since the -u<id> suffix marks a build only the mirror makes, so it was a wasted round trip on every install. Worse, when the mirror genuinely cannot serve a host the fallback lands on leejet's latest, which has none of the H3 fixes. For every other model that is the right trade, better a stock native engine than none. For H3 it is not visible: it aborts on the default cfg-scale, aborts on --vae-on-cpu, and a blanket --type renders a broken video rather than failing. A user who saw only the generic 'falling back to leejet' line had nothing connecting that to the output. It now says so. Second, hub/utils/gguf.py filtered H3 companion GGUFs by the old community repo id only. The mirror the family and catalog now advertise carries the Qwen3-VL encoder quants beside the denoisers, so a 12 GB text encoder was being aggregated as if it were a selectable transformer quant. Both bundle repos are now recognised, case-insensitively, and the cache-dir match follows. Tests are mutation-verified rather than assumed: restoring the upstream 404 attempt fails the ordering test, removing the H3 warning fails the fallback test, and dropping the mirror from the bundle set fails three. The ordering test deliberately makes the mirror serve nothing, because with the mirror serving the first attempt succeeds and the upstream attempts are never reached, which made an earlier version of it pass under its own mutation. A third test pins the native loader's H3_GGUF_REPO to the bundle set, since those live in different files and a future repo move that updated only the loader would silently reintroduce the same aggregation bug. * Exclude MiniMax-H3's small-M projections from int8 _INT8_FAMILY_EXCLUDE_NAME_TOKENS has entries for qwen-image and hunyuanvideo-1.5 but none for minimax-h3, and H3 needs one for the same reason they do. H3's adaLN projection is named adaln_proj, which no token in the generic list matches: 'norm' is the closest and does not appear in the name. On the dense checkpoint that projection is Linear(2688 -> 96768), so it clears min_features = 512, gets quantized, and then runs at M = 1. Inductor lowers int8 matmul to _int_mm, which requires M > 16, so it raises 'self.size(0) needs to be greater than 16, but got 1' at the first denoise. The offline prequant builder bakes it in happily, which is exactly the drift the exclude list exists to prevent for Flux and Qwen. The pruned-modulation form hides this rather than fixing it: there adaln_proj is Linear(8 -> 96768) and falls under min_features anyway. So this exclusion is what makes the DENSE path correct and is a no-op on the pruned one. context_embedder and token_refiner are added for the same reason hunyuanvideo-1.5 excludes its text stream. Measured at M = 10 text tokens against the video stream's thousands, they are 3.47% of GEMM time even in the slow eager int8 path, so leaving them bf16 costs nothing measurable. This is what made an earlier measurement conclude int8 does not work on H3. It does: on the pruned form int8 compiles and is 4.24% +-0.54% faster than fp8 at identical memory, paired over 12 renders. * Say which H3 component could not be downloaded, and why H3 pulls four files from two repos, and the Hub returns the same 'Repository Not Found ... make sure you are authenticated' for a repo that does not exist, one that is private, and one your token does not cover. A user reading that has no way to tell which of the four failed, and the wording points away from the real cause whenever the repo exists but is not public. That is the state the GGUF mirror is in today: it is unpublished, so picking H3 fails with a message suggesting the user fix their token, which will not help. This replaces it with the repo, the component, and the actual remedy, and says the other components are unaffected so the failure is not read as total. Gated repos get different wording, since accepting a licence is a different action from waiting for a repo to be published. Anything that is not a recognised access error is passed back unchanged rather than reworded, so a timeout or a full disk still reads as itself. The helper returns the exception instead of raising, so the caller keeps raise-from and the original traceback survives. Mutation-verified three ways: rewording every error (timeouts included), fixing the component name to 'denoiser', and giving gated repos the private wording each fail the test. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stop MiniMax-H3 holding two copies of its video VAE Two thirds of an H3 render's peak is not activations. Measured at 640x384 across 124 frames, a 20.25 GB int8 denoiser peaks at 36.96 GB, and the gap is almost all weights: the video VAE alone is 10.42 GB because diffusers pins it to float32, and a further 4.91 GB is autocast's own float16 copy of those same weights. A memory snapshot puts 92.9% of the transient in 437 blocks allocated from nn.Linear, largest 67.1 MB, which is the decoder's [2048, 16384] SwiGLU projection in float16. MiniMaxH3VideoDecodeStep wraps vae.decode in torch.autocast(float16), and autocast caches every weight it casts for the lifetime of the region, so the float32 original and its float16 twin are both resident through the whole decode. Storing those weights as float16 up front makes the cast a no-op and removes both. This is not an approximation: x.to(float16).to(float16) is x.to(float16), and the four regression tests check that on a real matmul rather than on the reasoning. The audio VAE decode is not under autocast, so it keeps float32. t2va starts from noise and never encodes, so vae.encoder and vae.quant_conv go too. That part is gated on the workflow name rather than dropped unconditionally, because an image-conditioned workflow needs them. Measured 36.96 -> 28.37 GB peak with the encoder drop and the pre-cast, 28.27 with expandable_segments as well, over 5 prompts x 2 seeds. Speed is unchanged (-0.05% +-0.86% eager, -0.34% +-2.23% compiled), and every arm hashes identically to its control on latents, audio_latents, frames and audio. The estimator's base still reads 68.5 GB. That figure was measured on the bfloat16 modular components, not the int8 arm above, so it stays put until it is re-measured in the same configuration rather than adjusted by arithmetic. * Pad MiniMax-H3's small-M int8 linears instead of leaving them dense torch._int_mm asserts self.size(0) > 16. torchao's eager path never trips it (safe_int_mm falls back to a widened matmul), but inductor lowers the same quantized linear straight to _int_mm, so any quantized Linear invoked at a small activation row count crashes under torch.compile. Until now the fix was to leave those linears dense bf16, which on H3 meant excluding context_embedder and both token_refiner blocks: 13 linears, 798M parameters, 0.80 GB of weights the int8 checkpoint was not allowed to touch. Pad instead. diffusion_quant_pad.PadToMinM pads the flattened row count up to 32, runs the GEMM and slices the result back, so the module compiles with no change to the quantization config and the caller's rows come back bitwise unchanged. Verified bitwise on all 65 (module, M) cases across H3's 13 linears at M = 10, 13, 14, 17, 19, on real torchao-quantized weights; compiling those same modules unpadded raises the _int_mm assert at M = 10, 13 and 14. Two properties carry that exactness and both are asserted rather than assumed. The pad rows replicate row 0, not zeros: an all-zero row has amax 0, so the activation quantizer divides by zero. And the activation scale must be per row, so each kept row's scale comes from that row alone; a quantized Linear whose granularity cannot be proven per row raises instead of being quietly skipped, because a half-padded transformer compiles on the wrapped modules and crashes on the rest. Everything below pad_to normalises to pad_to rather than only what is below the floor, so one inductor graph covers every prompt length in the range. H3's seven eval prompts run at M = 10..19, which straddles the floor, and padding only to 17 would leave three shapes behind. The wrapper reparents the Linear, so it runs after quantize_ on the runtime path and after load_state_dict on the prequant one. The offline builder drives quantize_ directly and saves the state dict, so it never sees a wrapper; PadToMinM is also state-dict transparent as a second line of defence, saving and loading under its own prefix so a wrapped transformer still writes context_embedder.weight. Scoped to minimax-h3. qwen-image, qwen-image-edit and hunyuanvideo-1.5 have the same small-M shape but published int8 prequant checkpoints whose metadata bakes the current exclusion set, and _validate_checkpoint compares that set against exclude_tokens_for_scheme, so they move only together with a rebuild. adaln_proj stays excluded for a different reason: on the dense checkpoint it is Linear(2688 -> 96768) and runs at M = 1, while on the pruned form it is Linear(8 -> 96768) and falls under min_features anyway (verified: the filter rejects all 51 for min_features). Measured on B200, 640x384 x 124 frames, 4 steps, 7 prompts x 2 seeds, the two arms alternated within each cell so drift on a shared box cancels: checkpoint 21.052 -> 20.254 GB (-0.798, -3.8%) transformer 21.051 -> 20.253 GB (-0.798) load peak 21.137 -> 20.336 GB (-0.801) render peak 37.766 -> 36.966 GB (-0.800, -2.1%) step time +0.0597 s +-0.0057 eager, +0.0533 s +-0.0086 compiled The time is torchao's un-fused eager activation quantization on 13 modules that run ONCE per render, so it is a fixed cost rather than a per-step one, and it does not scale with steps or resolution. Compiling those modules alongside the blocks removes it: +0.0034 s +-0.0040, no detectable difference. Quality is unchanged as far as n = 14 can resolve: against the same bf16 twin the padded arm sits +0.0070 +-0.0151 LPIPS from the excluded one, which rules out a degradation larger than 0.022. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Classify the MiniMax-H3 GGUF bundle as video in the cached inventory The H3 GGUFs are stable-diffusion.cpp conversions and carry no metadata keys at all (kv_count 0), so general.architecture is absent where the LTX-2 and Wan video GGUFs declare ltxv/wan. _arch_to_task therefore left the downloaded repo with no task, which drops it from the Video picker's On Device list and hands it to chat as a plain GGUF. Key the two bundle repo ids before the arch is consulted. * Release the VIDEO GPU claim when H3 native falls back to the CPU build On a CUDA/ROCm host /video/load acquires the VIDEO arbiter owner because the resolved device target is not CPU. _run_load_h3_native then asks for an accelerator-matched sd-cli, and the pinned prebuilt release publishes no Linux CUDA/ROCm archive, so ensure_sd_cpp_binary returns None and the load commits the CPU build with native_device = cpu. Nothing dropped the VIDEO claim, so the next chat or image acquire evicted and unloaded an H3 runtime that holds no VRAM. Release the claim once the CPU fallback is committed, through release_if so the token check is atomic against a newer load that already took ownership. Mirrors the CPU-only native release /images/load already does. * Video: protect the native H3 companion repos, cancel the modular denoise, forward the hub token - VideoBackend.loaded_repo_ids() publishes the repos the committed native H3 runtime re-reads every generation (Qwen encoder + both VAEs), and the delete-cached guard consults it, so deleting a companion under a loaded model is refused. - The H3 modular workflow no longer falls back to a null progress context: the denoise loop drives pipe.scheduler.step once per step, so the existing wrapper gives it the same per-step progress and cancellation the other callback-less pipeline gets. - load_components() gets the Settings token, so gated/private component loads are not issued anonymously. * Reject stable-diffusion.cpp builds that predate MiniMax-H3 support ensure_sd_cpp_binary hands back whatever find_sd_cpp_binary locates and only probes that it runs, so an install upgraded from an older Studio kept serving its pre-H3 managed sd-cli. The H3 load's only gate is SdCppEngine.version(), which that binary passes, so the load reported ready and the failure surfaced on the first generation, after the whole bundle had downloaded. Gate the H3 path alone on the capability instead of a version string: upstream added --ref-video and the other H3-only options in the same commit that added MiniMax-H3 (leejet/stable-diffusion.cpp#1854, master-812-ea7f0c8), and the release prebuilts report 'version unknown, commit unknown' because they are built without a .git directory, so --help is the only usable signal. Image generation keeps accepting any user-supplied build. A stale copy under the installer-owned root is removed so the pinned prebuilt reinstalls; a build the user supplied is left in place and the load fails naming it, the same ownership split _usable_or_discard_managed makes. A --help that cannot be read means 'cannot tell', never 'no H3'. * Distinguish a reused CPU sd-cli from an accelerator build on an H3 load On a Linux CUDA host the first H3 load installs the CPU prebuilt through the fallback and correctly commits native_device = cpu, because the pinned tag publishes no Linux CUDA, ROCm or Vulkan asset. Every later load then calls ensure_sd_cpp_binary(accelerator = cuda), which finds that same CPU binary and returns it without looking at what it was built for, so the fallback was skipped, native_device stayed cuda, and Studio applied GPU offload policy and retained the VIDEO gpu_arbiter claim while sd-cli ran wholly on the CPU. A later chat or image acquire then evicted an unrelated GPU model. Fall back on what the binary offers rather than on whether one was returned: sd-cli --list-devices prints one name/description line per available ggml backend device, so a CPU-only build answers with CPU alone. The second load now reaches the same cpu conclusion as the first, which is what lets the existing release_if drop the stale claim. Unreadable output, or an older build that rejects the flag, keeps the GPU: neither says the accelerator is missing. * Guard the H3 companion repos while a native video load is downloading _run_load_h3_native downloads from repo_id, the H3 GGUF companion and the H3 component repo, but the in-flight state only carried repo_id and base_repo. The cached-model delete guard reads loading_repo_ids(), so it allowed deleting Comfy-Org/MiniMax-H3, and the GGUF companion when the load comes from another mirror or a local file, while those files were still downloading, which fails the load. Carry the companions on the loading state, the way the image backend's _SdLoading already does, and publish them from loading_repo_ids(). This is the in-flight twin of loaded_repo_ids() and covers the same repos. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep curated Recommended rows searchable and give them their metadata Two things went wrong with the curated video models the Recommended list paints from the catalog rather than from a live Hub listing. Search dropped them. The Recommended search matched the query against `recommendedIds`, which filters out every id already on disk because a downloaded model gets its own On Device row. The unfiltered Recommended list does not filter that way: it renders the curated seeds and badges the downloaded ones. So a curated model was visible in the list and unfindable by typing its name the moment it was downloaded, and only a live listing row could bring it back, which a repo the listing does not return never gets. `searchableRecommendedIds` unions the seed ids with the listing ids, seeds first, deduped case insensitively (the HF cache lowercases repo ids), so both lists agree on what exists. Rows rendered bare. Everything past the id came from the listing alone, so a curated row the listing never returns showed no parameter chip and no capability glyph while its neighbours showed both. The catalog now carries the two facts nothing else can supply: `totalParams` per artifact and `capabilities` per group, read through `curatedTotalParamsFor` and `curatedCapabilitiesFor`. Both are fallbacks only. A listing row wins wherever there is one, because real tags and a Hub-reported total outrank anything hand written here. The parameter counts are measured, not guessed. The MiniMax-H3 figure is the sum of the tensor shapes in the BF16 GGUF of the FL2VA denoiser that repo publishes; the LTX-2.3 figure is what the Hub reports for its repo, carried so the row looks the same offline or rate limited. One more inconsistency fell out of the same place: `searchRowFits` hides anything it cannot size (`requireKnown`), and it could not size a curated repo with no listing row and no "<n>B" token in its id, so turning on "Fits on device" hid from search a row the unfiltered list still showed. It now falls back to the curated total the same way it already fell back to the curated size. Covered by studio/frontend/tests/recommended-curated-row-metadata.test.ts: 15 assertions over the search pool, the two catalog lookups, the fit check, and the four picker call sites that read them. * Load MiniMax-H3 from a hosted pre-quantized denoiser Video families had no way to reach a hosted pre-quantized DENOISER. VideoFamily carried gguf_repo and te_prequant_repos (the text encoder) only, and nothing in video.py consulted a denoiser table, so a MiniMax-H3 load either pulled the full 66.3 GB bfloat16 DiT or nothing at all. The image side already solves this with DiffusionFamily.prequant_repos plus family_prequant_repo(), so this follows that shape rather than inventing a second one. Four parts. VideoFamily gains prequant_repos, prequant_variant_repos and prequant_subfolder, resolved by video_family_prequant_repo() / video_family_prequant_schemes(), mirroring the image resolver. The registries stay separate, as the module header requires, so the base-id normaliser is local rather than imported. The shared resolver learns an optional subfolder. The hosted video checkpoints nest theirs one level down instead of keeping it at the repo root, and the prefix has to reach BOTH candidate names or the primary 404 is followed by a second one and the load silently falls back to the dense download. Always a literal forward slash: these are Hub repo paths, and a Windows join would miss the cache. The cache and download plumbing already handled a nested name, so only the filename builder changed, and every existing call site is byte-identical. The modular workflow builds its denoiser through its own component loader, so there is no dense module to quantise in place. A hosted checkpoint is therefore the only way to run that transformer quantized, and pre-seeding it with update_components() before load_components() is also what stops the dense download: load_components(names=None) skips a component whose attribute is already set. Passing names= instead would have forfeited the workflow's own block pruning, which is what already avoids the 61.7 GB Ref2VA transformer. Those checkpoints carry the pruned adaLN, where the modulation is a rank-8 affine factorization of the time-embedding curve plus a shared table, and roughly 40% of the released model's parameters go. Against the base repo's dense config the model is four keys short, one over and fifty-one shapes wrong, so the strict load fails and the checkpoint is unloadable by every route. video_minimax_h3_adaln.py reshapes the model between from_config and load_state_dict: table lookup with interpolation instead of the timestep MLP, no SiLU (the table already holds the activation's own output projected onto the basis), and the modulation cast to the block stack's dtype, without which the first quantized matmul dies on mismatched dtypes. Bound per instance, so a dense load in the same process is untouched. Finally the refusal becomes honest. A single-file pick on a modular family used to reach the loader only after ~98.7 GB had downloaded AND after the resident pipeline had been evicted to make room for it, because download-plan returned 200 and validation passed. Both refusals now run in validate_load_request, ahead of the diffusers availability probe so they still fire where diffusers cannot be imported, and each names what to pick instead. download-plan forwards transformer_quant to validation and to the plan, without which the quant-keyed refusal never fires on the route that stages the download and the plan stages shards the load never opens. * Point both MiniMax-H3 schemes at one hosted pre-quantized repo The two hosted pre-quantized denoisers were split across two repos with the checkpoint nested one level down, so reaching them needed a mechanism the image side has never had: a VideoFamily.prequant_subfolder field, a prequant_subfolder_prefix() normaliser, and a subfolder keyword threaded through both resolve_prequant_source() and usable_prequant_source(). Both schemes now live in ONE hosted repo, at the root, named <Model>-<SCHEME>.pt. That is the layout every image-side prequant repo already uses, and it is exactly what prequant_repo_filename() builds unaided, so the whole mechanism goes. Match an existing convention and the code should shrink, not grow: -22 lines in diffusion_prequant.py, -6 in video_families.py, -6 in video.py, with no new concept to carry. Landing on the primary name also fixes a memory-planning under-credit. cached_checkpoint_path() deliberately credits only the PRIMARY filename, so that a cached legacy artifact cannot pin a stale name once a repo ships the real one. While these checkpoints were published as transformer_<scheme>.pt the primary never existed: every hit came through fallback_filename, and planning therefore read an already-cached checkpoint as "this would have to download" and handed the pick to GGUF. The primary is now the published name, so the probe hits it. fallback_filename stays. It still covers repos that have not been renamed, and dropping it is a separate decision from this one. Tests: the subfolder-prefix cases are replaced by the naming they now guard -- both schemes resolving to one repo, the primary resolving to a root-level <Model>-<SCHEME>.pt with no directory component on any platform, the cache probe being asked for that primary name, and the repo's own scheme suffix being stripped and replaced rather than carried through. Five mutations run, each caught by the named test and reverted: two repos again (M1), the primary nested under prequant/ again (M2), the suffix strip removed (M3), the cache probe keyed on the fallback (M4), the fallback name dropped (M5). * Add MiniMax-H3 image and reference video conditioning * Improve H3 finalization progress * Report real sd.cpp progress on the Video page instead of a frozen 0 of 30 A native (GGUF) video generation reported phase "denoise", step 0/30 for its entire run and then flipped straight to "completed". Two separate things were wrong, and the progress endpoint could not move until both were fixed. The parser looked for r"(?:step|sampling)\D+(\d+)/(\d+)". sd-cli's sampling bar contains neither word. It prints |=========> | 7/30 - 21.50s/it so nothing ever matched. Anchor the pattern on the bar and on the trailing speed unit instead. A bare "n/m" is deliberately not enough: an unrelated ratio in some other log line must not drive the progress bar. The reader also delivered every redraw one step late. sd-cli redraws in place, and its carriage return LEADS the record while the newline only arrives on the final step, so a reader keyed on CR/LF cannot produce step 1 until step 2 has been flushed. Treat the erase-to-end-of-line that closes each redraw as a terminator too, and read the pipe with buffer.read1 so a record that carries no newline is not stuck behind a blocking readline. Escapes are stripped before a record reaches on_log or the error tail. Streams without a raw .buffer keep the old line iteration. The same bar shape is printed by three different things, so the parser now tells them apart rather than reporting whichever came last. Weight load prints it with a byte rate, and tiled VAE decode prints an identical s/it bar counting TILES: without a guard a run finished sampling at 30/30 and then jumped backwards to "step 1/16". Load and decode are real work with no sampling step, so they report their own phase and a null step rather than a fake 0 of 30. ETA is measured from the first step, not from job start, so the one-off weight load is not charged to every remaining step. Verified end to end against a real CUDA sd-cli MiniMax-H3 generation: the step advances 1..6 over 6.2s..9.7s of wall clock, load and decode are reported as themselves, and the ETA tracks. * Read sd-cli's in-place progress redraws so the Video bar moves during sampling The native H3 progress bar had two independent causes and fixing either alone changed nothing observable. The bar pattern is now correct, but the reader still was not. sd-cli redraws its sampling bar in place: one printf per step shaped "\r<bar> <n>/<total> - <speed>\033[K", with a newline only on the final step of a phase. The drain loop did `for raw in proc.stdout`, which terminates on LF only, so every redraw sat in the buffer until the next one's carriage return arrived and the last one until sampling was already over. The Video page saw nothing. Split the raw pipe into records on CR, LF, or the trailing erase-to-end-of-line, reading through buffer.read1 with an incremental UTF-8 decoder so a multi-byte character straddling two reads survives, and strip the CSI escapes before the record reaches on_log or the error tail. Streams without a raw .buffer (test doubles, non-pipes) fall back to line iteration. The new backend test drives the real byte stream through both halves, one flush per read, and asserts each step is visible on the read that carried it rather than one redraw later. * Give MiniMax-H3 first and last frame conditioning in the video backend MiniMax-H3's released transformer is the FL2VA one: text-to-video is the same checkpoint run with no keyframes. Studio only ever ran it text-only, so the Video generate request had nowhere to attach a reference frame. The load used to prune the block graph to t2va. That argument prunes STATICALLY, so an fl2va-pruned pipeline runs the keyframe blocks on every request and cannot serve a text-only one at all: it raises packing an empty conditioning list. The load now keeps the whole auto graph, which selects per request, and bounds load_components to the keyframe workflow's component set instead, so the 61.7 GB Ref2VA partition is no more loaded than before. Measured against the released checkpoint: a text-only request through this pipeline is bit-identical, video and audio, to the same request through a t2va-pruned one. A keyframe is a geometry anchor, not just conditioning, so the canvas comes from its aspect ratio through the released arithmetic (768 short edge, area capped at 768x1344, both axes rounded to 32) rather than from whatever resolution preset was selected. An arbitrary size produces a garbled clip rather than an error. sd.cpp already implements the same conditioning, so that side is the existing --init-img / --end-img flags with the frames staged as PNGs. Only MiniMax-H3 declares the capability, and status reports it, so Wan and LTX do not grow a control that does nothing. * Cover the MiniMax-H3 keyframe path with tests Registry (which families declare it, and the canvas a keyframe resolves to), the load wiring (whole block graph, component set still bounded, VAE encoder kept), request handling (decode, refusal, canvas override, what reaches the pipeline call) and the sd-cli argv. * Add the reference-frame controls to the Video page First and last frame pickers, shown only for a family whose status declares keyframe conditioning, so Wan and LTX are unchanged. The Images page's source picker moves to a shared component rather than the Video page growing a second upload path; both send the same data URL to the same backend decoder. While a frame is attached the Resolution preset is disabled and says why: the frame's aspect ratio resolves the clip's size, the way the model itself does. The gallery recipe records which ends were pinned. * Check the keyframe canvas against the pipeline's own resolver The canvas rule is a checkpoint contract, so pin it to the released implementation rather than only to hand-written expectations. Skipped where diffusers does not ship MiniMax-H3, which is most runners. * Revert the standalone H3 keyframe implementation oobabooga/unsloth#121 covers first frame, last frame, first-and-last, Ref2VA and the canvas rule, and it reached the same load construction independently. Two implementations of the same feature on one branch is worse than either, so this takes mine back out and leaves the branch ready for that work to land whole. The one finding worth keeping from it is already reflected there: passing workflow= to ModularPipeline.from_pretrained prunes the block graph statically, so the pipeline must be built unpruned and only load_components bounded. * Keep the pre-quantized MiniMax-H3 denoiser resident so a generation can run Loading H3 with a hosted pre-quantized denoiser worked, but every generation died on its first denoise step: Attempted to set the storage of a tensor on device "cuda:0" to a storage on different device "cpu". This is no longer allowed; the devices must match. ComponentsManager.enable_auto_cpu_offload parks every component on the CPU and moves each one onto the accelerator inside its own pre_forward, that is from within the block that is already executing. The text encoder and the VAEs survive that; a torchao-quantized denoiser does not, because the device change reaches return_and_correct_aliasing, which tries to alias a CPU storage to an accelerator tensor. Moving the same module at load time, outside any executing block, works. So place it once at load and take it out of the offload rotation: drop its hook, unlist it from the other components' eviction candidates, and move it. Everything else is unchanged, and the encoder and VAEs still offload around it. Keeping it resident is what asking for a quantized denoiser buys in the first place: the hosted checkpoint is about 20 GB against 66.3 GB dense. Verified end to end on a B200: MiniMaxAI/MiniMax-H3 loaded with the hosted fp8 denoiser, then a 1280x768, 124-frame clip generated from a start frame in 167s. The clip's first frame matches the supplied image and the motion is coherent. * Apply the pinned Diffusers revision on a fresh install, not just an update MiniMax-H3 needs a Diffusers revision newer than any published release, and Studio refuses to load it otherwise. The pin was in studio/backend/requirements/base.txt, and a clean install.sh run still ended up on diffusers 0.39.0 from PyPI, every time, with nothing in the log to say so. base.txt is never installed by install.sh. install.sh installs unsloth itself, whose own metadata pulls a diffusers release in transitively, and then runs install_python_stack.py with SKIP_STUDIO_BASE=1, where the base-packages step is a bare `pass`. So the pin applied on `unsloth studio update` and on the no-torch path (install.sh installs no-torch-runtime.txt directly) and was dead on exactly the path a new user takes. Reproduced on a clean install into a throwaway prefix before and after: 0.39.0, then 0.40.0.dev0 with MiniMaxH3Transformer3DModel present. The revision now lives in its own diffusers-pin.txt, installed by a step that sits outside every skip_base / NO_TORCH branch and after every other requirements file, so nothing left in the run can re-resolve diffusers back to a release behind it. No forced reinstall is needed: a direct URL requirement is not satisfied by a resident registry install, so the step is a no-op once the environment is already on the pin. tests/studio/install/test_diffusers_pin.py holds the shape in place: exactly one requirements file may name diffusers, the pin must be a full commit sha rather than a moving ref, the install step must sit at function top level rather than under a conditional, and it must come after every other requirements install. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: let the Video and Images pickers see a local diffusers pipeline A model already on disk never reached the pickers' On Device list unless it happened to keep a weight file beside a root config.json. Every image and video model downloaded as a pipeline keeps its weights in component subdirs under a root model_index.json instead, so the hub inventory scan behind /api/hub/local rejected it. Pointing Custom Folders at one was worse than empty: the LM Studio publisher walk descended into the pipeline and offered vae, transformer, text_encoder and audio_vae as four separate models, none of them loadable. Teach the hub scanner the same pipeline-root test routes/models.py already applies, in the three places that only accepted a root config plus loose weights, and keep a pipeline row through the custom-folder format filter: the layout has no loose weight to classify, so the row is "unknown" by construction rather than by fault. * Cover the local diffusers pipeline scan with tests * Pin what the pipeline exemption must not let through Three gaps in the cover added with the scan change, each found by mutating the fix and watching the suite stay green. The custom-folder format filter now waves a row through on its shape, and nothing said what it still has to reject: replacing the whole predicate with True passed. A folder holding a config.json and no weights, which an aborted download leaves behind, reports the same "unknown" format and no loader can start it, so it pins the boundary. The predicate is applied to every row the filter did not already accept, and a row's path can be a GGUF file rather than a directory. A missing path, a file, and a directory whose model_index.json is itself a directory must answer False rather than raise, because an exception there fails the scan and empties the picker. The publisher walk was only covered one level up. Adding the model folder itself as a scan folder is the obvious thing to do and used to publish vae, transformer and text_encoder as three models. * Drop the unused H3_TASK_KEYFRAMES import from the video backend video.py only branches on H3_TASK_REFERENCES; the keyframe constant is read from video_minimax_h3 directly by the tests that need it. The hoisted-import safety net in Source lint flags the unused name as a blocker. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stage the hosted pre-quantized H3 denoiser in the video download plan The plan already drops the dense transformer shards whenever a hosted pre-quantized checkpoint covers them, but nothing put that checkpoint back: an int8 or fp8 H3 stage skipped 66.3 GB of base shards and added none of the 20.25 GB artifact the load actually opens. The byte total under-reported the stage by the size of the checkpoint, the disk preflight cleared a volume that could not hold it, and an offline stage completed without the one file the load needs. _denoiser_prequant_hub_files mirrors the pre-cast encoder helper: it resolves the family's hosted checkpoint, confirms the file really exists on the Hub, and prefers the repo-root name over the legacy scheme name in the same order the load tries them. An unreachable repo is logged and yields no files, so a gated or renamed artifact keeps the dense shards instead of sinking the plan. The progress-bar estimate is deliberately left alone: it counts cached bytes for the checkpoint and base repos only, so adding a third repo there would leave the bar permanently short of 100 percent. * List both H3 denoiser partitions in the picker, not only FL2VA The bundle-repo filter accepted only minimax_h3_fl2va*, so every published minimax_h3_ref2va* quant was hidden from the variant listing. The loader disagrees: validate_h3_transformer_filename accepts either partition, on the grounds that which one is picked IS the task, and h3_transformer_task routes Ref2VA to the reference-video workflow this PR adds. The community bundle repo publishes three Ref2VA quants today, so the reference path was unreachable from the remote catalog. Accept both prefixes from one shared tuple and keep excluding the Qwen3-VL encoder and VAE companions, which are never picks for either partition. The filter test asserted the old behaviour and is updated with it. * Bound H3 reference-audio decoding to the trained window The reference-video decoder already selects, resizes and refuses incrementally because the encoded size says nothing about the decoded size. The audio decoder did not: it appended every resampled block to a list and then allocated a second full buffer in np.concatenate, with no duration or sample cap. The route accepts 32 MiB of encoded audio, which is over half an hour of compressed stereo. That lands as roughly 1.9 GB of float32 and doubles again in the concatenate, and up to three references are accepted per request, so an ordinary long music or podcast file picked by mistake could exhaust the host before the background job even started. H3's reference window is 15 seconds, so anything past it is unusable rather than merely large. Refuse it while decoding, with the same shape of message the video guard uses, instead of decoding it first. * Refuse a quantized H3 reference load instead of seeding the keyframe denoiser The hosted pre-quantized checkpoints are FL2VA (keyframe) denoisers. Ref2VA shares their module shapes and the same base model, so resolve_prequant_source handed one back for a reference load, it passed every metadata check, and seeding it made load_components skip the real Ref2VA transformer. The request then generated from the wrong partition rather than failing, which is the worst of the three outcomes. The route accepts h3_task, so this was reachable from the public API even though the picker does not expose the choice yet. validate_load_request now refuses the pairing with a message naming the workable alternatives, in the same place it already refuses a scheme with no hosted checkpoint, and the modular loader drops to the released components if a direct call reaches it. Nothing changes for keyframe loads, which are what the checkpoints are. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Skip modular-workflow families in the dense text-encoder plan assertion The plan-unchanged sweep from main walks every family and asserts the dense budget plan_diffusion_memory received. MiniMax-H3 is the first modular-workflow family to reach that list, and load_pipeline dispatches to the workflow's own loader before the planner runs: each component is built by its own from_pretrained, so there is no single dense pipeline to budget and no plan call to assert on. Skip it the way the sweep already skips wan2.2-t2v-a14b. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Scope the paired-axes canvas rule to keyframe requests A half-specified width/height is long-standing, documented API behaviour: the backend resolves the missing axis from the family's default preset, in both validate_video_request_shape and _resolve_keyframes. Applying the new paired-axes rule as an unconditional request validator rejected those calls with a 422 before family validation ever ran, breaking existing LTX, Wan, Hunyuan and prompt-only H3 clients. The rule still holds where it means something: with a keyframe present the canvas is matched to the source aspect whenever either axis is missing, so the axis the caller sent would be silently discarded. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep the CUDA sd-cli pin and translate a mirror-only tag upstream The merge with main brings in the accelerator-aware installer, whose fallback translates a mirror-only -u<id> pin back to the upstream release it was built from instead of skipping the upstream attempt. That is strictly better: skipping kept the round trip cheap but dropped the pin entirely on every host the mirror does not build, leaving them on upstream latest. test_a_mirror_only_pin_is_never_requested_upstream asserted the old shape, that the fallback settled for upstream latest. It now pins the new one: never the literal -u<id> string, the translated release instead, and no latest attempt at all because the translated pin succeeds. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
Daniel Han
|
82bf2e6b9f
|
Fetch the llama.cpp validation model only when the smoke test reads it (#8248)
* Fetch the llama.cpp validation model only when the smoke test reads it An approved bundle proves integrity by sha256 and skips the staged smoke test, so the tiny GGUF probe goes unread on the default install path. It was still downloaded up front, which made huggingface.co a hard gate on every prebuilt install: a 429 there raised PrebuiltFallback and forced a multi-minute source build over a file nothing opens. Fetch it lazily instead, at the one branch that reads it, memoised so the two smoke steps share a single download. test_install_prebuilt_does_not_skip_unhealthy_existing_install tripped on the probe download as a proxy for reaching the normal install flow. That proxy is gone on the default path, so it now trips on validate_prebuilt_attempts, which is the behaviour it actually names. * Cover the probe rate limit end to end over install_prebuilt The unit tests pin validate_prebuilt_choice's behaviour, but the bug was that a 429 short-circuited the whole install before the release loop. Assert that over install_prebuilt itself: with download_validation_model raising 429, the flow must still reach validate_prebuilt_attempts rather than exit EXIT_FALLBACK. Verified it fails against the pre-fix source. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Resolve the probe before the candidate loop when a plan will validate validate_prebuilt_attempts catches Exception per candidate, so a probe download failing inside that try read as a failure of the bundle itself: a transient 429 demoted a healthy GPU pick to the CPU asset or an older release, and because the thunk memoises success but not failure, each further attempt re-downloaded after extracting another bundle. Hashless attempts always validate, so resolve the probe once before the loop whenever any attempt will need it. Plans that skip validation never call the thunk and stay lazy. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Resolve the probe before the release loop too The per-release handler in install_prebuilt swallows PrebuiltFallback and moves to an older plan, the same shape as the per-candidate handler one level down. A probe failure raised inside it installed an older llama.cpp build over a transient 429, and since the thunk memoises success but not failure, each plan retried the download. The probe does not depend on which release was picked, so resolve it once before the loop whenever any plan will smoke-test. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
954468a3cb
|
fix(studio): wire llama's OpenMP runtime into slim whisper installs (#8230)
* fix(studio): wire llama's OpenMP runtime into slim whisper installs The slim wiring mirrors libggml*/ggml*.dll plus libomp*.dll from the paired llama bin dir. The DLL glob was added for llama's clang-built windows-arm64 ggml-base.dll; the ELF side was left out on the assumption that Linux ggml takes libgomp.so.1 from the host. That holds for the gcc-built x64 slice but not for arm64: llama's arm64 libggml-base.so is clang-built and NEEDS libomp.so.5 with RUNPATH $ORIGIN, and that library ships inside the llama bundle rather than on the host. Every slim install on linux arm64 therefore wired a ggml stack whose loader immediately failed with "libomp.so.5: cannot open shared object file", so dictation fell back to Transformers. Add libomp*.so* and libomp*.dylib to the globs, and bump SLIM_RUNTIME_WIRING_VERSION so installs made under the old wiring re-wire instead of reporting themselves current: their marker does not list libomp.so.5, so the missing-library check that would otherwise repair them never fires. whisper.cpp side is unslothai/whisper.cpp#18 (CI wiring) and #19, which adds the library to requires_ggml_sonames. That gate reads the llama bin dir, where libomp.so.5 is a real file, so the new manifests pair correctly with both the old and the new installer. Verified against the real published llama.cpp arm64 and x64 cpu bundles: wiring the arm64 bundle now places libomp.so.5 beside whisper-server and pulls in no llama libraries; every soname the new whisper manifests require exists as a file in the paired llama bin dir on both arches; and an x64 whisper-server wired through link_ggml_runtime starts and returns a real transcription. All 350 whisper/llama/prebuilt-core install tests pass. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the libomp wiring comments for PR #8230 * Fix the sidecar's ROCm wiring version check for PR #8230 --------- Co-authored-by: danielhanchen <unslothshared@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
Etherl
|
968d2e50ff
|
Studio: fix Windows desktop updates failing integrity checks (#8185)
* Fix Windows desktop updater integrity checks * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Exclude nested compiled cache packages * Tighten updater comments * Keep package discovery test dependency-free * Shorten the einx Windows pin comment * Drop the einx pin comment --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
Daniel Han
|
b0ea23bbf2
|
Catch the llama-prebuilt validator stubs up with rocm_gfx (#8182)
* Catch the llama-prebuilt validator stubs up with rocm_gfx Both validators take a rocm_gfx keyword and the four stubs standing in for them do not, so the real call site passes it and the stub raises TypeError. It lands inside whatever assertion was running, which reddened four unrelated fallback tests on main rather than saying a parameter had moved. Adds the keyword, and a test naming the keyword-only set of each validator so the next one fails once, here, with the parameter that moved. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
oobabooga
|
69d555b98e
|
Studio: check llama.cpp cache access before setup (#8032)
* Windows: preflight managed llama.cpp cache access before setup and update * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix denied llama.cpp cache update errors * Trim canonical paths on both branches and probe listing on POSIX for PR #8032 Follow-ups from reviewing the denied llama.cpp cache preflight. Get-CanonicalDir only trimmed a trailing separator on the lexical branch. Resolve-Path preserves one, so "...\studio\" compared unequal to "...\studio" and Test-StudioHomeIsCustom called the default home custom. Get-ManagedLlamaCppDir then handed the preflight ...\studio\llama.cpp instead of the real cache, so a denied cache went undetected. The trim now runs after both branches, still guarded so a path root keeps its separator. setup.sh gated the prebuilt install on _studio_dir_unsearchable, which probes search (+x). Mode 111 passes that and still raises PermissionError inside install_llama_prebuilt.py, which lists the tree. Added _studio_dir_unreadable for call sites that list or replace rather than probe a known child. setup_fail only emitted [TAURI:ERROR] under UNSLOTH_TAURI_MODE. update.rs sets UNSLOTH_TAURI_UPDATE on every platform, so macOS and Linux desktop updates still showed "Update exited with code N" while Windows showed the reason. A blank USERPROFILE threw a raw binding error from the relocated resolver before Exit-SetupFailure could report it, so no [TAURI:ERROR] reached the app. Tests: the marker-readable, listing-denied case was asserted only as a literal substring, so one extra space reintroduced the early return with all 49 Python tests and all 125 PowerShell checks green. That assertion is now a regex, and the PowerShell suite builds the shape for real (icacls /deny :(RD) on Windows, mode 111 on POSIX) with a negative control. The shell suite gains the same mode 111 case and no longer compares empty grep output as an integer. * Fix the Tauri marker gate and narrow the USERPROFILE guard for PR #8032 Two defects in my previous commit, both found by simulating the changes rather than reading them. setup_fail joined both variables into one case subject, which is not an exact match. "*,1" matches any subject ending in ",1", so an unrelated UNSLOTH_TAURI_UPDATE=a,1 printed a stray [TAURI:ERROR] on a plain CLI run, and "1,*" did the same for a comma in UNSLOTH_TAURI_MODE. Testing each variable separately matches setup.ps1, which uses exact membership. Verified over the 144-case product of both variables: no false positives, no false negatives, exit codes preserved, and byte-identical to main for every value of UNSLOTH_TAURI_MODE when UNSLOTH_TAURI_UPDATE is unset or 0, so no existing CLI invocation changes. Added a regression test that fails on the old gate. The USERPROFILE guard used IsNullOrWhiteSpace, but Join-Path only rejects null and empty; it accepts a whitespace-only value. The guard therefore also stopped a run that previously completed, USERPROFILE=" " with a fully qualified UNSLOTH_STUDIO_HOME. IsNullOrEmpty keeps the clean message for null and empty, which is where the raw binding error was, and changes nothing else. * Run the llama.cpp access guard before the prebuilt and source branches The guard sat inside the prebuilt else-branch, so UNSLOTH_LLAMA_FORCE_COMPILE=1, a llama.cpp PR or source override, or anything else setting _SKIP_PREBUILT_INSTALL bypassed it. Those paths reach the phase 9 swap, which only probes access after `rm -rf "$LLAMA_CPP_DIR"` has already failed, so a denied cache stranded a completed source build instead of failing before it started. _assert_studio_owned_or_absent does not cover it either: it returns early unless the studio home is custom, so a denied default cache had no guard on that path at all. Verified by driving both helpers against a mode-000 tree: the ownership guard returns 0 on a default home while the access probe reports denied. Hoisted both checks, in the same order so the custom-home wording still wins, to just before the branch. The local-link paths are excluded because they already replaced or reused the tree. The late checks stay as defense in depth. * Use the read probe in both llama.cpp replace postconditions Mode 111 defeats `rm -rf` but stays searchable, so both postconditions fell through `_studio_dir_unsearchable` to the generic "could not be replaced" text and the user got no recovery guidance. Reproduced against a mode-111 tree: the rm fails, the search probe does not fire, and the run exits with the generic message. The local-link site sits above the hoisted guard, so it is the one that is reachable. The source-build site is only reachable when a tree becomes unreadable during the build, but that is the most expensive path to end with the wrong message, and it is the same one-word probe. Both now use `_studio_dir_unreadable` and print the permissions block. * Tighten the comments added by this PR --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <moonshotaisubstack@gmail.com> |
||
|
Daniel Han
|
8b34acebcd
|
Studio: stop the macOS launcher killing a warming backend, and the Mac tab blackout (#8076)
* Studio: recover the MLX self-heal when uv cannot resolve the venv interpreter On Apple Silicon the self-heal reinstalls mlx/mlx-lm/mlx-vlm to re-enable Train/Export. It ran uv with --python sys.executable and nothing else, so when uv declined that path the repair gave up and Train stayed disabled for good: MLX self-heal failed (staying chat-only): error: No virtual environment or system Python installation found for path `<studio_home>/unsloth_studio/bin/python`; run `uv venv` to create an environment A venv's bin/python is a symlink into the base interpreter, and macOS breaks that link routinely: a Homebrew or python.org point upgrade moves the target and the venv keeps a dangling symlink. The running process never notices, because it mapped the binary at exec time, so uv is the first thing to fail. Name the environment as well as the interpreter. _uv_python_target prefers sys.executable and falls back to the venv root when the interpreter no longer re-resolves, _mlx_install_env sets VIRTUAL_ENV from sys.prefix, and a uv run that still reports an unresolvable interpreter is retried once against the venv directory. Only that specific failure retries; an ordinary resolution failure stays a single run. VIRTUAL_ENV is set from sys.prefix rather than forwarded from os.environ, matching how UV_OVERRIDE is already handled: it names the install target, so inheriting it would let a caller redirect the install. When both attempts fail the venv itself is broken rather than merely missing MLX. uv's own text says to run `uv venv`, which would build an environment Unsloth does not manage, so the warning now names `unsloth studio update` instead. Reported on macOS 0.1.524-beta. * Studio: stop double-logging exceptions and sqlite-vec spam A macOS diagnostics bundle came back at ~300KB, and roughly half of it was the same tracebacks written twice. LoggingMiddleware logs request_failed with exc_info, which structlog renders as a full traceback inside the JSON "exception" field, then re-raises. Uvicorn logs that same exception again on stderr as "Exception in ASGI application", and the desktop shell mirrors every stderr line into tauri.log individually, so one failure cost about 90 log lines. Mark the exception once request_failed has reported it and filter uvicorn's duplicate on the uvicorn.error logger, the same technique run.py already uses for the startup line. An exception raised above the middleware carries no marker and keeps its traceback, and --verbose restores both copies. The bundle's own failure was a missing sqlite_vec/vec0.dylib, which the import check does not catch: every /api/rag/knowledge-bases poll opened a connection, failed to load the extension and 500ed. rag_db now warns once per process and raises RagExtensionUnavailable (a RuntimeError subclass, so existing handlers are unaffected), and list_knowledge_bases degrades to an empty list for that case only. A locked or corrupt database still surfaces as before. Also quiet the 2xx line for four boot-burst catalog reads (/api/providers/ registry, /api/providers/, /api/models/loras, /api/settings/personalization); 4xx/5xx and every mutation still log. And tauri.log only checked its 5MiB rotation threshold at startup, so a long session grew unbounded: the file logger now writes through a size-tracking handle that rotates in place. On the reported bundle this removes 945 of 2063 lines (89KB of duplicate stderr traceback) plus the 9 sqlite-vec request_failed events (48KB), leaving one warning line. * Studio: stop Mac launches blacking out the Train and Video tabs The platform store seeds chatOnly from the browser user agent, so on every Mac both rows rendered disabled from first paint, visually identical to a measured "this machine cannot do that", until /api/health answered. Since the hardware detection went lazy that reply can take seconds to minutes. Add capabilitiesUnknown() next to isChatOnly(), derived from the fetched flag that already means "a server-reported verdict is stored" (a deferred reply counts as settled: under the torch-warm kill switch nothing else is coming). NavRowDef gains a pending field, folded in by a small import-free resolver both render sites go through, so an unmeasured row stays enabled and reuses the existing spinner column instead of graying out. The root guard lets /studio and /video wait the verdict out rather than one-way redirecting them to /chat, and each page shows its own loading state while it does. Video also gets a real capability answer. There is no Apple path in the video backend, but a healthy Apple Silicon host is not chat-only, so the tab was enabled and would just fail at load. video_capability() mirrors export_capability() and is spliced into GET /api/system and GET /api/system/hardware as additive fields; the page renders a coming-soon panel on an authoritative false, and the sidebar tooltip is now derived from the reason instead of hardcoding "needs an NVIDIA or AMD GPU". Also retry a failed hardware probe in useHardwareInfo: it resolved to the unloaded default with nothing scheduled to run again, which would have left the new Video gate spinning for the session. * Studio: add a macOS tab-capability UI smoke and nav row test hooks Covers the two things reported on Apple Silicon 0.1.524-beta in one live run: Train and Video rendering blacked out for minutes after launch, and the desktop launcher killing the backend about a minute in with "Server stopped unexpectedly". The nav button now carries data-testid="nav-row-<id>" and data-spinner, so the smoke can tell a spinning row from a greyed-out one. Nothing reads them at runtime; resolveNavRowState still owns the behaviour. tests/studio/playwright_mac_tab_capabilities.py drives a live Studio: it samples the Train and Video rows from first paint and fails if either renders disabled while /api/health still reports hardware_detecting, walks Chat, Hub, Images, Train, Video and Export clicking each row and screenshotting the result, and polls /api/liveness and /api/health on a background thread for the whole run. The poll window is 330s by default, deliberately past the launcher's 300s startup grace: the reported crash landed at t+66s, so a backend that dies to the watchdog fails here rather than looking like a slow boot. Redirects away from /studio and /video are allowed only once the verdict is measured. * Desktop: stop the health watchdog killing a backend that is still importing torch The macOS "Server stopped unexpectedly" report was three probe timeouts inside the first 64s of a normal cold start. #7958 fixed the grace period being bypassed; this is the rest of it, on the probe itself. Probe /api/liveness instead of /api/health. Health awaits hardware detection through _await_hardware_detection on purpose, so probing it every 15s bills the watchdog for the warm thread's torch import. Liveness reads module-level caches only, which is what it was added for. Backends older than the route answer 404, so fall back to health, in the same order process::generic_backend_health_ok and desktop_backend_owner::fetch_liveness use, and accept "alive" or "healthy" so a downgrade still validates. Raise the per-probe budget from 2s to 10s. The C-extension imports hold the GIL and the process can go quiet for seconds at a time on a cold start (3735ms measured on /api/health, with a ~27s silence around it). preflight::backend keeps its own 2s: a timeout there dead-ends the launch instead of retrying, and the backend derives _HEALTH_DETECT_BUDGET_S from that number. Stop one early reply ending the startup grace for good. A backend that answers now can still miss the next three probes while it loads a large model, so has_seen_healthy is set only once a reply says the hardware verdict has settled. /api/liveness now carries the same hardware_detecting marker health publishes, plus hardware_detection_deferred when the warm is switched off and nothing will ever settle it; a backend too old to send either reads as settled, which is what the launcher assumed before. Adds the regression test #7958 landed without: the failure policy replayed against the reported timeline, and the probe covered against a stub backend on both routes. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: accept either password env var in the macOS tab smoke The repo's macOS workflow exports STUDIO_OLD_PW; the staging harness exports STUDIO_PW. Reading only the first made the script KeyError before it reached the backend under staging CI. * Studio: make the new liveness and RAG tests hold on a macOS runner Two assumptions that hold on this dev box but not on a bare macos-15 runner: studio_root_id is environment-derived and is legitimately empty on a fresh runner, so the liveness test asserts the key is present (which is what the launcher reads) rather than that it is truthy. python.org macOS builds ship a sqlite3 without enable_load_extension, so the healthy-path RAG test cannot open a connection at all and errored in fixture setup. It skips there. The unavailable-path tests, which are the ones this PR changes behaviour for, still run everywhere. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: drop the MLX self-heal venv-root retry, which cannot work Codex was right that passing the venv root to uv does not recover a dangling bin/python. Confirmed against uv directly, with a venv whose interpreter symlink was pointed at a missing target: --python <venv>/bin/python error: No virtual environment or system Python installation found for path ... --python <venv> error: No virtual environment or system Python installation found for directory ... VIRTUAL_ENV set, no --python error: Failed to inspect Python interpreter from active virtual environment All three fail the same way, so the fallback and the retry were a placebo: when the first attempt already resolved to the venv root the retry was skipped, and when it ran it repeated a call that cannot succeed. Keep the part that does work. uv refusing the environment means the venv is broken rather than merely missing MLX, and nothing this process can pass to an install command fixes that, so the warning names 'unsloth studio update' (which rebuilds the environment) instead of uv's own 'uv venv' suggestion, which would build one Unsloth does not manage. _venv_root stays: it names the environment in that message. * Studio: hold the startup grace for the whole warm, not just hardware detection The health watchdog ends its five minute startup grace once a liveness reply says the backend is no longer warming, and that signal was `hardware_detecting`. But hardware detection is only the first of utils/torch_warmup.py's stages: the marker disappears while inference_backend, transformers, datasets and unsloth_zoo are still importing, and those are the C-extension imports that hold the GIL longest. So the grace could end mid warm and a stall spanning three 10s probes would count as three dead probes against a backend that was starting normally, which is the unresponsive_health_check kill the grace exists to prevent. Publish `torch_warm_in_progress` on /api/liveness and /api/health, derived from warm_status(), and read that in the launcher. A new field rather than a wider `hardware_detecting`: that marker also means "this hardware verdict is provisional, re-read it", and config/hardware-verdict.ts keeps the UI provisional and polling while it is set, so keeping it lit through datasets would hide Train for the whole warm over a verdict that settled seconds in. Additive and backwards compatible both ways. The launcher keeps its `hardware_detecting` path as a fallback, so a backend that predates the new field still gets the grace it gets today, and a backend that sends it talks to an older launcher exactly as before. The deferred case is preserved by construction: the field is published only while a warm thread is actually running, so it is absent both when the warm finished and when none is coming. UNSLOTH_STUDIO_DISABLE_TORCH_WARM=1 never starts one, and a warm retired mid stage by a shutdown never sets finished; deriving the field from "not finished" would have reported either as warming forever and held the grace open until it expired on its own. * Studio: poll the hardware verdict out of its unknown state on every platform Train and Video now spin instead of graying out while the verdict is unmeasured, so something has to end the spin. fetchDeviceType spends its bounded wait at most once per page load, so a host that detects slower than that keeps the provisional reply and `fetched` stays false. The sidebar's recovery poll returned early unless the host was chat-only or deferred, which off macOS it is not: the store seeds chatOnly from the user agent, so on Linux and Windows nothing re-read /api/health. A cold GPU host importing torch sits squarely in that window, and there the rows spun and /studio held its loading panel until the user navigated or reloaded. Poll while the verdict is unknown too, on any platform, and keep the mlx_unavailable and deferred cases as they were. The poll re-reads with force, which is what gets past the cached provisional reply and the spent wait latch, and the effect re-runs when the verdict lands, so the interval is cleared as soon as it is known. Re-reads are skipped while one is outstanding, bounded so a request that never settles cannot hold the poll off. studio-page and video-page gate on the same store value, and the sidebar is mounted on both routes, so they recover with it rather than growing a second poll. Covered by tests/hardware-verdict-recovery.test.ts, which drives the real store through a slow non-Mac detection and asserts the guard arms on the unknown state. * Studio: teach the run-module test stub about the new loggers export run.py imports install_uvicorn_duplicate_exception_filter at module scope, and load_studio_run_module replaces 'loggers' with a stub module carrying only get_logger, so importing run raised ImportError before any test body ran: ImportError: cannot import name 'install_uvicorn_duplicate_exception_filter' from 'loggers' (unknown location) A no-op is the right stub here. The filter only de-duplicates uvicorn's copy of a traceback and this module never starts a server. * Studio: run the tab-capability smoke in the macOS UI job Codex was right that the script was dead code: nothing under .github invoked playwright_mac_tab_capabilities.py, so the regression coverage it claims never executed and the tab blackout could come back unnoticed. It gets its own boot on a cold port. The assertion is that Train and Video spin rather than grey out while the verdict is unmeasured, and that window only exists on a backend that has not warmed yet, so reusing the already-warm 18897 server would have passed vacuously. For the same reason this phase deliberately does not wait for a healthy backend first, unlike every other phase here: waiting is how you miss the window. The script does its own wait, and health answers provisionally inside a 1s budget. The liveness poll is cut from its 330s default to 120s here. The full window exists to outlive the launcher's 300s startup grace, and nothing in this job runs that watchdog; it boots 'unsloth studio' directly. Spending five macOS-runner minutes to prove something this job cannot observe is not worth it, and the watchdog is covered by the Rust tests in commands.rs. * Studio: let /video reach its own gate, and stop stale polls freeing the guard Two review findings on this branch. /video was still outside CHAT_ONLY_ALLOWED, so on a measured chat-only host, a CPU-only box or a Mac without usable MLX, a direct link or a reload bounced to /chat before VideoPage could render. That is exactly where the unsupported explanation this branch added has something to say, so the message was unreachable in the only cases it was written for. Video now follows /export: the route is allowed through and the page self-gates on the backend's video verdict, so nothing loads on a host that cannot run it. The recovery poll's no-stacking guard could be freed by a read that no longer held it. A read outliving the 30s stall window is abandoned and the next tick starts a replacement, but the abandoned read's finally still zeroed the shared marker, so every following tick saw a free guard and fired another forced /api/health. On the backend this poll exists for, one still importing torch, that is a read every three seconds piled onto the process being waited for. A generation counter now means only the owning read can clear it. Both regressions verified to fail without the fix: 3 of 754 tests go red. * Studio: make the tab-capability smoke fail instead of passing vacuously The first staging run went green having tested nothing. The log tells the story: [mac-tabs] spinner observed during warm: {'train': False, 'video': False} [mac-tabs] Train: redirected to http://127.0.0.1:8888/login ... (allowed) [mac-tabs] Train: nav row not pinned inline; reached by route instead [mac-tabs] PASS The login form takes a username as well as a password and the script filled only the password, so the submit was a no-op and the browser stayed signed out. Every later check then read an empty shell: no sidebar, so no nav row, so no assertion had anything to act on. The redirect check called the bounce to /login 'allowed' because it only asked whether the verdict was measured, and the row checks downgraded a total miss to an info line. Three changes, all so this run would have gone red: log_in fills the username, then proves the session took by navigating to /chat and checking it stayed; a failure there aborts rather than continuing into checks that cannot mean anything. A bounce to /login, /onboarding or /change-password during the tab walk is a failure. The session was proven live before the walk started, so losing it mid-walk is never an allowed redirect. A walk that never locates a single nav row now fails. That is the shape a signed-out or unrendered run takes, and it has to be loud. The survival poll was the one part that did mean something: 65 samples on each of /api/liveness and /api/health, zero non-200, backend alive past 319s. * Studio: make RAG unavailability one coherent state across the router Degrading the KB list to an empty response when sqlite-vec's native library cannot load stopped the 500-plus-traceback on every poll, but it left the rest of the router behind: the frontend read the empty list as a working empty state, offered Create, and the POST went straight to get_connection() and raised. So the fix for the log spam reintroduced the log spam one click later, and told the user nothing. One contract now. The polled KB list still degrades, but carries ragAvailable and ragUnavailableReason so a client can tell "no knowledge bases yet" from "RAG cannot run on this machine". Every other endpoint answers 503 stating the same reason, via a rag_available() gate up front and a _rag_connection() wrapper that catches the case where the first request of a session is the one that discovers the library is missing. That wrapper also reaches the connections ingestion opens for itself, and removes an upload it had already saved rather than orphaning it in the uploads root. The two new fields are additive and the document listings are deliberately left erroring rather than degrading, so a frontend that has not learned to read the marker keeps every error surface it has today. rag_available() remembers only that the extension loaded, never that it failed, so a one-off cannot latch RAG off until restart. The warn-once stays: a 503 path that logged per request would be the same regression in a new place. Genuine database errors are untouched and still surface as real errors. reconcile_orphaned_ingestion_jobs() gates on rag_available() too, so it is the no-op its docstring already claimed instead of raising out of startup to be logged as a reconcile failure. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: let the UI read the RAG availability the backend already reports routes/rag.py answers a host where sqlite-vec will not load as a contract: the polled KB list degrades to 200 with ragAvailable/ragUnavailableReason beside an empty list, and every other endpoint answers 503 with the same reason. The client dropped both, so a broken-RAG Mac still showed an apparently working, empty Knowledge bases page whose Create button could only 503. features/rag/api/rag-availability holds the verdict, modelled on the platform store in config/env: optimistic until the backend has actually answered, so a slow first poll, an unreachable server and a backend that predates the marker all render exactly as they do today. Only a measured unavailable gates anything. Wired in at every response path, including the two that bypass ragRequest (ragUpload, streamJobEvents), so a user who lands on a mutating route first gets a coherent UI instead of waiting for the list poll. listKnowledgeBases reads the marker, which is the only way to tell an empty store from a host where RAG cannot run. The dialog then disables New knowledge base and Create/Save, guards submitForm for the keyboard path, and shows the backend's reason in place of "No knowledge bases yet.". Also breaks a hang this condition triggers. targetHasIndexingDocuments answers "still indexing" whenever its listThreadDocuments probe throws, and dispatchQueuedPrompt reschedules on that with no cap, so on a broken-RAG host a queued prompt on a thread using documents was never dispatched at all. A 503 is now distinguishable, and there are no documents to wait for, so it sends. A transient failure still holds the prompt back as before. This is deliberately not folded into useRagToolDisabled: that is a model capability gate and is false when no model is loaded. * Studio: report video as macOS-unsupported on Intel Macs too video_capability() keyed the macOS branch on is_apple_silicon(), so an Intel Mac fell through to pytorch_not_installed or no_accelerator and was told to install PyTorch or add a GPU. Neither enables video: the diffusers pipelines have no supported macOS path at all, so both Macs get the same honest answer. The accompanying AST test also asserted is_apple_silicon() was named in the function, which kept passing on the word surviving in a comment. Strip comments before asserting so it tests the gate rather than the prose. * Studio: only the backend's own 503 detail is a RAG capability verdict A 503 is what a reverse proxy, Cloudflare or a briefly overloaded server returns, and those bodies say nothing about sqlite-vec. Recording one as unavailable gated the Knowledge bases dialog for the rest of the session behind a transient outage, explaining it with an extension failure that never happened -- and only a 2xx from a gated endpoint could clear it. Match the RAG router's own wording instead. A bodyless or unrelated 503 now leaves availability unknown, which is what the store was built to represent. * Studio: fix the macOS tab smoke signing in, which made every assertion vacuous The helper read the password field with count() straight after domcontentloaded. auth-form.tsx returns null while the auth-status request is in flight, so there is no form in the DOM yet, count() does not wait, and the run fell through to 'assuming desktop auth' and checked an empty signed-out shell. It also filled a username the login form does not have and clicked a button labelled 'Sign in' when the label is 'Login'. Wait for #password, click Login, and wait for the post-auth route. Verified against a live Studio: the helper now reports 'signed in' where it previously did not. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make the tab-capability smoke create the window it asserts on Second hole of the same shape in this file. assert_row_never_greyed_while_unmeasured computed seen_spinner and only logged it, so when the verdict settled before the browser arrived the loop broke on its first iteration, nothing was sampled, and the run went green having observed nothing. Requiring the window would not have fixed it, because the window is not there to be required. hardware_detecting covers stage 0 of the warm, and on the macOS runner's --no-torch install with no MLX that stage is a failed `import torch` plus one failed metadata lookup: it settles inside a second of the port binding, while Playwright is still launching Chromium. Add the login and two navigations, one of which spends the frontend's own 5s wait on the verdict, and the sampler arrives 15 to 20s late on every host, fast or slow. The workflow comment claiming a cold boot keeps the window open was wrong; it is corrected to say what boot ordering actually buys, which is a provisional reply to the first /api/health probe and nothing past it. So the script opens its own window instead of racing that one. It answers the browser's /api/health with a real reply that has the measurement taken back out, holds it there, and requires nav-row-train to render enabled with data-spinner="true", the way pending beats disabled in resolveNavRowState. That window is open for as long as the check needs, on any host, and a missing row fails rather than skips, so there is no path through it that reports success without having read the row. The real warm is still sampled and a real grey-out still fails, but nothing is required of it. Two more things in here could observe nothing: The Video half of this file was structurally empty. Video is not pinned inline (SIDEBAR_NAV_DEFAULT_PINNED), so it renders inside the More dropdown, which mounts nothing until it is opened and carries no data-testid even then. Every query for nav-row-video returned null on every host, so seen_spinner["video"] could never be true and "Video: nav row not pinned inline" was a permanent info line, not a finding. Train carries the same pending flag through the same resolver, so it is the observable end of that wire; the rows the sidebar does pin are now named, an inline row that does not render is a failure, and the More rows are documented as expected misses. _saw_any_row was satisfied by any row on any route, and the sampler swallowed a dead page with a bare `except Exception: break`. Now the walk requires every default-pinned row to have been seen, and a page that cannot be evaluated at all fails. tests/studio/test_mac_tab_capability_warm_window.py drives all of this with the page and the backend stubbed, so the red cases are checked in the tests/ walk rather than only on a macOS runner. Against the previous file, its first case reproduces the staging log exactly: "spinner observed during warm: {'train': False, 'video': False}", then PASS. * Studio: give an adopted backend the startup grace when it is still warming The watchdog already knows that one healthy answer is proof of life and not proof that startup is over, but only on the path where this app spawned the backend. An adopted backend starts with the latch already set, on the reasoning that it was serving before the app attached. That is the same fallacy: a force-quit during a cold start leaves the backend running and still importing the ML stack, and the relaunched app adopts it. Three GIL-stalled probes later it was cleared and the user got a crash screen for a host that was starting normally. The ownership probe carries no warm-up signal, so ask the backend directly once ownership is verified, and clear the latch on a warming reply rather than merely declining to set it. The grace stays bounded at 300s either way. This path predates the fix on the owned side; it is the other half of the same report rather than a regression. * Studio: fix five guards in this PR's own tests that could not fail Each was verified by mutation: the regression the test names was applied, the suite stayed green, and it now goes red. - provisional-hardware-verdict: the slice end was located by searching for the latch it then asserted was absent. String.search returns the first match, so moving the latch into the loop moved the boundary with it and the assertion passed against exactly the regression its header describes. Anchor on the loop's closing brace, and require the latch to exist after it so deleting it outright is not a pass either. - liveness warm state: the subprocess harness builds its result with body.get(studio_root_id), so the key is present whether or not the reply carried it. Deleting the field from liveness_check() left the test green. Report presence explicitly, as the two neighbouring fields already do. - warm window, DEVICE guard: asserted a count of at least two when the function has three comparisons, so deleting the poll loop's requirement still left two. Bind to the sites instead of counting them. - warm window, boot silence: scanned only for _fetch_top_models, but the fetch is started through the public wrapper now, so putting _start_top_models_fetch back in the constructor restored the huggingface.co call on every boot undetected. Check both names. - rag availability: asserted only after noteRagAvailability, which writes unavailable unconditionally, so the exemption under test could be deleted. Assert between the two calls. Also replaces a dead 'disabled: chatOnly;' string check: that is an object entry, so the regressed form ends in a comma and the literal could never match. * Studio: deliver the hardware verdict to a component that subscribed a tick late useHardwareInfo seeds its state from the module cache during render, but joins the listener set in the effect. A probe resolving between those two points notifies the listeners registered at the time, which does not include this one, and leaves the cache set, so 'if (!cached) load()' skipped the fetch as redundant. Nothing remained that would ever call setInfo, and the component sat on the unloaded default for its whole life. That was survivable while callers read individual fields. This PR gates whole pages on 'loaded', so it now reads as 'Checking this machine for video support' for the rest of the session, which is the stuck-loading state the PR exists to remove. Hand the cache straight to the listener instead. Also stops a successful 200 that a later invalidate superseded from resolving as the unloaded default, which load() reads as a failed probe. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: complete the forced password change in the macOS tab smoke The repo's own macOS smoke hands this script the raw bootstrap password, and a backend that still holds one injects it into the page and signs itself in, landing on /change-password with no login form ever rendered. This script treats that as a signed-out route, since it has no sidebar to assert against, so the run failed with 'could not sign in'. The staging harness rotates over the API before driving, which is why the same script passed there and failed here. Rotate it in the browser instead of giving up, and check for that screen before waiting on the login field so an authenticated session does not spend a minute waiting for a form that is correctly absent. Verified against a live Studio on both paths: a fresh instance holding its bootstrap password now reports 'password rotated' then 'signed in' where it previously reported 'could not sign in', and an already-rotated instance still signs in through the login form. * Studio: give the adopted ownership probe the watchdog's probe budget The warm-up read added for adopted backends is gated on ownership verifying, and that probe runs every request at the 2s default. During the multi-second GIL stall the watchdog exists to ride out, both requests inside it time out, the backend comes back unverified, and the warm-up read never runs at all, so the grace never reopens and three stalls still clear the backend. The longer budget has to be applied before verification, not after it. probe_owned_backend_state keeps its signature and delegates to a variant taking an explicit budget, so only the watchdog changes. A guard binds that call site to HEALTH_PROBE_TIMEOUT and fails if it drifts back to the default. Also uploads the tab-capability phase's log and Playwright evidence, which the artifact list did not name, so a red macOS run discarded the only record of it. * Studio: match the RAG 503 on the extension name only Matching either fragment meant the loose half carried the same weight as the specific one: anything RAG-aware in front of the backend can answer a transient 503 saying RAG is unavailable without meaning the extension, and that persisted a capability verdict which only a later successful gated request could clear. Keep sqlite-vec and drop the English phrase. Nothing upstream emits a package name by accident, and the capability being gated is exactly that extension, so this is narrower than requiring both fragments would be while still tolerating a reworded backend detail. Requiring both would have made the matcher brittle to precisely the rewording the fragment match exists for. Two proxy phrasings added to the generic-503 case, which restoring the loose marker now fails. * Studio: stop the MLX install env claiming a recovery it does not perform _mlx_install_env's docstring still described VIRTUAL_ENV as the second half of a dangling-symlink recovery, and named the helper that performed the first half. That helper was deleted earlier in this PR when the retry was found not to work, but the claim outlived it and is false: an explicit --python outranks VIRTUAL_ENV, so uv reports the same unresolved-interpreter error either way. It is not a harmless stale comment. It is the tree asserting a repair that does not happen, and a reviewer reading it asked for the mechanism to be restored. State what uv actually does, and why --target and --prefix are not the escape hatch they look like: both exit 0 against a broken venv but resolve against whatever ambient interpreter uv finds, writing a wrong-ABI or off-sys.path install that leaves mlx_stack_available() False while looking like success. Two guards: the deleted helper must stay deleted and the claim must not come back, and the unresolved-interpreter path must stay one attempt plus a diagnosis, never a second install with a different target. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make the watchdog-budget guard line-ending agnostic include_str! embeds the file exactly as checked out, so on Windows the source is CRLF and the \n}\n search for the end of check_watchdog_health never matched. The guard panicked on the Tauri CI runner while passing on every Linux and macOS job. Normalise before searching. Verified both ways: 149 tests pass on the LF tree, and the guard still passes after converting the file to CRLF, which reproduced the runner failure before this. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
2d231f8845
|
test: isolate the no-version gfx-override test from a real /opt/rocm (#7397)
--------- Co-authored-by: LeoBorcherding <borchborchmail@gmail.com> |
||
Tai An
|
0c12d6473f
|
fix(studio/rocm): don't install for a shadowing iGPU on mixed AMD hosts (#7776) (#7778)
* fix(studio/rocm): don't install for a shadowing iGPU on mixed AMD hosts (#7776) On a board with both an AMD APU and a discrete Radeon, HIP enumerates the iGPU first, so _detect_windows_gfx_arch picked index 0 and the installer pulled the iGPU's wheel family -- a gfx1036 Raphael iGPU shadowing a gfx1200 RX 9060 XT, leaving the discrete card unused until the reporter set HIP_VISIBLE_DEVICES=1 by hand. When no visible-device mask is pinned and more than one distinct arch is enumerated, skip a leading shadowing APU arch so the discrete card decides the wheel family, and print which GPU was chosen plus the HIP_VISIBLE_DEVICES override. An explicit HIP_VISIBLE_DEVICES / ROCR_VISIBLE_DEVICES value still wins verbatim. The Strix arches (gfx1150/1151/1152) are deliberately excluded from the skip set: they are first-class unified-memory training targets, so their selection is unchanged. Signed-off-by: Tai An <antai12232931@outlook.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix(studio/rocm): mirror the dGPU repick in setup.ps1 and honour CUDA_VISIBLE_DEVICES Addresses both review findings on #7778. 1. setup.ps1 resolved the gfx arch itself (hipinfo and amd-smi paths) and built $ROCmIndexUrl from it *before* invoking install_python_stack.py, and _ensure_rocm_torch() returns early once UNSLOTH_ROCM_TORCH_INSTALLED=1 -- so a fresh Windows install on a gfx1036 + gfx1200 host still received gfx103X-all wheels and never reached the Python-side repick. Resolve-ShadowingGfxPick mirrors _dedup_pick() and is applied at both PowerShell pick sites. 2. HIP honours CUDA_VISIBLE_DEVICES with the same semantics as its own masks -- _pick_rocm_gfx_target in install_llama_prebuilt.py already resolves all three identically -- so a ROCm install launched with only CUDA_VISIBLE_DEVICES set was treated as unpinned and could be overridden by the iGPU skip. It now counts as a pin on both sides ("" / "-1" still mean "no mask"). Tests: CUDA_VISIBLE_DEVICES pin + empty-is-not-a-pin cases, a setup.ps1 <-> Python parity check on the shadowing-arch list (the list now exists in two places), and the pre-existing shadowing tests now clear CUDA_VISIBLE_DEVICES so CI runners that export it cannot flip the assertions. Signed-off-by: Tai An <antai12232931@outlook.com> * fix(studio/rocm): keep a supported APU over a discrete card with no Windows wheels The shadowing-iGPU preference returned the first non-integrated arch in the enumeration regardless of whether AMD ships Windows wheels for it. On an unpinned gfx1036 + gfx1010 host that deposed a supported Raphael APU for a discrete card absent from _GFX_TO_AMD_INDEX_ARCH, so _windows_rocm_index_url resolved to None and the install fell back to CPU -- strictly worse than the shadowing the preference exists to undo. Only prefer the discrete arch when it actually has an index, unless the integrated pick has none either, in which case the swap costs nothing and the discrete card still wins. Both directions are covered: gfx1036+gfx1010 keeps the APU (fails without this change, returning gfx1010), gfx1013+gfx1010 still yields to the discrete card so the guard is not over-tightened. Signed-off-by: Tai An <antai12232931@outlook.com> * fix(studio/rocm): close two setup.ps1 gaps in the shadowing-iGPU preference Both halves of the #7776 preference existed in install_python_stack.py but only half of it in the PowerShell mirror, which resolves the arch and builds $ROCmIndexUrl itself before the Python installer ever runs. - Resolve-ShadowingGfxPick deposed a supported APU for any discrete arch, even one AMD ships no Windows wheels for (gfx1036 + an older gfx1010): the repick resolved to no index at all and dropped the host to CPU, strictly worse than the shadowing it undoes. It now consults $archFamilyMap, mirroring the _pick_has_wheels guard in _dedup_pick(). The map moves to script scope so detection can read it; contents are unchanged, so the four-way parity test still sees the same 18 entries. - The WMI fallback took the first AMD adapter before name -> arch inference, so an Adrenalin-only host listing a 780M ahead of an RX 9060 XT still inferred gfx1103 and installed gfx110X-all. It now keeps every AMD adapter, infers an arch for each, and runs the same preference over the list. Regression tests fail on the previous revision and pass on this one. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix(studio/rocm): index the enumeration with CUDA_VISIBLE_DEVICES too _visible_devices_pinned() treats CUDA_VISIBLE_DEVICES as a pin, but _pick_visible_index() only read HIP/ROCR. On the probes that enumerate every GPU regardless of the masks (amd-smi, WMI), CUDA_VISIBLE_DEVICES=1 on a gfx1036 + gfx1200 host therefore suppressed the shadowing skip *and* resolved to index 0, installing the iGPU's wheels for the device the user masked away. Same mismatch in setup.ps1's $_hipVisIdx and $visGpu picks. All three masks are now read at every site, matching _pick_rocm_gfx_target in install_llama_prebuilt.py. Also corrects the _detect_windows_gfx_arch docstring, which still claimed the first GPU always wins without a mask. Signed-off-by: Tai An <antai12232931@outlook.com> * Share one mask resolver across every ROCm pick site for PR #7778 The shadowing-iGPU preference was correct, but each pick site still resolved HIP/ROCR/CUDA_VISIBLE_DEVICES with its own inline expression and they disagreed, so a mask the pin check honoured could resolve to a different GPU than the one the user asked for. On a mixed host that lands on index 0, which is the iGPU the preference exists to skip. setup.ps1 - Add Resolve-VisibleGpuIndex and use it at all four pick sites (hipinfo, amd-smi list, amd-smi static --asic, WMI name inference). Previously the hipinfo expression rejected " 1 " and the amd-smi one rejected "1,0". - The static --asic branch now collects every gfx token and runs the repick instead of taking the first regex match. - WMI inference indexes the adapter list rather than the inferred arch list, so an unrecognised name cannot shift a mask onto the wrong physical card, and it only repicks when every adapter mapped: an unknown name may itself be the discrete card. - Filter WMI adapters on ConfigManagerErrorCode so a disabled or driver-errored Radeon cannot depose a working iGPU. Get-CimInstance to match the rest of the repo. install_python_stack.py - _pick_visible_index now skips "" and "-1" and reads the next mask, matching _visible_devices_pinned. Before, HIP_VISIBLE_DEVICES="" with CUDA_VISIBLE_DEVICES=1 counted as pinned while the index resolved to GPU 0. - Out-of-range and unparseable masks warn instead of silently using GPU 0. - Strip the ":sramecc+:xnack-" suffix from gcnArchName like setup.ps1 does; a suffixed token matched neither the wheel table nor the skip set. - Prefer a wheel-backed candidate whenever one exists, not only when the picked arch has wheels, so gfx1036 + gfx1010 + gfx1200 no longer stops at gfx1010 and drops the host to CPU torch. Arch list - Drop gfx1037: it is not an AMDGPU target in LLVM, so no Windows tool emits it. - Add gfx1033 (Van Gogh) and gfx1153 (Krackan Point 2), both APUs. gfx1033 has a wheel family, so leaving it out let it act as the "discrete" card. - gfx1013 is Cyan Skillfish, not Van Gogh. Both advisories now tell the user to setx HIP_VISIBLE_DEVICES so the chosen GPU is used at runtime, not just at install time: the wheels alone do not change which device HIP enumerates first. Tests - TestSetupPs1ShadowingBehaviour actually executes Resolve-ShadowingGfxPick and Resolve-VisibleGpuIndex under pwsh, slicing them out by AST. The existing parity class only greps text, so a rename failed it while a semantic bug passed. - Regression tests for each fix above. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Warn on a pinned wheel-less GPU and silence a bogus range warning Both found by simulating the change across the OS x GPU-vendor product rather than by reading it. - _dedup_pick now says so when an honoured pin selects a GPU AMD ships no Windows wheels for while another enumerated GPU has them. The pin is still honoured verbatim, but the install drops to CPU torch and the mask is the reason, which was previously invisible. - _pick_visible_index takes warn=False for callers whose list is deduplicated. The Linux Strix reroute indexes _detect_amd_gfx_codes(), which collapses duplicates, so a dual same-arch box (two gfx1151) has a 1-element list and a perfectly valid HIP_VISIBLE_DEVICES=1 read as out of range. That printed a false "out of range" warning on a healthy Linux host. The Windows arch-selection path still warns, where the index space really is devices. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Align the visible-device masks with the ROCm runtime, and two repick fixes Three review items on the last round, all confirmed against the code at head. Mask semantics (mine to fix: 6d0ac82 got this wrong). "" and "-1" do not mean "no mask", they select no GPU at all, so falling through to the next variable was wrong. The ROCm runtime stores an explicitly empty var as " " (clr flags.cpp), then picks the HIP mask whenever its first byte is not NUL (paldevice.cpp on Windows, rocdevice.cpp on Linux), so an empty HIP_VISIBLE_DEVICES shadows CUDA_VISIBLE_DEVICES rather than deferring to it; parseRequestedDeviceList surfaces zero devices for " " and "-1", which ROCR states outright in amd_filter_device.h. _visible_devices_pinned and _pick_visible_index are now first-set-wins and treat any set value as a deliberate selection, matching _pick_rocm_gfx_target in install_llama_prebuilt.py and PyTorch's own _parse_visible_devices. Resolve-VisibleGpuIndex and Resolve-ShadowingGfxPick mirror it. Three tests asserted the old premise and now assert the runtime's. Resolve-ShadowingGfxPick did not prefer wheel-backed cards when the APU has no wheels either. The predicate went vacuously true and took the first non-integrated arch, so gfx90c,gfx1010,gfx1200 picked gfx1010, left $ROCmIndexUrl null and installed CPU torch despite the supported gfx1200. Now mirrors _dedup_pick's `_withWheels or (...)`. The Python WMI probe listed disabled adapters. setup.ps1 filters on ConfigManagerErrorCode but `(Get-CimInstance Win32_VideoController).Name` did not, so on a driver-only laptop a disabled RX 9060 could depose a working 780M and pull wheels for a GPU Windows never exposes. Same filter both sides. * Stop double-applying the mask to hipinfo, and two selection fixes hipinfo is itself a HIP application, so under a mask the runtime filtered and renumbered its device list before we ever read it. Indexing that output again applied the mask twice: with HIP_VISIBLE_DEVICES=1,0 on a gfx1036 + gfx1200 host, HIP exposes [gfx1200, gfx1036] and the second lookup landed on the iGPU, installing its wheel family for the card the mask put first. _dedup_pick now takes mask_resolved for the hipinfo probe and setup.ps1 reads $_hipAllArches[0]; amd-smi and WMI list every GPU regardless of the masks, so they keep the explicit index. The repo already stated this in _hip_visible_device_mask_set: "hipinfo, itself a HIP application, so under a mask it enumerates the VISIBLE devices, not the physical ones". The advisory hard-coded device 1. On gfx1036,gfx1010,gfx1200 the selected card is device 2, so following the message exposed the gfx1010 the installed wheels do not target. Both messages now name the selected arch's real index. The WMI path substituted another adapter's arch when the selected one had an unrecognised name. Unpinned that is the point (the #7776 iGPU has no entry in the name table, so the named discrete card should decide), but under a mask it installed wheels for a GPU the user masked away. The fallback is now gated on Test-VisibleDevicesPinned, which also replaces the inline pin loop in Resolve-ShadowingGfxPick so both sides share one definition. Two existing tests described a host that cannot exist: unfiltered hipinfo output under a mask. They now model the filtering HIP actually performs. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep setup's dGPU repick for llama.cpp, and index WMI by adapter * Reinstall Windows ROCm torch when the wheel family changes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep the WMI arch probe silent on non-AMD adapters * Read the active ROCm family from the rocm meta-package * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Resolve the visible-device mask against devices, not deduplicated arches * Harden the WMI probe and the PowerShell index parse * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Pin the platform in the two new Linux reroute tests * Pin the arch too in the new Linux reroute tests * Parse rocminfo per agent and honour ROCR filtering on Linux * Tighten comments --------- Signed-off-by: Tai An <antai12232931@outlook.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
oobabooga
|
f8730f4339
|
Installer: select CUDA wheels that cover the host's GPUs (#7814)
* Installer: select CUDA wheels that cover the host's GPUs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix Windows venv wipe and warning dedupe for PR #7814 - Windows pins torch<2.11, whose cu128 still ships sm_70, so capping a Volta to cu126 there rewrote a working family. The stale-venv check then read that as drift and deleted the venv on a direct "unsloth studio update", which cannot recreate it. Make the pre-Turing floor per-family (70 for cu128). - Repair an unpinned cu* -> cu* move in place instead of rebuilding the venv. - Decide the cu126 advice before deduping the uncovered-host warning: the host facts are release invariant but the artifact list is not, so the release walk-back let an unhelpful release swallow the remedy. - Gate the new coverage repair and the cu126 advice on x86_64, matching the cap. - Add tests/studio/test_pre_turing_cap.ps1: the parity test only greps for the call spelling, so neither PowerShell copy had behavioural coverage. * Tighten comments for PR #7814 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> |
||
|
Daniel Han
|
9b452cb3b3
|
studio: pair slim whisper bundles on the ggml tree id, not the -mix- tag suffix (#7817)
* studio: pair slim whisper bundles on the ggml tree id, not the -mix- tag suffix * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added by this PR * Backfill ggml_tree into the marker when an existing llama install is reused * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Backfill on the third reuse path, gate the tree on its binary source, guard malformed trees * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Pair only on a declared ggml tree, and refresh a tree-less install so that stays recoverable * Withdraw the forced marker migration; it broke the combined update path * Tighten the comments added across the review rounds --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Daniel Han
|
cc0185fbb1
|
Windows: guard the source-build and whisper.cpp probes on an unreadable install tree (#7757)
* Windows: guard the source-build and whisper.cpp probes on an unreadable install tree Follow-up to #7735, which routed the prebuilt llama.cpp probes through three-state path probing but left two gaps. Phase 4 read $LlamaServerBin with a bare Test-Path under "Stop". A forced compile, a pinned PR or a custom llama source skips Phase 3.4 entirely, so on those routes this was the first probe inside the tree and a denied build\ aborted with the raw "Test-Path : Access is denied" the merged PR set out to remove. It now probes three-state, and the CMakeCache.txt read below it is guarded too: a listed file can still deny the read, which the probe cannot see. The probe is skipped for a linked UNSLOTH_LOCAL_LLAMA_CPP_DIR, where it would read through the junction into the user's own checkout, and the denial reports -OwnershipUnverified under a custom home, where nothing on this route has proven the tree is ours. The whisper.cpp phase promises failure is never fatal, but under a custom UNSLOTH_STUDIO_HOME an unreadable tree exited the whole run, taking llama.cpp inference down with it. Assert-StudioOwnedOrAbsent gains a -NonFatal mode that hands the denial back instead; an unowned tree still stops. The check stays behind the installer-exists gate it used to sit inside, so a tree without install_whisper_prebuilt.py remains the no-op it was. Backend: _is_runnable let Path.is_file() propagate EACCES. Now that setup leaves a denied whisper.cpp in place, that turned into a 500 out of /api/inference/audio/stt/status, the one endpoint reporting both dictation engines, so the setup message promising Transformers dictation still works was not true. It reads as engine-unavailable instead. * Harden the denial contract tests against surviving mutations Mutation testing found six ways to reintroduce the bugs this branch fixes while the tests stayed green. Assert-StudioOwnedOrAbsent: the -NonFatal returns were counted, not ordered. Moving one below its Exit-PathAccessDenied makes it dead code and the whisper phase fatal again; hoisting one above the custom-home gate reports a fresh install as unreadable. Each return is now pinned immediately above the exit it pre-empts, with no unpaired return allowed. The whisper denial branch had no assertion scoped to its own body. Both phrases it was checked for already occur elsewhere in the phase, so the branch could be turned back into an Exit-SetupFailure and stay green. The branch is now sliced out and checked for step/Yellow, both phrases, and the absence of any exit. The installer gate was checked for presence, not for being a conjunct, so -or-joining or negating it reopened the installer-less tree the test is named for. The denial subject was unpinned, so it could name llama-server.exe and tell the user to move aside one file instead of the tree. Slice terminators are now asserted through one helper: an unasserted terminator does not fail, it silently widens the window to end-of-file and makes everything inside it near-vacuous. The whisper binary probe test gated its only behavioural case on geteuid() == 0, which silently drops it in any root container. It probes for a real denial instead. Two pre-existing exact counts in the ownership guard tests become floors: this branch consumed the last of their headroom, so the next legitimate route added there would break two tests that say nothing about it. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix the -NonFatal negative control for Windows ACL semantics The control probed a marker file that did not exist. Windows reports a missing child of a denied directory as absent rather than throwing, so the control read as "this host cannot deny" and failed the suite on windows-latest while passing under chmod on Linux. It now probes a file that exists inside the locked tree, matching the control the suite already uses. That difference also splits the routes by platform for a tree with no ownership marker, which is the fresh custom-home case: Linux catches it on the marker probe, Windows has to catch it on the adoptable-state read. Added a case that accepts either route and rejects anything but Denied, so the Windows one is exercised for the first time. * Detect a denied tree that has no ownership marker on Windows Staging CI on windows-latest caught this. Get-StudioAdoptableState decided "denied" only from probes of two marker files inside the tree, but Windows reports a MISSING child of an unreadable directory as absent rather than throwing. A denied tree holding neither marker therefore returned "No", and Assert-StudioOwnedOrAbsent fell through to "path is not an Unsloth-owned install" and exited: the wrong cause, and fatal, on the only platform any of this runs on. It also defeated the whisper -NonFatal path, since an unowned tree is still fatal by design. Listing the directory itself distinguishes "no markers here" from "cannot look", so that is the fallback when neither probe reported a denial. A readable tree with no markers still returns "No" as before, and the catch swallows anything that is not a denial because this helper must not throw. This also corrects the message a denied custom-home llama.cpp tree produced on Windows, which reported the same wrong cause. chmod 000 blocks the child probes outright, so it never reached the new code. chmod 111 allows stat of a named child while forbidding a listing, which is exactly the Windows shape, so the test now covers both and the negative control fires only on the 111 case. * Tighten comments in the denied-tree and whisper install changes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
Michael Han
|
ebfefcf84e
|
Windows: do not abort setup on an unreadable llama.cpp install (#7735)
* Windows: do not abort setup on an unreadable llama.cpp install
Test-Path raises UnauthorizedAccessException instead of returning $false
when an ACL denies the probe. setup.ps1 runs under $ErrorActionPreference
= "Stop", so the bare probe of UNSLOTH_PREBUILT_INFO.json in the llama.cpp
prebuilt phase killed setup with a raw "Test-Path : Access is denied" and
exit code 1. The desktop app had nothing but [TAURI:ERROR_DEFAULT] to fall
back on, so it showed "unsloth studio setup failed (exit code 1)".
~/.unsloth/llama.cpp sits beside the app, not inside it, so reinstalling
reused the unreadable folder and hit the same line again, including a
reinstall to a different drive.
Add Get-PathState (Present / Absent / Denied) plus Test-PathQuiet, route
the probes that read inside install trees we do not own through them, and
report a denied llama.cpp install through Exit-SetupFailure so the reason
and the recovery steps reach the desktop UI.
Reported in unsloth-test/unsloth-test#9
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Stop on every denied path, and split the recovery commands
Review follow-ups:
- Assert-StudioOwnedOrAbsent treated a denied root as absent and returned,
so the caller could go on to replace a tree it cannot read. Probe the
root three-state and stop on Denied, still gated on $StudioHomeIsCustom
so default-home behaviour is unchanged.
- The source-build .git probe treated a denied checkout as "no checkout"
and cloned a replacement. The swap that follows recursively removes the
original and moves the temp tree over it under "Continue" and unchecked,
so a denied child could leave a half-deleted install. Stop instead.
This path already treated denied as absent before the previous commit
(that probe runs under "Continue", so it printed an error and took the
false branch), so the hazard is older than this branch, but it is in
scope for the same reason.
- Probe $LlamaCppDir itself three-state, so an unreadable parent is
reported rather than dying on the bare probe under "Stop".
- takeown and icacls were printed joined by "then", which is not a
PowerShell separator: takeown would swallow the rest as arguments and
icacls would never run. Print them on separate lines.
Fold the repeated guidance into Exit-PathAccessDenied so all five denial
routes report the same thing.
* Harden the denial reporting path, found by simulation
Ran the real decision blocks against simulated filesystems (denied file,
denied parent, traverse-only and list-only dirs, symlinks, dangling links,
wildcard and unicode paths, 3000 random paths) plus PSScriptAnalyzer's
5.1/6.2/7.0 syntax check. Two things came out of it:
- Get-PathDenialDetail threw a parameter-binding exception on an empty
path. It runs while a failure is being reported, so it would have
replaced the actionable message with a raw binding error at exactly the
wrong moment. Null and empty are now accepted and return no detail.
- The link-target lookup used an empty catch, which PSScriptAnalyzer flags
and which hid the intent. It assigns $null explicitly now.
Both are covered by new checks. Also promoted the strongest invariant from
the simulation into the suite: Get-PathState must agree with a bare
Test-Path on every probe that did not throw, and Denied may only appear
where the old probe threw, so no path that worked before can take a
different branch now.
Verified: PSUseCompatibleSyntax reports nothing for 5.1, 6.2 and 7.0; the
Python contract tests pass on 3.10 through 3.13 in separate uv venvs; the
tauri install:: unit tests pass (17), which is the code that prefers the
[TAURI:ERROR] line over the generic exit-code message.
* Trigger the Windows PowerShell tests when they change
studio-windows-inference-smoke.yml runs six PowerShell unit tests out of
tests/studio, but its pull_request paths filter matched none of them, and
no other workflow runs them. A PR touching only one of those tests never
ran it. Five predate this branch; the sixth is the ACL test added here.
Scope the filter to tests/studio/*.ps1 rather than tests/studio/**, so a
python-only change under that directory does not pull in the GGUF smoke
jobs. This matches what the other two workflows already do: parity-ci
lists its .ps1 test outright and update-smoke uses a scoped glob.
Guard it in test_ci_shell_suite_coverage.py, which exists for this exact
failure (tests/sh had the same hole): every tests/*.ps1 a workflow invokes
must be matched by that workflow's paths filter, and must exist. The
GitHub glob matcher it needs has its own table-driven test, since a wrong
matcher would make the guard pass on everything.
Verified by reverting the one-line filter change: the guard then names all
six unrun tests.
* Make the Windows PowerShell test step fail when a test fails
Verifying the path-filter fix turned up a second hole in the same step. A
`shell: pwsh` step inherits only the LAST command's exit code, and this
step ran five tests as five bare commands, so only the last one could fail
the build. test_resolve_cuda_toolkit.ps1 has been printing
FAIL exits non-zero (scenario 2, forced source build)
FAIL exits non-zero (scenario 6, no toolkit, forced)
2 check(s) FAILED
on every Windows run, exiting 1, and the job reported success. Confirmed
on main (run 30723608191, sha
|
||
|
Etherl
|
1faa0377e5
|
Fix Windows llama.cpp prebuilt setup with inaccessible PATH entries (#7696)
* Fix Windows prebuilt fallback on inaccessible PATH * Fix inaccessible inherited Windows PATH entries * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix Windows PATH test on POSIX * Keep transient prebuilt failures on the source-build path Narrowing the source build to helper exit 2 made every other nonzero exit fatal, but the helper reports a rate-limited or unreachable api.github.com as EXIT_ERROR: fetch_json raises a bare RuntimeError for HTTP 403, and the fork branch of the release resolver has no wrapper of its own where the ggml-org branch does. Unauthenticated GitHub API calls are limited to 60 an hour per IP, so a shared runner or a NAT'd network hits this routinely, and a source build clones over git rather than the API, which is why falling back used to recover. This PR's own macos-14 kill@torch run failed exactly that way. Classify release-listing failures on the install path as PrebuiltFallback so they exit 2 again. Scoped to install_prebuilt rather than the resolver: --resolve-prebuilt turns PrebuiltFallback into a successful {"prebuilt_available": false} payload and update_flow caches any exit-0 answer for RESOLVE_TTL_SECONDS, so wrapping there would pin a transient 403 as "no prebuilt" for 24 hours. Also close the paths that stayed fatal: - collect_system_report ran inside the PrebuiltFallback handler, so a probe that raised replaced the in-flight fallback with EXIT_ERROR. - python_runtime_dirs and windows_runtime_dirs stat sys.path entries and %ProgramFiles% outside dedupe_existing_dirs, so skip_unusable could not protect them and a denied entry still aborted Windows discovery. - binary_env still required inherited LD_LIBRARY_PATH and DYLD_LIBRARY_PATH to be readable, the same thing the Windows branch stopped requiring. - sync_marker_force_cpu and sync_marker_llama_backend guarded the marker read but not the write, so a read-only marker on an otherwise up to date install failed setup. - runtime_libs.python_runtime_dirs, the serve-time copy, had the same unguarded sys.path stat; the sidecar turns the raise into an empty dir list and loses every CUDA wheel dir. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Only reclassify transient resolver failures, and persist the reuse marker Three follow-ups from cross-platform validation. The resolver wrapper caught Exception, so a TypeError or AttributeError from a bug in host-conditional resolver code (per-gfx ROCm, Windows arm64, the macOS walk-back) also exited 2 and bought a source build, logged under a message that blames the network. Narrow it to OSError, RuntimeError and ValueError, which covers every transient shape by MRO: URLError, HTTPError, SSLError and TimeoutError are OSError, JSONDecodeError is a ValueError, and fetch_json raises a bare RuntimeError for HTTP 403. Code defects go back to EXIT_ERROR, with a test pinning that direction. Guarding the marker write stopped a read-only marker from failing setup, but it also let a deliberate --force-cpu go unrecorded while setup reported success, which is how the updater re-routes a CPU user onto a GPU bundle (#7213). Write through atomic_write_bytes first, which swaps in a sibling temp file and so lands on a read-only marker in a writable dir, and warn loudly when it genuinely cannot be recorded. Skip the setup.sh routing test on Windows: setup.sh is the POSIX installer, Windows runs setup.ps1, and driving a POSIX script through Git Bash with Windows paths proves nothing about either. The PowerShell branch of the same routing stays covered by the platform-independent textual test. * Make Python runtime discovery optional, and never truncate the reuse marker Guarding only the search root left the strict dedupe on python_runtime_dirs' own return to raise, so a readable site-packages with a denied torch/lib or nvidia/*/lib child still aborted Windows discovery. That is the same shape as the bug this branch is about, one level down: the parent lists fine and the entry underneath is denied. These candidates are optional CUDA wheel dirs found by globbing, and one that cannot be stat'd could not have served DLLs to the loader either, so skip them like the serve-time copy already does. Drop the in-place retry in the marker rewrite. It opened a valid marker with truncation, so an ENOSPC or I/O error mid-write would strand a partial UNSLOTH_PREBUILT_INFO.json and later updates would stop recognising the install. The atomic path already covers the case the retry was there for, a read-only marker in a writable dir; when it fails, leave the old marker alone and warn. * Preserve the marker mode across the atomic refresh os.replace keeps the source file's mode and NamedTemporaryFile is 0600, so refreshing a shared install's marker left UNSLOTH_PREBUILT_INFO.json readable only by whoever ran setup, and other users could no longer recognise or update that installation. Reproduced: a 0444 marker came back 0600. Build the temp file here instead of calling atomic_write_bytes, so the original mode is restored before the swap rather than after, leaving no window where the marker is private. Clean up the temp file if the replace fails, so a failed refresh strands nothing next to the marker. * Claim only transport failures, and never strand a temp marker A plain OSError is not evidence of a network problem. EMFILE after file descriptor exhaustion, ENOMEM, or a local EACCES reading TLS configuration were all being converted to EXIT_FALLBACK, so setup started the resource heavy source build that this branch otherwise refuses for unexpected helper failures, and a build needs more descriptors and more memory, not fewer. Name the transport shapes instead: URLError covers HTTPError and the socket/DNS errors urllib wraps, plus SSLError, ConnectionError, TimeoutError, and the RuntimeError and ValueError that fetch_json raises for a 403 and for an undecodable payload. ENOSPC still reaches EXIT_NO_SPACE, now through __main__'s classifier rather than install_prebuilt's, and the test asserts it the same way __main__ decides it. The marker temp file was only unlinked when the chmod or the replace failed. A write, flush or fsync that raised, which is the ENOSPC case this is built to tolerate, jumped straight past the cleanup and left a partial UNSLOTH_PREBUILT_INFO.json.tmp-* beside the valid marker, one per attempt on a full volume. Track it across every failure path instead. Also restore the original owner and group on the replacement where the caller is permitted to, since os.replace installs the temp file's ownership and a shared marker would otherwise pick up the invoking user's primary group. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added by this change --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
Daniel Han
|
9bfa18cdb0
|
Windows: unblock the consumer install on clean and no-winget machines (#7549)
* Windows: unblock the consumer install on clean and no-winget machines Four independent things stop a clean Windows box today. git was a hard Exit-SetupFailure in setup.ps1, justified as required by pip for git+https:// deps and by npm. Neither holds on the consumer path: the unsloth-zoo git+https URL is only used under STUDIO_LOCAL_INSTALL, node is a pinned nodejs.org prebuilt that never touches system npm, and the frontend lockfile has no VCS dependencies. It stays fatal for --local, where it really is needed. Ensure-VCRedist was winget-only, so on hosts without winget (LTSC, Server, managed corporate images) it silently did nothing while the install reported success, and torch then failed to import on a missing VCRUNTIME140.dll. Adds a direct aka.ms/vs/17/release/vc_redist.<arch>.exe download with /quiet /norestart, accepting exit codes 0 and 3010. The redistributable stays required: it is the runtime the prebuilt llama-server and torch link against, not the MSVC compiler, which is already detection-only. Windows on ARM has no PyTorch at all. Measured with uv against download.pytorch.org/whl/cpu and PyPI for aarch64-pc-windows-msvc / cp313: torch, torchvision and torchaudio all resolve to nothing, wheels exist only for win_amd64 and the manylinux targets. The installer burned three uv retries on an unsatisfiable resolution and reported a bare 'Failed to install PyTorch (exit code 1)'. Now it says what is actually wrong and points at --no-torch, which works because llama.cpp does publish windows-arm64-cpu. install_node_prebuilt.py hit '[WinError 5] Access is denied' on os.replace of the freshly extracted directory during a FRESH install, which is a scanner or indexer holding handles for a moment. Retries only winerror 5, 32 and 145 with capped exponential backoff; any other OSError still raises immediately. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Give the ARM64 dead end a recovery that works for web installs The only remedy printed was .\install.ps1 --no-torch, but the documented path is irm | iex, where no file exists and flags cannot be forwarded. Name the env var the script already honours at line 145. * Windows on ARM: drop torchaudio, do not abort the install The fail-fast was based on a wrong premise. Counted against download.pytorch.org/whl/cpu: torch has 42 win_arm64 wheels and torchvision 60; only torchaudio has none. PyTorch has shipped Arm-native Windows builds since April 2025, so aborting blocked a platform that mostly works. Drop the one unsatisfiable pin instead. Decide from the interpreter uv will resolve for, not the PowerShell host: an x64 CPython under emulation gets working win_amd64 wheels on an ARM64 box, and powershell.exe inherits PROCESSOR_ARCHITECTURE from its parent. * Carry the ARM64 torchaudio omission into studio setup Dropping it from the first PyTorch command was not enough: install.ps1 then runs studio setup with SKIP_STUDIO_BASE=1 and setup.ps1 reinstalls the bare trio from the CPU index, so the ARM64 path still aborted. Apply the same interpreter-based test there. An unreadable platform keeps the full trio. * Build the torch spec list outside the verbose branch The ARM64 guard landed inside `if ($script:UnslothVerbose)`, so on the default path $_torchTrio was never assigned and the splat expanded to nothing: uv ran as `uv pip install --index-url ...` with no package, exit 2, straight to Exit-SetupFailure. That broke the ordinary Windows install. Hoist it above the branch and use substep, which prints on both paths. Realign the two parity guards to the splat form; they asserted the pre-refactor literal command and were the actual cause of the red parity legs. Both halves are still checked: the bounded list is built, and it reaches the install. * Tighten the comments on the Windows install path * Windows install: honour the ARM64 torchaudio skip everywhere and keep git for source builds Hoist the venv-interpreter platform probe above every torch branch in studio/setup.ps1 so the win_arm64 torchaudio omission applies to the ROCm, CPU and CUDA/custom paths. A pinned index whose leaf is not cpu routed an ARM64 host into the CUDA/custom branch, which still asked for torchaudio. Require git again when a llama.cpp source build is opted into up front (UNSLOTH_LLAMA_FORCE_COMPILE, UNSLOTH_LLAMA_PR / PR_FORCE, a non-upstream source). Those paths git clone in phase 4, so setup used to report git as not required, install the build toolchain, then fail at the clone. A local llama.cpp dir overrides them, and the automatic source fallback after a failed prebuilt download stays non-fatal. Also tighten the comments across the changed install paths. * Install the x64 VC++ runtime unconditionally in the direct-download fallback The winget branch always installs Microsoft.VCRedist.2015+.x64, but the direct-download fallback picked the package from PROCESSOR_ARCHITECTURE, which reports the architecture of the running PowerShell process rather than the interpreter that will load the DLLs. Find-CompatiblePython in install.ps1 selects an interpreter on version and non-Conda status alone, with no architecture predicate, so a native ARM64 shell can settle on an emulated x64 Python whose win_amd64 torch and prebuilt llama-server need the x64 runtime, while the fallback had just installed the ARM64-only package. Ensure-VCRedist also runs well before the venv exists, so the interpreter cannot be probed at that point. Microsoft ships the x64 redistributable as an Arm64X superset that carries both ARM64 and x64 binaries, so it is correct on both machines and the manual instruction printed on failure already pointed at it. * Windows on ARM: prefer an x64 Python interpreter An ARM64 host cannot complete the install with a native ARM64 interpreter. pyarrow, pulled in by unsloth -> datasets, has never published a win_arm64 wheel on any version, and neither has hf-transfer, a direct dependency. Both therefore fall back to a source build: pyarrow dies in scikit-build-core CMake configuration and hf-transfer dies in openssl-sys for want of perl, several minutes into a run that looked healthy. torch and torchvision are not the problem, they have win_arm64 wheels and install fine. Windows 11 on ARM runs x64 binaries under emulation and both packages ship win_amd64 wheels, so an x64 interpreter installs cleanly. Find-CompatiblePython accepted an interpreter on version and non-Conda status alone. It now ranks candidates by architecture on ARM64 hosts and returns an x64 one when present, asking each interpreter for its own sysconfig.get_platform() rather than guessing from its path. Host architecture comes from PROCESSOR_ARCHITEW6432 and OSArchitecture as well as PROCESSOR_ARCHITECTURE, which describes only the current process and reads AMD64 in an emulated shell. This is a preference, not a requirement. If only ARM64 is found, x64 is bootstrapped through winget --architecture x64 or the python.org fallback, and if neither works the installer names pyarrow and hf-transfer up front instead of failing later on a CMake or Rust error. The ARM64 torchaudio skip stays live for that path. Non-ARM hosts return on the first match exactly as before, with no extra interpreter probing. * Windows install: three correctness fixes on the ARM64 and git-less paths Ensure-VCRedist never reached its x64 download on an ARM64 machine that already had the arm64 redistributable: Test-VCRedistInstalled accepted System32\vcruntime140_1.dll regardless of architecture, and there that file can be the pure-ARM64 package. An ARM64 PE cannot load into an emulated x64 process, so the x64 Python this branch now prefers would have been left without a usable runtime. The x64 registry entry is the only x64-specific proof, and Microsoft registers Runtimes\{x86|x64|arm64} per architecture, so vc_redist.x64.exe still writes Runtimes\x64 on an ARM64 host and the check cannot loop. The DLL probe stays for x64 hosts. Phase 1 demanded git for any non-blank UNSLOTH_LLAMA_PR_FORCE, but the promotion that actually turns it into a source build requires a positive integer, so PR_FORCE=0 or a non-numeric value aborted a git-less consumer install for a build that never runs. Both sites now use the same predicate. The automatic fallback after a failed prebuilt llama.cpp download reached git clone with no git check anywhere in between, and Invoke-SetupCommand returns 0 for a command-not-found, so a git-less host did not stop there: it continued into an empty directory and reported a cmake configure failure instead. Git is now resolved where the source build is decided, with a last winget attempt, and a missing git degrades exactly like a missing cmake rather than aborting, since the opt-in source triggers already required git in Phase 1. Also tightened the comments across the changed Windows install code, keeping the reasons on the guards that prevent a specific failure. * Rank ARM64 Python candidates by minor version before architecture The x64 preference filtered the whole candidate list on architecture, which outranks the version preference the candidates were collected in. With UNSLOTH_PYTHON=3.12 on a Windows ARM64 box holding an ARM64 3.12 and an x64 3.13, it returned the x64 3.13: the explicit pin was silently broken, and because a x64 interpreter was found the caller never ran Install-X64Python to fetch an x64 3.12. With no pin it was worse still, since an x64 3.11 outranked a newer ARM64 3.13 and defeated the newest-first fallback. Walk $minors in order and take the x64 build of the best minor available, falling back to that minor's ARM64 build so the caller bootstraps x64 for the version actually requested. x64 still wins within a minor, and non-ARM hosts are untouched. * Windows install: see every registered Python, order git before the toolchain Find-CompatiblePython only ever probed `py -3.X`, which runs the launcher's preferred build for that minor. On an ARM64 box that is the native ARM64 interpreter, so a same-minor x64 install that is registered with the launcher but neither preferred nor on PATH never became a candidate. The x64 preference then lost to ARM64, and Install-X64Python re-downloaded an x64 CPython that was already on the machine; when that download is unavailable the install continues on ARM64 and source-builds pyarrow and hf-transfer, which publish no win_arm64 wheels. Enumerate `py -0p` on ARM64 hosts and probe each listed path. The `-3.12-64` suffix cannot be used for this: it has meant "not 32-bit" since 3.11 and does not distinguish arm64 from amd64. studio/setup.ps1 ran Ensure-BuildToolsForLlamaSourceBuild before checking git in Phase 4. That helper calls Exit-SetupFailure when Visual Studio Build Tools cannot be installed, so on a clean no-winget box the git degraded path added by this PR was unreachable and a standalone update aborted instead of finishing in limited mode; where winget does exist it spent a multi-GB Build Tools download on a clone that could never run. Check and install git first, skip the toolchain helper when git is still missing, and report the git branch before the cmake branch so the message names the real cause. _swap_into_place retried the forward rename for about 16 seconds but rolled back with a bare os.replace. A scanner holding the backup for the same WinError 5/32 then left no install_dir at all and stranded the working runtime in .old-*, and its exception replaced the original failure. The rollback now uses the same backoff and logs instead of masking the error it is recovering from. * Installer: use an already installed x64 Python on ARM64 when none can be downloaded Find-CompatiblePython ranks x64 within one minor and returns the native build when that minor is ARM64-only, leaving Install-X64Python to bootstrap x64. On an offline or winget-less box that bootstrap fails, and the retry went through the same resolver, so an x64 build of a lower-priority supported minor already on the machine was never picked up and setup continued on ARM64 Python, where pyarrow and hf-transfer have no wheels. Add an -X64Only mode that returns the best installed x64 interpreter or nothing, and call it as the last resort in Install-X64Python. The version-first preference is unchanged: x64 of the requested minor is still bootstrapped first. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten comments in the Windows ARM64 installer changes * Setup: require Git for a source build behind an unbuilt local llama.cpp dir UNSLOTH_LOCAL_LLAMA_CPP_DIR only overrides the source-build opt-ins once the directory holds a reusable llama-server.exe. Pointing it at the canonical install location with nothing built there falls through to the normal install, so the Phase 1 gate now probes the same layout candidates as the Phase 4 reuse check before dropping the requirement. * Setup: require Git when UNSLOTH_LLAMA_TAG=master forces a source build * Tighten comments in the Windows installer changes * Setup: negotiate TLS 1.2 for the direct VC++ runtime download --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothai@gmail.com> |
||
|
Leo Borcherding
|
411cb86d62
|
amd: require bitsandbytes>=0.50.0 in the amd extra (fixes ROCm 4-bit NaNs) (#7535)
* amd: require bitsandbytes>=0.50.0 in the amd extra bnb <= 0.49.2 NaNs at decode shape on every AMD GPU. The ROCm 4-bit GEMV fix (bnb PR #1887) first ships in 0.50.0, on PyPI since 2026-07-24, so the old >=0.49.1 floor could still resolve the broken range. Mirrors the same change made on the pip release branch in #7278. * amd: cite the 0.50.0 ROCm work accurately in the bnb floor comment The comment credited bnb PR #1887 as "the ROCm 4-bit GEMV fix" for every AMD GPU. #1887 decouples blocksize from warp size and fixes a hardcoded warp size of 32 in kgemm_4bit_inference_naive, which is a CDNA problem by construction. The RDNA-side work is #1979 (fused 4-bit SIMT GEMM) and #2012 (RDNA3/4 workgroup resonance). All three first ship in 0.50.0, so the >=0.50.0 floor is unchanged; only the justification was wrong. * amd: raise the installer bitsandbytes fallback floors to 0.50.0 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * amd: stop reporting the bitsandbytes PyPI fallback as broken * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten AMD bnb floor comments * Keep the amd extra citation and the AMD install guide reference * amd: do not promise aarch64 a ROCm 4-bit backend it never gets * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * amd: fall back to the PyPI bitsandbytes floor on Windows ROCm too * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
Willow Lopez
|
77971d0deb
|
fix(rocm): prefer system LLVM runtime on native Linux (#7448)
* fix(rocm): prefer system LLVM runtime on native Linux * Fix/adjust the nested LLVM probe for PR #7448: lib64 hosts and non-directories Two gaps found while simulating the fix against real ROCm layouts. 1. lib64 hosts got no LLVM dir. The candidate was built from the HSA dir, so a host with libhsa-runtime64 under lib64 probed <root>/lib64/llvm/lib. ROCm installs LLVM under <root>/lib/llvm regardless, so that host kept binding system libamd_comgr to the bundle's libLLVM: exactly the bug #7446 reports. Probe both spellings, the HSA dir's own first so a genuine lib64 layout still wins. When lib_sub is already "lib" the seen set collapses them. 2. os.path.exists accepted a non-directory. The serve-time caller joins these straight into LD_LIBRARY_PATH with no is-dir filter, so a file named llvm/lib reached the loader. os.path.isdir instead. Verified on a 27-case matrix built from real directory trees (not mocks), run on both Windows and Linux against three revisions: main, this PR as-is, and this commit. Zero regressions and zero reorderings of the pre-existing entries in every case, and the installer and launcher copies never disagree. The lib64 case goes [lib64] -> [lib64, lib/llvm/lib]; the file case drops the bogus entry; a symlinked llvm/lib resolves correctly on Linux. End-to-end loader check: built real ELF objects mirroring the shipped bundle (RUNPATH=$ORIGIN, an incomplete libLLVM.so.23.0git next to llama-server, system comgr from /opt/rocm/lib) and reproduced the reported failure verbatim, then confirmed the prepend clears it: before undefined symbol: LLVMInitializeSPIRVTarget -> after exit 0 Test helper now patches os.path.isdir alongside os.path.exists, else every fake host reports its nested llvm dir as missing. New cases: lib64 finding llvm under lib, lib64 preferring its own when both exist, and a real-filesystem check that a non-directory is not prepended. Removing the lib fallback from one copy reddens three tests including the two-copy parity guard. tests/studio/install: 1361 passed on Linux, 4 pre-existing environmental failures unchanged (3 managed-node-runtime under root, 1 the real /opt/rocm case already covered by #7397). 30/30 on the helper suite on Windows and Linux. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Lee Jackson
|
d7594ec10f
|
Fix Windows no-torch setup (#7511)
* Fix Windows no-torch setup * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix no-torch env normalization on Windows * Accept on for Windows no-torch mode * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep no-torch mode across studio update on Windows Guarding the direct torch/Triton install made `install.ps1 --no-torch` actually produce a torch-free venv, which then broke the next `unsloth studio update`. That path exports no UNSLOTH_NO_TORCH, so $NoTorchMode was false, the stale-venv check read the missing torch as a broken venv, and setup tried to delete the venv it was running out of: [ERROR] Could not remove stale venv: Access to the path 'python.exe' is denied. That teardown can never succeed there, because setup.ps1 runs via unsloth.exe out of that same venv. The same gap also let the shared dependency pass reinstall torch from PyPI, unpinned, into a GGUF-only environment. install_python_stack.py now records the mode in the install manifest and setup.ps1 reads it back when no env var is exported, then re-exports a canonical value for the dependency pass (setup.ps1 drops the manifest before invoking it, so the child cannot repeat the lookup). The key is additive and MANIFEST_SCHEMA is unchanged, so existing manifests stay valid and a missing key keeps today's behaviour. Also: - read_manifest() caught only OSError, but UnicodeDecodeError is a ValueError. That is now on the installer's import path, so a manifest re-saved as ANSI or truncated mid-write would abort every install. - The env predicate now trims surrounding whitespace, matching the Python side. - The Windows update smoke workflow asserts the update leaves the venv GGUF-only, which is what would have caught this. Known follow-up, pre-existing: an install killed between the manifest drop and the dependency pass leaves no recorded mode, so a later update still walks the stale-venv path. Closing that needs a marker the installer never drops. * Persist no-torch mode in a marker the dependency pass cannot drop The install manifest alone was not enough. Both setup.ps1 and install_python_stack.py remove it before every dependency pass, and it is only rewritten on success, so a no-torch install interrupted in between left nothing recording the mode. The next update then resolved no-torch as false, read the expected missing torch as a stale venv, and tried to delete the environment whose python.exe was running it, which leaves the install unrepairable from the CLI. Add .unsloth-no-torch next to the existing .unsloth-studio-owned marker, written before the pass and cleared when torch is wanted. setup.ps1 writes it as soon as the mode resolves, so the window between the manifest drop and its own torch install is covered too. Read order stays manifest key first, then marker, so migrating out of no-torch is never blocked by a marker an earlier run left behind. Neither present still reads as "install torch", so nothing changes for installs made before either existed. Also adds the AGPL-3.0 header the new test file was missing. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothai@gmail.com> |
||
|
Daniel Han
|
1781770bee
|
Studio: detect an interrupted dependency install instead of launching a backend that cannot import (#7492)
Some checks are pending
Unsloth GGUF CI / JSON, images (push) Waiting to run
Mac Studio UI CI / Chat UI Tests (push) Waiting to run
Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Windows Unsloth API CI / Unsloth API & Auth Tests (push) Waiting to run
Windows Unsloth GGUF CI / OpenAI, Anthropic API tests (push) Waiting to run
Windows Unsloth GGUF CI / Tool calling Tests (push) Waiting to run
Windows Unsloth GGUF CI / JSON, images (push) Waiting to run
Windows Unsloth GGUF CI / Unsloth install + inference without Visual Studio (push) Waiting to run
Windows Unsloth GGUF CI / GPU prebuilt resolves without Visual Studio (push) Waiting to run
Unsloth GGUF CI / Tool calling Tests (push) Waiting to run
Mac Studio API CI / Unsloth API & Auth Tests (push) Waiting to run
Mac Studio GGUF CI / OpenAI, Anthropic API tests (push) Waiting to run
Mac Studio GGUF CI / Tool calling Tests (push) Waiting to run
Mac Studio GGUF CI / JSON, images (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-14) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-15) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-26) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-15-intel) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-26-intel) (push) Waiting to run
Mac Studio Update CI / Unsloth Updating Tests (push) Waiting to run
Unsloth Tauri CI / Tauri Linux debug build (no codesign) (push) Waiting to run
Unsloth UI CI / Chat UI Tests (push) Waiting to run
Windows Unsloth GGUF CI / VC++ runtime detect + install round-trip (windows-latest) (push) Waiting to run
Windows Unsloth UI CI / Chat UI Tests (push) Waiting to run
Windows Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Wheel CI / Wheel build + content sanity + import smoke (push) Waiting to run
Windows Unsloth GGUF CI / setup.ps1 unit tests (VS 2026 / CMake guard) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2022) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2026) (push) Waiting to run
Windows Unsloth GGUF CI / VC++ runtime detect + install round-trip (windows-2025-vs2026) (push) Waiting to run
* Studio: detect an interrupted dependency install instead of launching a backend that cannot import An installer killed part-way leaves a venv with a working CLI but without studio.txt's dependencies. Nothing recorded that, so three separate places all reported it healthy: - the desktop preflight probed only `unsloth -h` (typer + rich) and a hardcoded desktop-capabilities dict, neither of which touches studio.backend, so it returned ManagedReady and spawned a backend that died on `import structlog`; - setup.sh's fast path compared the installed unsloth version against PyPI, which matches on a half-built venv because unsloth is installed early, so `unsloth studio update` printed "up to date" and repaired nothing; - start_managed_repair calls that update and then re-checks with the same blind probes, so Repair reported success without fixing anything. install_python_stack.py now clears a completion manifest before the dependency pass and writes it only after the final step. `unsloth studio verify-install` and desktop-capabilities' new studio_install_ok field read it, the preflight turns a false answer into ManagedStale so auto-repair runs, and setup.sh / setup.ps1 gain an escape hatch next to the existing anyio one. Separately, the wheel ships studio/ and studio.backend* but declared none of their dependencies, so `unsloth train`, `export`, `chat`, `inference` and `studio` all ended in a rich traceback after a plain pip install. structlog is the only hard module-level import that chain reaches once starlette's annotation-only import moves under TYPE_CHECKING, so it becomes a core dependency and the rest of the server stack becomes a [studio] extra mirroring studio.txt. The CLI import sites now report missing dependencies as a sentence with two remedies. Fixes #4701, #5260, #7147 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Match the trimmed comments merged on the pip branch * Put the install manifest in the preflight fingerprint for PR #7492 The capability cache keyed the venv on pyvenv.cfg, uv.lock, requirements.txt, the interpreter and site-packages/unsloth_cli/commands/studio.py, none of which a repair touches when it only reinstalls studio.txt. So an entry cached while the install was healthy stayed valid after the manifest was dropped, and the probe returned Ready on exactly the half-built venv this is meant to catch. * Address the review findings on PR #7492 Fail the install when the completion manifest cannot be written, instead of exiting 0 without the record every later check requires, which is a repair loop by construction. Compare the version of the package the manifest names, so `studio update --package X` does not read as a permanent version change. Read the manifest from the venv that owns it when the CLI runs outside the managed venv, and drop the dependency verdict in that case: the walk ran against the wrong interpreter and says nothing about that venv. Name the import that actually failed. `unsloth train` reaches torch through the same guard, and the studio extra does not carry it, so recommending that extra alone left the command failing in the same place. * Declare click, which typer stopped providing, for PR #7492 unsloth_cli/commands/start.py imports click at module scope and unsloth_cli/__init__.py imports that module, so every unsloth command needs it. typer carried click through 0.19 and dropped it in 0.27, and the declared floor is typer>=0.12.0, so a fresh resolve gets no click. On the published wheel it still arrives because huggingface_hub requires click<9,>=8.4.2, which is luck rather than a declaration. A wheel built from this branch's dependency list has neither, and every command dies at import. Verified: before, `unsloth --help` on a fresh venv raised ModuleNotFoundError for click; after, it exits 0. The drift test now covers it. * Keep a running backend from the previous app version manageable The manageability bump gated two unrelated things through one constant. For the managed CLI probe 2 is right: a CLI reporting 1 cannot answer studio_install_ok. For a RUNNING backend it is wrong, because a process already started cannot change what it reports, so bumping studio/backend/main.py in lockstep does not help one the previous app version spawned. That backend is proven ours by root id and ownership token, but lifecycle_control_block_reason returned Unmanageable, and that branch never calls adopt_verified_backend. has_owned_backend() stays false, so Repair falls into block_external_conflict, which finds the same process and refuses: the app could no longer stop a backend it owns the token for. The same regression in backend.rs turned a terminal-launched same-root server from AttachedReady into ExternalConflict. Split the constant: DESKTOP_BACKEND_MANAGEABILITY_VERSION = 1 for the two live-backend probes, DESKTOP_MANAGEABILITY_VERSION = 2 for the CLI probe. Every real gate (protocol, auth, ownership, desktop-login, MIN_DESKTOP_BACKEND_VERSION) is untouched, so an old backend still reaches OwnedStale, adopt, stop, repair. Also stop the installer when the stale manifest cannot be removed. Windows raises on a read-only or locked file, and the pass would then run behind a marker that still names this version and these digests, so a run killed part-way would verify as complete. * Answer for the managed venv, not the one the CLI happens to run in The guard matched ModuleNotFoundError.name, an import name, against missing_requirements(), which returns distribution names. So a missing PyJWT printed 'pip install jwt', and jwt, docx and fitz are each a real but unrelated PyPI project (fitz is a neuroimaging workflow tool), so following the advice installed the wrong package and left the backend just as broken. Map the import to its distribution before deciding, and never offer the import itself. install_state() verified the caller's own prefix. The wheel ships studio/, so a CLI installed outside the managed venv always finds its own copy of the helper first, and a healthy managed install reported studio_install_incomplete with a missing list copied from the wrong venv. Selecting the root is not enough: _installed_version() reads the running interpreter and req_root defaults to the caller's studio.txt, so both checks still answered for the wrong venv. Hand verify_install() that venv's own metadata, enumerated through Distribution.discover(context = ...path), which does not fall back to sys.path. The candidate order is untouched, so shadowed-tree detection is unchanged. setup.ps1 replaces pip, torch and triton before install_python_stack.py runs, so the manifest it drops is not dropped before the first mutation. A run killed in between kept a marker that still verifies while torch was half-replaced; drop it at the top of the dependency pass instead. setup.sh is unaffected, the stack is the first thing its pass runs, and a test now pins both. pip uninstall rewrites nothing that was fingerprinted, and cache_matches re-reads the cached studio_install_ok rather than re-checking, so a venv that lost a studio.txt package kept being served the healthy verdict. Fold a sorted hash of the installed dist-info names into the marker hash. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * A missing manifest helper is a torn install, not an old one studio/install_manifest.py ships in the same wheel as _studio_deps.py, so nothing legitimately has one without the other: a CLI predating both never reaches this code, and the desktop already calls such a CLI stale on desktop_manageability_version. Returning ok=true there reported a healthy install for a tree the package update had half replaced, and the preflight then launched a backend whose own run.py could be just as absent. Report it incomplete so repair runs. * Tighten comments across the install-detection changes * Validate Studio dependency readiness --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> Co-authored-by: Wasim Yousef Said <wasimysdev@gmail.com> |
||
Souravrajvi0
|
7917c7828c
|
Installer: opt-in Vulkan llama.cpp backend (and fallback when no AMD card is HIP-supported) (#7373)
Some checks are pending
Unsloth GGUF CI / JSON, images (push) Waiting to run
Unsloth load-orchestrator CI / test (push) Waiting to run
Mac Studio API CI / Unsloth API & Auth Tests (push) Waiting to run
Mac Studio GGUF CI / OpenAI, Anthropic API tests (push) Waiting to run
Mac Studio GGUF CI / Tool calling Tests (push) Waiting to run
Mac Studio GGUF CI / JSON, images (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-14) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-15) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-26) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-15-intel) (push) Waiting to run
Mac Studio Install Matrix CI / Install + load (macos-26-intel) (push) Waiting to run
Mac Studio UI CI / Chat UI Tests (push) Waiting to run
Mac Studio Update CI / Unsloth Updating Tests (push) Waiting to run
Unsloth Tauri CI / Tauri Linux debug build (no codesign) (push) Waiting to run
Unsloth UI CI / Chat UI Tests (push) Waiting to run
Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Windows Unsloth API CI / Unsloth API & Auth Tests (push) Waiting to run
Windows Unsloth GGUF CI / OpenAI, Anthropic API tests (push) Waiting to run
Windows Unsloth GGUF CI / Tool calling Tests (push) Waiting to run
Windows Unsloth GGUF CI / JSON, images (push) Waiting to run
Windows Unsloth GGUF CI / Unsloth install + inference without Visual Studio (push) Waiting to run
Windows Unsloth GGUF CI / GPU prebuilt resolves without Visual Studio (push) Waiting to run
Windows Unsloth GGUF CI / setup.ps1 unit tests (VS 2026 / CMake guard) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2022) (push) Waiting to run
Windows Unsloth GGUF CI / real-VS detection (VS 2026) (push) Waiting to run
Windows Unsloth GGUF CI / VC++ runtime detect + install round-trip (windows-2025-vs2026) (push) Waiting to run
Windows Unsloth GGUF CI / VC++ runtime detect + install round-trip (windows-latest) (push) Waiting to run
Windows Unsloth UI CI / Chat UI Tests (push) Waiting to run
Windows Unsloth Update CI / Unsloth Updating Tests (push) Waiting to run
Wheel CI / Wheel build + content sanity + import smoke (push) Waiting to run
* feat(install): opt-in Vulkan llama.cpp backend and HIP gfx fallback (#7357) Add UNSLOTH_LLAMA_BACKEND=vulkan and --llama-backend vulkan to force the upstream Vulkan prebuilt on any host, persist llama_backend in the install marker, and re-assert it during Studio updates. On Windows AMD, auto-fallback to Vulkan when no detected gfx arch is in the upstream win-hip-radeon GPU_TARGETS set (e.g. gfx803 / RX 480). Mixed setups where at least one card is HIP-supported still default to HIP unless opted in. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix(install): address Codex P2s on Vulkan gfx routing (#7357) Honor ROCm family tokens (gfx110X), include fork-supported gfx1103, require a known active gfx before auto-Vulkan, and base the HIP floor check on the visible-device target instead of every physical GPU in hipinfo. * Address Codex review: env namespace, physical-NVIDIA guard, test kwarg - llama_backend_from_env: stop reading UNSLOTH_LLAMA_CPP_BACKEND. That is a separate pre-existing setup variable meaning auto/cpu; setup.sh/setup.ps1 warn and ignore other values, so reading it here forced Vulkan behind that warning. Vulkan opt-in stays on UNSLOTH_LLAMA_BACKEND / UNSLOTH_FORCE_VULKAN. - _should_auto_vulkan_for_amd_windows: gate on not has_physical_nvidia (not merely has_usable_nvidia). A CUDA-masked NVIDIA card keeps has_physical_nvidia while has_usable_nvidia goes False; Vulkan ignores CUDA_VISIBLE_DEVICES and could enumerate the reserved card. Mirrors the Intel auto path. Explicit opt-in still overrides. - test fakes: validate_prebuilt_attempts/validate_prebuilt_choice gained a llama_backend kwarg; the four fake signatures in the fallback tests now accept it, clearing the TypeError that reddened Backend CI / Repo tests (CPU). Tests: UNSLOTH_LLAMA_CPP_BACKEND=vulkan no longer triggers Vulkan; hidden physical NVIDIA suppresses AMD auto-Vulkan while explicit opt-in overrides. * Keep gfx1034 on the ROCm path (fork gfx103X bundle covers it) The WINDOWS_HIP_PREBUILT_GFX_TARGETS allow-list omitted gfx1034, so _route_to_vulkan_prebuilt downgraded RX 6500/6400-class hosts to the upstream Vulkan prebuilt before published_rocm_choice_for_host could match the fork windows-rocm gfx103X bundle (whose members include gfx1034). Add gfx1034 to the allow-list and a regression test asserting it stays on the fork ROCm asset. * Fix auto-Vulkan stealing fork windows-rocm gfx908/gfx90a hosts for PR #7373 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix Vulkan marker claiming a backend that was never installed for PR #7373 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the Vulkan backend routing comments for PR #7373 * Keep the visible-device-aware gfx when setup forwards --rocm-gfx setup.ps1 resolves the gfx arch from its own probe, and that pick is not fully visible-device aware: neither the hipinfo nor the amd-smi branch reads CUDA_VISIBLE_DEVICES, and the amd-smi branch matches a bare integer only, so a comma-separated HIP/ROCR mask such as 1,0 also falls back to GPU 0. The resulting arch was then forwarded through --rocm-gfx and replaced the arch detect_host() had already resolved for the runtime-visible GPU. On a mixed-AMD Windows host that flipped the auto-Vulkan decision: with GPU 0 gfx1100 and a masked-in gfx1010, the forward reinstated gfx1100, _should_auto_vulkan_for_amd_windows() saw a HIP-supported arch and the HIP bundle was installed for a GPU that cannot run it. Fold the forward in as a fill rather than a replacement: it still supplies the arch on amd-smi-only, driver-only and name-inferred hosts where the probe reports none, which is what --rocm-gfx exists for, but no longer overwrites a successfully detected active arch. An explicit UNSLOTH_ROCM_GFX_ARCH stays authoritative, since it is the documented manual override for hosts whose arch the probes get wrong. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Scope the Windows AMD Vulkan fallback per device and per repo Three follow-ups on the auto-Vulkan routing for #7357. Keep an explicit --rocm-gfx authoritative. The previous round stopped a forwarded gfx from replacing an arch detect_host() had already resolved, but --rocm-gfx is also the documented operator override for hosts whose probe is wrong or stale, and both arrive as the same argv. Narrow the advisory case to the two shapes setup can actually be describing: an arch the probe saw on this host (setup picked a different physical GPU of the same box), or a family label such as gfx110X, which is a bundle name the update path derives from the marker asset rather than a real GPU arch. Any other value is an override for an arch no probe reported and stays authoritative. Keeping family labels advisory also preserves the rule that an in-generation-but-unbuilt arch (gfx1033) is never upgraded into the gfx103X bundle. Do not auto-route to Vulkan from a HIP-only device mask. HIP_VISIBLE_DEVICES, ROCR_VISIBLE_DEVICES and CUDA_VISIBLE_DEVICES select the active arch, but the Vulkan runtime honours none of them: it enumerates through GGML_VK_VISIBLE_DEVICES and Vulkan ordinals in LlamaCppBackend._get_gpu_free_memory_vulkan. Masking down to a below-floor card therefore used to install a backend that could still enumerate the HIP-capable card the user deliberately hid, possibly one reserved for another workload. Require every physical AMD gfx to be below the floor, matching the has_physical_nvidia gate right above it. So the per-GPU list survives to that check, a forward that agrees with the probe no longer collapses rocm_gfx_targets to a single entry. Make the HIP support predicate repository-specific. The floor constant is a union of ggml-org's windows-hip gpu_targets and the fork's windows-rocm bundles, so it only answers "is this arch served" for the fork. With --published-repo ggml-org/llama.cpp, direct_upstream_release_plan() offers win-hip-radeon then CPU and never Vulkan, so the four fork-only archs (gfx908, gfx90a, gfx1034, gfx1103) were declared supported and fell through to CPU instead of the Vulkan bundle that would actually run. Add UPSTREAM_WINDOWS_HIP_GFX_TARGETS and select the set from the planned repo. * Keep probe-confirmed AMD GPUs in the physical list when a gfx is forwarded rocm_gfx_targets is the physical inventory _should_auto_vulkan_for_amd_windows() reads, so a forwarded --rocm-gfx that the probe never reported was deleting cards the probe had confirmed. On a mixed Windows AMD box whose active device is masked down to a below-floor card, a stale UNSLOTH_ROCM_GFX_ARCH or a name-inferred arch for the other GPU collapsed the list to that one arch, the floor check concluded no AMD GPU on the host reaches the Windows HIP prebuilt, and the install auto-fell back to Vulkan, which honours no HIP mask and would enumerate the reserved HIP-capable card. Add the forwarded arch to the list instead of replacing it: it selects the HIP target, it does not redefine what hardware is present. An empty probe still yields a single-entry list, so the driver-only Windows AMD host the forward exists for keeps its automatic Vulkan fallback, and an explicit --llama-backend vulkan is unaffected. * Do not auto-fall back to Vulkan when a HIP device mask filtered the probe hipinfo is itself a HIP application, and AMD documents HIP_VISIBLE_DEVICES as "only devices whose index is present in the sequence are visible to HIP", with that spelling recommended on Windows. Under a mask the Windows probe therefore enumerates the visible devices, so rocm_gfx_targets is what survived the mask rather than the physical inventory the auto-Vulkan floor check assumes. A masked-out gfx1100 next to a visible gfx803 made the check conclude that no AMD GPU on the box reaches the Windows HIP prebuilt and route the install to Vulkan, which honours none of these masks and would enumerate the reserved card. Decline to guess when a mask is set: the physical inventory is unknowable from a masked probe, so keep the HIP / fork / source path. This only ever turns the automatic fallback off, never on. The driver-only single-GPU host the fallback exists for sets no mask, an all-hiding "" / -1 mask is still handled as no active target rather than a partial view, and an explicit --llama-backend vulkan or UNSLOTH_LLAMA_BACKEND=vulkan is unaffected. Reading the physical inventory through an unmasked re-probe would also correct _pick_rocm_gfx_target, which indexes the token list by the mask value and so already assumes an unmasked probe. That is pre-existing behaviour on main and is left alone here. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Treat an all-hiding HIP device mask as suppressing the Vulkan fallback too The mask guard exempted an empty or -1 value on the grounds that the probe reports no active target under it, but that only holds for the probe: a forwarded --rocm-gfx still reconstructs an active arch, and setup infers that arch from the display-adapter name, which no HIP mask touches. A user who hid every AMD GPU from HIP could therefore still be auto-routed to Vulkan, which honours none of these masks and would then use all of them. That is the strongest form of the hazard the guard exists for, not an exemption from it. Presence of any of the three variables is now the whole test, which also removes the value parsing. An explicit --llama-backend vulkan or UNSLOTH_LLAMA_BACKEND is still unaffected. * Grant the fork-only Windows HIP coverage to the fork, not to every mirror The floor set is a union of the fork's windows-rocm bundles and only the fork is planned from its manifest: resolve_simple_install_release_plans() compares == DEFAULT_PUBLISHED_REPO and sends every other --published-repo through direct_upstream_release_plan(), whose AMD branch offers win-hip-radeon then CPU and never Vulkan. Exempting only the exact ggml-org spelling therefore told a mirror carrying upstream-standard assets that fork-only archs such as gfx1034, gfx1103 and gfx908 were HIP-served, landing them on HIP or CPU instead of the Vulkan bundle that would actually run. Gate on the fork instead. Matching the dispatch exactly, spelling included, also fixes a differently cased repo: that really does take the upstream path, so it must be answered with upstream coverage rather than the fork superset. An empty repo still defaults to the fork, as the resolver does. * Derive the Windows HIP gfx floor guard from the published manifest The guard compared WINDOWS_HIP_PREBUILT_GFX_TARGETS against a second hardcoded tuple in the same test file, so a windows-rocm arch newly published by the fork passed both. Affected hosts would then be routed off the hash-approved fork ROCm bundle onto an unhashed upstream Vulkan build with nothing failing. Read the fork's llama-prebuilt-manifest.json through the installer's own resolver instead, and assert the floor, the family labels, and the routing tuple all still cover what it publishes. The manifest ships only as a release asset, so an unreachable release skips with an explicit reason rather than flaking. Both literals match the manifest as published today. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Compress the Vulkan backend routing comments and docstrings for PR #7373 * Correct the family-label rationale in the Windows HIP coverage check The comment justified serving gfx103X / gfx110X against any repository by claiming upstream's windows-hip targets build every member of those families. The fork manifest maps gfx103X to gfx1030..1032 plus gfx1034 and gfx110X to gfx1100..1102 plus gfx1103, and UPSTREAM_WINDOWS_HIP_GFX_TARGETS carries neither gfx1034 nor gfx1103, so the stated reason is wrong even though the answer is right. State the real reason instead. A family label is a bundle name, not an arch, so the concrete GPU is unknown at this point; answering unsupported to cover the two uncovered members would move gfx1030..1032 and gfx1100..1102 off a working HIP build onto Vulkan for a card the label cannot identify. Those two archs still reach Vulkan through the concrete-arch branch below, which does answer per repository. Comment only. No behaviour change: the 5850-combination override sweep still reports 0 rocm_gfx_target changes, 0 auto_vulkan False to True flips and 680 True to False flips all backed by a probe-confirmed HIP GPU, and both the feature and override profile matrices are byte-identical. * Pin that a deliberate CPU install outranks Vulkan for PR #7373 UNSLOTH_LLAMA_CPP_BACKEND (setup.sh / setup.ps1, "auto" or "cpu") and UNSLOTH_LLAMA_BACKEND (this module, a backend name) are separate variables at separate layers, and both accept "cpu". setup translates its own =cpu into --force-cpu, which is what pins the CPU-only bundle on a GPU host and keeps Intel iGPU Vulkan crashes away (#7213), so no trigger this PR adds may outrank it. _route_to_vulkan_prebuilt already gets this right, since force_cpu short-circuits ahead of the forced, auto-Intel and auto-no-HIP triggers. Cover it so it stays that way: the matrix runs [Linux, Windows, macOS] x [NVIDIA, AMD, Intel, CPU only] x [unset, vulkan, hip, rocm, cpu] with the legacy UNSLOTH_FORCE_VULKAN set as well, and asserts the published bundle survives every one. WSL presents as Linux to this resolver, so it rides the Linux row. Also assert the guard is not vacuous: the same host still takes Vulkan once the CPU pin is gone, so the matrix cannot pass on a resolver that had simply stopped routing to Vulkan. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: LeoBorcherding <borchborchmail@gmail.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
Leo Borcherding
|
f03e669442
|
AMD: enable ROCm torch on gfx906 (MI50 / Radeon VII) on Linux (#7354)
* Add community-maintained legacy support path for gfx906 (MI50 / Radeon VII) rocm6.4+/7.x torch wheels bundle ROCm libraries whose Tensile kernels dropped gfx906 (rocBLAS 'TensileLibrary.dat ... not read for gfx906', ROCm/TheRock#1844), so on MI50/Vega 20 hosts with newer ROCm the installer picked wheels that fail at the first BLAS call. The rocm6.3 index is the last one whose wheels run on gfx906 (torch 2.7.0 verified on MI50 32GB, up to 2.9 in community use). Dynamo/Inductor codegen is also broken on this arch, crashing compiled graphs that train fine in eager mode. - install.sh: when the runtime GPU is gfx906 and the picked index is newer than rocm6.3, reroute torch to the rocm6.3 index and reset the constraint trio to the default <2.11 window (a rocm7.2 pick raises the floor to 2.11, which rocm6.3 cannot satisfy), with a legacy-path warning. - install_python_stack.py: mirror the reroute in _ensure_rocm_torch using the _default pkg specs, including repairing an existing +rocm7.x torch and leaving a working rocm6.3 install alone. - device_type.py: default TORCHDYNAMO_DISABLE / TORCH_COMPILE_DISABLE / UNSLOTH_COMPILE_DISABLE on gfx906 (setdefault, user override wins). Windows allowlists are untouched: repo.amd.com publishes no gfx906 wheel family (verified in the RDNA2 enablement PR). 16-bit LoRA and full finetuning work out of the box; 4-bit QLoRA needs a source-built bitsandbytes for gfx906. Based on the verified MI50 32GB setup in namnguyen0503/mi50-gfx906-unsloth-bnb4bit-lab. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * gfx906: second Codex pass (bnb skip under pin, override beats Strix) - Compute the gfx906 runtime-target flag independently of any torch-index pin or Strix override, so the bitsandbytes skip still applies when a user pins the ROCm index and sets UNSLOTH_ROCM_GFX_ARCH=gfx906 (the pin suppresses the torch reroute, not the bnb skip). Probe only when no pin is set (an explicit pin means don't second-guess it, matching the Strix path's asserted no-probe invariant); an explicit gfx906 override needs no probe. - Let UNSLOTH_ROCM_GFX_ARCH=gfx906 suppress the Strix reroute (both install.sh and install_python_stack.py) so a mixed Strix + MI50 host routes to rocm6.3 instead of the gfx1151 wheels probe order would pick. - Fix test_hardcoded_torch_constraint: the default <2.11 window literal now legitimately appears on two TORCH_CONSTRAINT= assignments (default + the gfx906 reroute reset after the rocm7.2 floor bump); assert it only ever appears on assignment lines, never on a pip install line (its real intent). New tests: bnb skipped under an explicit pin, gfx906 override wins over Strix, install.sh suppresses Strix on the override. rocm_support + selection + cross-platform parity: 667 passed; structural constraint 9/9. * gfx906: collapse single-line asserts to match pre-commit formatting * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * gfx906: keep bnb skip + rocm6.3 routing correct under pins and suffixed overrides Address the four Codex P2 findings on #7354: - bnb skip under a pinned index (install.sh + install_python_stack.py): a real gfx906 host that pins UNSLOTH_TORCH_INDEX_URL to rocm6.3 without also setting UNSLOTH_ROCM_GFX_ARCH no longer reinstalls the generic bitsandbytes wheel over a source-built gfx906 bnb. A pin now suppresses only the torch reroute, not the gfx906 detection used for the bnb skip (Python drops the pin gate on _runtime_is_gfx906; bash _is_gfx906_bnb_skip probes via _probe_amd_gfx_arch when the index is pinned). - clear the Radeon marketing-name flag for every gfx906 target, not only when the >=6.4 reroute fires, so a Radeon VII already on rocm6.3 does not divert to the repo.radeon.com branch (whose wheels lack gfx906 kernels). - normalize a copied HIP gcnArchName (gfx906:sramecc-:xnack- -> gfx906) before the exact comparisons in install.sh and install_python_stack.py, mirroring device_type.py. Tests: relax the three Strix-pin tests (the gfx probe may now run for the bnb flag but must not reroute the pinned index) and add coverage for the pinned bnb skip, the suffixed override, and the bash Radeon-clear / pinned-probe paths. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * gfx906: log skipped vLLM aimv2 fix + robust source-scan test bounds Follow-up review polish: - import_fixes: log at info level when the vLLM aimv2 fix is skipped because the dist metadata is unreadable, so the skip is diagnosable instead of silent. - test_rocm_support: bound the gfx906 install.sh source-scan on the ';;' that closes its case arm via a shared _gfx906_reroute_block helper, replacing the brittle fixed-length (3200/3800) slices that shift when the block grows. * gfx906: trim whitespace on UNSLOTH_ROCM_GFX_ARCH in install.sh (py parity) The bash gfx906 comparisons lowercased and stripped the gfx906:… feature suffix but not surrounding whitespace, while the Python paths do .strip(). A stray newline (e.g. export UNSLOTH_ROCM_GFX_ARCH=$(cmd)) would make bash miss gfx906 while Python catches it. Trim with `tr -d '[:space:]'` at both comparison sites so the reroute target and bnb-skip agree across bash/Python. * gfx906: remove generic bitsandbytes pulled in transitively after the skip --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothai@gmail.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> |
||
|
Daniel Han
|
ef97f3c961
|
tests: record the gfx1152 llama.cpp bundle gap so the next one is not silent (#7491)
* tests: record the gfx1152 llama.cpp bundle gap so the next one is not silent #7431 made gfx1152 (Krackan Point, Radeon 860M/840M) a first-class arch, which fixed torch wheel selection: those laptops were pulling gfx1150 wheels built for a different LLVM target. It also changed llama.cpp prebuilt selection, because no gfx1152 bundle is published. published_rocm_choice_for_host deliberately refuses to serve a sibling-family bundle, so those hosts now fall back to a HIP source build. That is the right outcome, a wrong-ISA binary fails at the first BLAS call rather than merely installing slowly, but nothing recorded it and nothing would have caught it. TestPublishedRocmGfxSelection builds its release from a hardcoded family list, so it can only assert about arches someone already thought to add. Adds TestPublishedRocmBundleCoverage: - PUBLISHED mirrors the mapped_targets in llama-prebuilt-manifest.json. - KNOWN_GAPS lists arches _GFX_TO_AMD_INDEX_ARCH routes torch for that no bundle covers: gfx1033/1035/1036 (RDNA 2, never built) and gfx1152. - test_known_gaps_fall_back_to_source_build pins each to None. - test_every_torch_routed_arch_is_covered_or_a_known_gap compares the routed set against bundle coverage, so adding an arch for torch without a bundle has to be a deliberate KNOWN_GAPS entry. The invariant fires both ways. Simulating a new routed arch fails with "coverage drifted: ['gfx1153'] newly uncovered"; simulating a published gfx1152 bundle fails with "gfx1152 is in KNOWN_GAPS but a bundle now matches it; drop it from the set", so closing the gap cannot leave the list stale. Reads _GFX_TO_AMD_INDEX_ARCH from source instead of importing install_python_stack, which this suite does not otherwise depend on. No production code changes. Install suite 1355 passed, no new failures. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Leo Borcherding
|
1dd2fc4583
|
tests: read checked-in files as UTF-8 instead of the platform default (#7438)
* tests: read checked-in files as UTF-8 instead of the platform default Path.read_text() with no encoding uses locale.getpreferredencoding(), which is UTF-8 on the Linux runners and cp1252 on a stock Windows install. Nine module-level reads of checked-in source files were relying on that default. studio/backend/routes/inference.py carries the DeepSeek tool-call token regexes, so it holds U+FF5C and U+2581. Under cp1252 that read raised UnicodeDecodeError on byte 0x81 at position 97806, and because the reads run at import time it took test_cancel_atomicity.py and test_cancel_id_wiring.py out at collection, not as failures. Green on CI, permanently broken for a Windows contributor running the suite locally. Adds a guard: at module scope there is no tmp_path fixture, so a bare read_text()/write_text()/open() there is always touching a checked-in file. That makes the rule mechanical enough to enforce with no allowlist, while staying quiet about temp-dir I/O inside test bodies where the platform default is harmless. The repo already spells this correctly in 464 other places; this only stops the stragglers coming back. * tests: cover import-time helper reads and keep the guard py3.9-safe Follows up on the Codex review: - add `from __future__ import annotations`, since `str | None` in `_offender` is evaluated at import on Python 3.9 and pyproject declares requires-python ">=3.9,<3.15". - widen the guard from module scope to import time. Class bodies and the bodies of module-level helpers called from an executing statement run during collection too, so `CODE = _extract_mixed_precision_code()` was the same hazard as an inline read. `if __name__ == "__main__":` blocks are skipped: pytest never executes them. - scan studio/backend/tests/ as well as tests/. Both trees are collected on Windows by separate CI jobs, and the offender that started this, test_tool_xml_strip.py reading routes/inference.py, lives there. Widening it surfaced seven more import-time reads of checked-in sources; all now name utf-8. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Harden the import-time encoding guard for PR #7438 Close the detector gaps raised in review, all of which I reproduced against the actual AST before changing anything. False negatives (the guard let a real hazard through): - _is_main_guard ignored the comparison operator, so if __name__ != "__main__" counted as script-only even though its body runs at import. - The else arm of a main guard was discarded with the rest of the If node. - Decorators and argument defaults on a module-level def were skipped with the body, though both are evaluated when the def executes. - Path.open() in text mode was invisible; only builtin open() was matched. - encoding = None and encoding = "locale" both re-select the platform default, but the keyword merely being present counted as pinned. False positives (the guard would have blocked a compliant contributor): - A non-literal mode fell through to the "r" default, so open(p, mode) was flagged even when mode is "rb", where adding encoding= is a ValueError and there is no edit that satisfies the rule. - Same for open(*args) and a **kwargs splat, which hide the mode and can hide an encoding. - Lambda bodies and comprehension elements were walked even though neither runs at definition. Verified: still reports the same 22 offenders on unpatched main, green on this branch and on the tree merged with latest main (557 files), and an adversarial corpus of 33 cases now scores zero false positives and zero false negatives. Also corrected two docstring claims: neither collecting job runs on Windows, and the read is governed by locale.getencoding(). * Walk eager comprehensions and treat io.open as the builtin Two regressions from the previous commit, both reproduced against the AST before changing anything. Lumping list, set and dict comprehensions in with generator expressions was wrong. Only a genexp is lazy; the other three run their element expression, their filters and their nested iterators immediately, so CONTENTS = [p.read_text() for p in PATHS] at module scope is an import-time read the guard was silently missing. Comprehensions are now walked in full and only the genexp keeps the outermost-iterable-only treatment. io was also in the not-a-path-opener list, but io.open is the builtin, with the same mode position and the same platform default. io.open(CHECKED_IN_FILE) is exactly the hazard this guard exists for, so it is matched now, with binary modes and a pinned encoding still exempt. tarfile.open and fitz.open stay exempt since neither has an encoding to name. Verified: 13 targeted cases covering all five eager comprehension forms and io.open in text, binary and pinned shapes all classify correctly; still 22 offenders on unpatched main; green on this branch and on the tree merged with latest main. * Close three more walker gaps in the import-time guard All three reproduced against the AST first. A generator expression handed straight to a call is consumed there, so DATA = "".join(p.read_text() for p in paths) runs its element at import. Only an unconsumed genexp bound to a name stays lazy, so the walker now follows the consumed ones in full and keeps the outermost-iterable-only treatment for the rest. if "__main__" == __name__ is an equivalent and accepted spelling of the main guard, but requiring __name__ on the left meant its body was treated as import-time code. That is a false positive on a block pytest never runs, so both operand orders are recognised now. The helper table was built from module-level defs only, so a def in a class body invoked while the class is constructed was never followed, contradicting the walker's stated coverage of class bodies. Helpers are now collected from the module body and from class bodies at any nesting. Verified: 15 targeted cases including all three fixes and the earlier ones still classify correctly; still 22 offenders on unpatched main; green on this branch and on the tree merged with latest main. * Handle positional read_text encodings, lazy generators and nested helpers * Guard reads reached from test bodies, unbound Path calls and __file__ paths * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Follow derived paths, skip lazy generator helpers, cover compressed openers * Guard the CLI tests, helper parameters and unbound Path arguments * Discover test roots and follow literal, in-place and tuple-derived paths * Identify module openers by import, unwrap starred paths, pin subprocess snippets * Resolve import origins, seed helper locals, follow named generators and parametrize * Scope imports lexically, list tracked test files, bind unpacked names * Resolve aliased openers, keyword-only params, destructured targets, next() * Pin the encoding on subprocess snippets, workflow lint and CLI output for PR #7438 * Harden the CLI encoding guard against detached streams for PR #7438 * Tighten the encoding guard's path and scope analysis for PR #7438 * Resolve path provenance more precisely and keep POSIX stream encodings for PR #7438 * Resolve qualified path classes and scope conditional imports for PR #7438 * Scope CLI stream setup to the entry point and align two encoding pairs for PR #7438 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
Daniel Han
|
170b412c1d
|
Fix the CPU-only ROCm routing errors and two font-scale UI flakes (#7469)
* Fix the CPU-only ROCm routing errors and two font-scale UI flakes Two unrelated causes of red CI on every PR, both reproduced before fixing. ROCm routing: 12 errors on Repo tests (CPU). The spoof reports an AMD GPU, and unsloth_zoo pulls in bitsandbytes, which picks a compute backend at import. Once torch looks like a GPU is present, bnb loads its ROCm/CUDA ops, which a CPU-only torch cannot satisfy (no libhipblas.so.2, no torch._C._cuda_getCurrentRawStream), so the child died before printing RESULT. Nothing here tests bitsandbytes, so import it first, under the honest hardware. Reproduced in a CPU-only torch venv: 11 passed with 12 errors before, 23 passed after. Still 23 passed on a CUDA build. Font-scale UI: the select-viewport step pressed ArrowDown six times behind fixed sleeps, but Radix moves focus into the listbox after the content opens, so on a loaded runner the keys landed on the trigger and nothing scrolled. Wait on the overflow and press until it moves, bounded at 40. The same fixed-sleep pattern made open_appearance miss the dialog when the shortcut fired before the app wired its handler; alternate both chords on a bounded retry and wait for the control the caller is about to drive. Both were reproduced locally by running the suite against a real Studio under full CPU load. Original: 2 of 10 passed, with the exact CI signature 'keyboard did not scroll the select viewport: 0' five times. Fixed: 10 of 10. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep the ROCm routing assertion live on Apple Silicon for PR #7469 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: danielhanchen <unslothai@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Nilay
|
ae6b96ba93
|
Studio: fail fast on out-of-disk instead of a doomed llama.cpp source build (#7420)
* guard llama.cpp prebuilt against out-of-disk instead of doomed source build * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * address review comments on out-of-disk guard * keep reusable installs and Windows parity in the out-of-disk guard * preserve the ENOSPC cause when re-raising fallback errors * catch out-of-disk before the attempt loop and accept all llama-server layouts * Fix out-of-disk detection gaps and false positives for PR #7420 Follow-ups found while testing the guard against a real ENOSPC (LD_PRELOAD shim returning errno 28 under a path prefix, real network, real release): - hydrate_source_tree retried the next mirror after an ENOSPC and only raised on the last URL. Both source fallbacks 404 for the published mix commit, so the reported cause was HTTP 404 and the run fell through to the source build exactly like before the guard. Stop at the first environment-fatal error. - The 5 GB preflight rejected hosts that install fine. A full CUDA install peaks at 0.87 GB, the largest published bundle is 0.77 GB and macOS is 0.01 GB, so at 3 GB free the install succeeded before and exited 4 after, with the source-build fallback suppressed too. It is now advisory, and a real ENOSPC still exits 4. This also drops the case where an install matching an older release plan was rejected before its reuse check. - ENOSPC raised inside shutil.copytree arrives as shutil.Error with errno None and no __cause__ or __context__, so it was never classified. That path covers the hydrated source tree, the runtime overlay and the activation fallback copy. - _causal_chain followed __context__ even when __suppress_context__ was set, so `raise ... from None` over an unrelated ENOSPC reported disk full and wrongly suppressed the source build. - TemporaryDirectory now ignores cleanup errors: an rmtree failure on the way out replaced the in-flight SystemExit and lost EXIT_NO_SPACE. - setup.sh skips the arm64 CPU last resort after exit 4; it re-ran the same disk-rejected installer and buried the hint under a second error dump. - The in-app updater turns exit 4 into a readable message instead of "installer exited 4" plus a log tail. Adds tests/studio/install/test_llama_prebuilt_no_space.py covering the classifier, the advisory warning and the exit codes. * Fix Python 3.9 breakage and Windows disk-full detection in the out-of-disk guard Found by running the guard across the whole supported interpreter range (requires-python is >=3.9,<3.15) and a spoofed [Linux, WSL, macOS, Windows] x [NVIDIA, AMD, CPU] host matrix. - TemporaryDirectory(ignore_cleanup_errors = True) is 3.10+, so the previous commit raised TypeError at install time on 3.9 and turned a working install into a hard failure. Replaced with a scratch_dir() contextmanager built on mkdtemp plus rmtree(ignore_errors = True), which behaves the same on every supported version. - getattr(exc, "winerror", None) crashed on 3.9. urllib's HTTPError is an OSError that proxies unknown attributes to a wrapped file object and raises KeyError, which getattr does not swallow, so any mirror 404 during an install would have blown up inside the classifier. Read it defensively instead. - Classify Windows disk-full by winerror as well as errno. CPython's PC/errmap.h maps ERROR_DISK_FULL (112) to ENOSPC but has no case for ERROR_HANDLE_DISK_FULL (39), which arrives as EINVAL, so a Windows os.replace() onto a full disk read as an ordinary failure and fell through to the source build. Tests cover both winerror codes, a non-disk winerror, and HTTPError alone and wrapped in a PrebuiltFallback. 116 simulation cases pass on 3.9 through 3.14. * Classify quota, flattened Windows and validate-install out-of-disk for PR #7420 - EDQUOT counts as out of space: a quota'd home has free blocks this user cannot have, so the source build is just as doomed. Reported separately so df does not mislead. Confirmed end to end with a real kernel EDQUOT: the installer went from 6 retries then a source build (exit 2) to exit 4. - Match the flattened Windows disk-full text. copytree stringifies each per-file OSError, and OSError.__str__ returns early on winerror, so the text reads [WinError 112] and never [Errno 28]. Captured on a real NTFS volume. Markers are bracketed so WinError 112 does not match WinError 1120. - --validate-install now exits 4 on a full disk. It caught PrebuiltFallback and exited 2 before the classifier ran, and setup.sh answered 2 by deleting the GPU build that had just succeeded and starting a CPU rebuild that needs more of the space that ran out. Both halves are needed: the call site only tested nonzero. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten comments in the llama.cpp out-of-disk guard --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
Leo Borcherding
|
3ea6d14c39
|
AMD: CI coverage for recent fixes, plus three wrong gfx ids (#7431)
* ROCm/AMD CI coverage: arch-table parity, native-Linux lib prepend, RDNA4 grouped_mm, discovery-based shell suite
Three merged ROCm fixes shipped without tests, and the CI wiring that
would have run them was gated on files the fixes do not touch.
Tests added (113):
tests/studio/install/test_rocm_arch_table_parity.py (27)
diffs the four duplicated gfx -> AMD pip-index tables across
install.sh, install.ps1, studio/setup.ps1 and install_python_stack.py,
plus the GPU-name -> arch tables and the torch 2.11 pin allowlist.
tests/studio/install/test_rocm_native_linux_lib_dirs.py (26)
covers #7233: system-ROCm lib dirs prepended ahead of bundled
libggml-hip, the /dev/kfd + not-WSL + libhsa gate, the opt-out env
var, root resolution order, and source parity between the two copies.
studio/backend/tests/test_grouped_mm_rdna4_fallback.py (46)
covers #7292: registration on the CUDA dispatch key, grouped and
ungrouped numerics, bias/dtype promotion, and the Linux HIP<7.13 +
RDNA4 name gate, executed from the shipped source rather than a copy.
tests/studio/test_ci_shell_suite_coverage.py (14)
fails if either shell runner goes back to a hardcoded list or skips
a file without a recorded reason.
CI wiring:
studio-backend-ci.yml: add install.sh / install.ps1 to the path filter
(the suites it runs assert against those two files, so install-only
changes -- the shape most AMD/ROCm routing fixes take -- skipped it),
and replace the 13-file hardcoded shell list with directory
discovery. That list had fallen seven files behind, including
test_strixhalo_wsl_reroute.sh, the only shell coverage of the ROCm
WSL reroute, which had never run on a PR.
tests/run_all.sh: same discovery loop so local and CI agree.
* Test review fixes: assert on outcomes, not on the code under test
Self-review of the previous commit found four tests that passed for the
wrong reason.
1. The arch-table parity test pinned expected gfx ids copied out of the
shipped tables, which enshrined three upstream inaccuracies as
correct: RX 9070 (non-XT) is gfx1201 not gfx1200, RX 7800 XT is
gfx1101 not gfx1100, and PRO V710 is gfx1101 not gfx1102 per AMD's
ROCm compatibility matrix. The expectation is now the AMD pip index
leaf -- the thing the tables exist to produce, and what a wrong
answer costs the user. The three known drifts are listed explicitly
with a test asserting they stay cosmetic, i.e. that the wrong and
right ids still map to the same wheel index. That test turns red the
day one of them starts routing users to the wrong wheel.
2. The RDNA4 device-name test extracted the regex from worker.py and
then matched with it, so it could not fail. Widening the pattern --
the dangerous edit, since it forces the slow Python mm fallback onto
RDNA3 users -- would have been silently accepted. It now reads the
live pattern and checks it against fixed cases, plus asserts the
name match stays guarded by `not _lin_arch` and that the name is
lowercased before matching.
3. The CI-coverage test matched a verbatim line of studio-backend-ci.yml,
so reindenting the step would fail the build while a real regression
to a hardcoded list could slip past a reformat. It now parses the
YAML, finds the step by name, and asserts on the glob plus the
absence of individual filenames. The path-filter test likewise reads
the parsed trigger instead of scanning raw text.
4. A set comprehension in the parity helper had a ternary whose branches
were identical.
Mutation-tested: widening the RDNA4 regex, desyncing one copy of the
name table, dropping install.sh from the path filter, and re-skipping
the ROCm WSL shell suite each fail at least two tests. Verified on
Linux (WSL Ubuntu 24.04) with CI's torch pin: 86 + 48 pass.
* Fix three wrong gfx ids in the GPU-name arch tables
The name -> gfx tables disagreed with AMD's ROCm compatibility matrix on
three entries. Corrected against the "Radeon GPU" list at
rocm.docs.amd.com/en/latest/compatibility/compatibility-matrix.html:
RX 9070, RX 9070 GRE gfx1200 -> gfx1201 (Navi 48, same die as the XT)
RX 7800 XT, RX 7700 XT gfx1100 -> gfx1101 (Navi 32, not Navi 31)
PRO W7700 gfx1100 -> gfx1101
PRO V710 gfx1102 -> gfx1101 (Navi 32, not Navi 33)
No wheel changes for anyone: gfx1200/gfx1201 both resolve to gfx120X-all
and gfx1100/gfx1101/gfx1102 all resolve to gfx110X-all, in all four copies
of the index-family map. That collapse is why the errors survived being
copied into six places -- the leaf-level tests could not see them.
It was not purely cosmetic, though. install.sh's second copy feeds
"Tip: set UNSLOTH_ROCM_GFX_ARCH=<arch>", so a 7800 XT user following the
printed advice exported gfx1100 and made a wrong id authoritative for
every later run. It would also have become a real misroute the moment AMD
split a family across index leaves, as they already do for gfx1151/gfx1150.
Fixed in all six places, which is two more than the table's own "kept in
sync with" comments claim exist:
install.sh _infer_amd_gfx_arch_from_gpu_name
install.sh case "$_gpu_disp_mkt" (banner + env tip; undocumented)
studio/setup.sh
install.ps1
studio/setup.ps1
studio/install_python_stack.py
Ordering is preserved: the gfx1102 arm still precedes gfx1101 in the shell
copies so "RX 7700S" cannot fall onto the "RX 7700" glob, and the
PowerShell copies keep the (?!S) lookahead.
Test changes:
- test_rocm_arch_table_parity.py gains _AMD_DOCUMENTED_ARCH, exact gfx
ids transcribed from AMD rather than from the tables. Agreement between
six copies proves nothing when all six were transcribed from the same
mistake, so the ground truth has to come from outside. Verified it
catches the bug: against the pre-fix tables it fails 6 tests.
- The parity check now covers all six copies. It had four; the two
install.sh copies were being treated as one, and
_WIN_GPU_NAME_ARCH_TABLE was not checked at all.
- test_rocm_support.py's TestGfxArchNameFallback pinned two of the wrong
ids as expected values; updated, and extended with a 9060 XT and a
7900 XTX case so each RDNA3/4 die is represented.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Guard against unregistered copies of the GPU-name arch table
Counting the copies by hand is what let them drift: the in-code "kept in
sync with" comments claimed four, the arch-id fix found six, and scanning
the tree turns up a seventh.
TestNoUnregisteredArchTable rediscovers the copies from the source tree
instead of trusting a hand-maintained list. A table line is one that names
a card and gives its arch; real tables score 9-17 such lines and the only
other hits in the repo are two single-line prose comments, so the
three-line threshold is not load-bearing. A companion test asserts the
scan still finds the known copies, so the heuristic cannot go blind and
pass by finding nothing.
The seventh copy is tests/_zoo_rocm_spoof.py, the fixture other ROCm tests
build their fake AMD host from. It states the mapping backwards (gfx ->
the name torch should report), which makes it an independent witness: it
had gfx1101 -> RX 7800 XT and gfx1201 -> RX 9070 XT right while all six
installer copies were wrong, and nothing compared the two. Now they are
round-tripped against each other.
RX 6700 XT is pinned as a known divergence rather than normalised. AMD's
compatibility matrix documents no consumer RX 6000 card and no gfx1031 at
all, the installer arm is commented "gfx103X family", and gfx1031 appears
only as an index-family key, never as a value a name table emits. With no
external source to correct against, changing shipped behaviour would be
guesswork. A test fails if the divergence ever disappears, so the
exemption cannot go stale.
Also adds the reverse of the AMD-matrix check: a documented card that
matches no arm anywhere is a silent CPU fallback rather than a wrong id.
This cannot detect hardware nobody transcribed, which would need a live
fetch of AMD's matrix and a non-hermetic suite; the docstring says so
rather than implying coverage that is not there.
Verified on Linux: 478 passed, plus all five new guards mutation-tested
to confirm each fails when its invariant is broken.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Docstring said six copies; the list under it now has seven
* tests: run discovered shell tests with bash, not sh
tests/run_all.sh discovered tests/sh/ instead of listing files, but still
invoked each one with sh. Every file there declares a bash shebang, and on
Debian/Ubuntu /bin/sh is dash: test_apt_distro_prompt.sh,
test_studio_home_node_dir.sh and test_with_llama_cpp_dir_link_behavior.sh
fail on bashisms under dash and pass under bash. The old hand-written list
happened to name only dash-clean files, so switching to discovery is what
surfaced it. Backend CI already used bash, so this was a local-only break.
Guarded by a new test asserting both runners invoke tests/sh/ with bash.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Fix Krackan Point (Radeon 860M/840M) routed to the gfx1150 wheel index
The GPU-name tables map 860M/840M and the Ryzen AI 7 350 / AI 5 340 CPU
strings to gfx1150, but Krackan Point is gfx1152. AMD's own lemonade table
(src/cpp/server/system_info.cpp) maps both Krackan iGPUs to gfx1152.
Unlike the three ids already fixed here, this one is not wheel-neutral:
repo.amd.com publishes gfx1150 and gfx1152 as separate index leaves with
separately built torch wheels, so these laptops were installing wheels
built for a different LLVM target. gfx1152 was absent from the codebase
entirely, so it needed the index-family maps, the torch 2.11 floor lists
(same _grouped_mm bug as gfx1150/1151), the Strix reroute set and the
Windows arch allowlist as well as the seven name tables.
The parity test added in this PR did not catch it because its AMD-matrix
expectations stopped at 890M/880M. Added the APU rows, so the case that
actually changes a wheel is now covered: reverting the tables fails 9
tests naming 860M, 840M and Krackan.
gfx1153 (Ryzen AI 5 430 era) is left alone; AMD publishes no gfx1153
wheel family, so there is nothing to route it to.
Verified: bash -n on both shell installers, PowerShell AST parse on both
.ps1 files, python ast.parse on all touched modules, install suite 1334
passed with no new failures against main, shell suite 20 files.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Add gfx1152 to unified-memory classifiers, make parity allowlist set-based
Krackan Point (gfx1152, Radeon 860M/840M) is the third RDNA 3.5 APU and
shares one GPU/system-RAM pool exactly like Strix Point (gfx1150) and
Strix Halo (gfx1151), but only the installers knew about it. The two
runtime classifiers still had two-element arch sets, so a Krackan laptop
got the 0.90 discrete headroom factor on a shared pool and ran llama.cpp
without GGML_CUDA_ENABLE_UNIFIED_MEMORY.
- worker.py _rocm_classify_unified_memory: add gfx1152 to the arch set,
and 860m/840m to the device-name fallback. The NVIDIA GeForce 840M
cannot collide there: the function is only reached under _hw.IS_ROCM.
- llama_cpp.py _amd_apu_wants_unified_memory: add gfx1152 to the arch set.
- Tests for both, including the :sramecc-:xnack- suffix form.
TestGfx211AllowlistParity compared four hardcoded allowlist strings, so
adding gfx1152 to all four installers correctly turned three assertions
red without any installer actually disagreeing with another. Each test
now extracts the set its installer holds and compares it to one EXPECTED
constant. Order and spacing are free, membership is not, and the next
leaf is a one-line edit instead of four.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Daniel Han <danielhanchen@gmail.com>
|
||
|
Daniel Han
|
6f60bf4f82
|
Studio whisper: pair slim bundles on the ggml commit, not the full llama tag (#7381)
The slim whisper bundle is ggml-less and links the ggml runtime out of the installed llama.cpp prebuilt, so each whisper release pins a paired llama tag. The gate required an exact tag match, but llama fork tags are b<upstream_build>-mix-<ggml_commit> and the build number tracks upstream llama and fork PRs that live outside ggml. When llama republishes a newer build with the same ggml commit (a frequent event), the installed llama advances past the whisper pin and curated dictation goes unavailable until whisper is republished, even though the ggml runtime is ABI-identical. Key the pairing gate on the ggml commit after -mix- instead of the full tag, in all three comparison sites (slim_pairing_for_artifact, _slim_release_incompatibility, resolve_selection). requires_ggml_sonames stays the real per-file ABI gate, and a genuine ggml skew still fails closed. Tags without a -mix- marker fall back to exact matching. |