mirror of
https://github.com/unslothai/unsloth.git
synced 2026-11-13 18:04:10 +00:00
4 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
50b9deacb0
|
Trim comments in tests (#13844)
Comment and docstring changes only, in test files, split out of #12968 so the test-only part can merge first. |
||
|
|
e2104761c5
|
Windows installer: always build Studio on a uv-managed Python, never one already installed (#13399)
* Windows installer: always build Studio on a uv-managed Python, never one already installed A fresh install on a machine with Python 3.12 took that interpreter, so the cu130 torch 2.13 route (cp313 only) closed and the box got torch 2.11 with no prebuilt flash-attn / causal-conv1d / mamba-ssm. uv only provided Python when nothing 3.11-3.13 was installed, and Windows on ARM never used it. The environment is now always built on a uv-managed CPython, requested by minor and architecture (cpython-3.13-windows-x86_64-none, or aarch64 for the native Windows on ARM CUDA stack and UNSLOTH_ALLOW_ARM64_PYTHON), resolved after uv is installed and before the existing environment is touched. A reinstall keeps the environment's minor so its cached wheels are reused; UNSLOTH_TORCH_UPGRADE=1 moves it to 3.13. When uv cannot provide one, an earlier uv-installed runtime is used, else the installer stops with the mirror / pre-install fix; there is no system fallback. --python X.Y[.Z] now works as the header always said, and an explicit request is exact. Legacy layouts on a non-uv base are rebuilt instead of moved in. * Tests: drop the system-Python bootstrap cases; winget now only installs uv * Legacy environments are reused only on the Python this install resolved A legacy .venv / ~/unsloth_studio on a uv-managed base was moved in and kept even when --python, UNSLOTH_PYTHON or UNSLOTH_TORCH_UPGRADE asked for another version. It is now reused only when its pyvenv.cfg version matches the resolved interpreter's minor (its exact patch when one was requested). * Trim comments * Windows installer: legacy layouts keep their minor and arch, UTF-8 pyvenv.cfg, older cached patch past a skip - A legacy .venv / ~/unsloth_studio reinstall keeps its Python minor, as the current layout does. - A legacy environment is reused only on a uv build of the selected architecture. - pyvenv.cfg is read as UTF-8 (uv writes it without a BOM; Windows PowerShell 5.1 read it as ANSI and rejected a non-ASCII store path). - Past a skipped patch the resolver tries the next one, then an older one, so an offline machine with 3.13.7 cached still installs. * Trim comments |
||
|
|
62923f2b15
|
Studio installers: add --no-rollback, and report a full disk as a full disk (#11364)
* Studio installers: let a reinstall opt out of the rollback copy, and say what it costs A reinstall or Repair renames the live venv aside, builds a new one beside it, and deletes the old one only on success. uv hardlinks a wheel only within one filesystem, so when its cache is on another volume every file is a real copy and the install needs room for two whole environments. Measured on a 200 MB environment: same volume 0 MB extra, other volume 200 MB extra. A full disk then surfaces as nothing but 'studio setup failed (exit code 1)'. Add --no-rollback and UNSLOTH_INSTALL_NO_ROLLBACK=1 to both installers, which delete the old environment right after renaming it aside. The rename still happens first, because uv creates only into a path that is absent or empty. The restore state is cleared before the delete, as the commit path already does, so an interrupt is never handed a half-deleted backup; a failed install then cannot be undone, which is the trade the flag makes. Warn before the move when the volume has less free space than the existing environment occupies, naming both figures and the opt-out, and warn once after the cache is selected when that cache is on another volume. Both warn and continue: the estimate is a guess and being wrong must not stop an install that would have fitted. On failure, ask the volume directly and say the disk is full rather than leaving it at an exit code. The cache ranking itself is unchanged. It is pinned across install.sh, install.ps1, studio/setup.sh and unsloth_cli/commands/studio.py by tests/python/test_uv_cache_selector_agreement.py, and both installers lift the selector out and run it alone in their own tests, so the new warnings live at the call site rather than inside it. Reported in #11313. * Never let the disk diagnostics abort the install Enumerating the new predicates over their full input space turned up two gaps on the Windows side. The cross-volume cache notice was the only one of the new diagnostics not wrapped. It runs under the installer's "Stop", so anything escaping Test-StudioSameVolume would abort the install at the point the cache is selected, which is the same defect class this branch's sibling work fixes elsewhere: a diagnostic must never cost the operation it describes. The three helpers also threw ParameterBindingValidationException on an empty path, because a mandatory [string] rejects one. No caller can reach that today, since each guards its argument, but a helper whose whole job is to answer "I cannot tell" should not have a degenerate input that throws instead. They now accept an empty string and answer the quiet way: no size, no free space, and same volume, so nothing is printed. Rollback lifecycle suites still pass on both platforms, 63 shell checks and the full PowerShell set. * Do not let --no-rollback send the ARM64 migration after a deleted tree The Windows on ARM migration decides what to do from the rollback state: if a rollback is active the previous environment has already been moved aside and only needs marking as kept, otherwise it calls Start-StudioVenvRollback to move it now. --no-rollback left that state merely inactive, which the second arm reads as "not moved aside yet". It then tried to rename a directory this flag had already deleted, Move-Item threw, and the whole migration went out through Exit-InstallFailure -- after the environment was already gone. A user on ARM64 running a reinstall with --no-rollback lost their environment and got a failed install rather than the x64 rebuild. Track discarded separately from never-started, and give the migration its own arm for it: there is nothing to move and nothing to keep, so say that the previous environment was discarded on request and build fresh, instead of promising an unsloth_studio.arm64.* copy that no longer exists. Found by walking the interruption and lifetime paths of the new flag rather than by the flag's own tests, which only covered the reinstall path. The regression check fails on the previous commit. * Say what --no-rollback actually did, and stop advising a flag already given * Only warn about rollback space where keeping it costs any, and name a remedy that works * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Document the rollback opt-out where the installer says the options live * Answer for the volume that holds the path, not its drive letter * Let the desktop install see the disk-full diagnosis too * Do not prescribe the opt-out to a run that already used it * Match the mount point itself, not only paths beneath it * A cacheless install still needs room for two environments * Make the volume-probe check true on Windows as well * Build the volume-matching cases the way the code reads paths * Measure the volume a link points at, not the one it sits on * Opting out of the rollback copy must not cost XPU detection * Record what the volume lookup does not cover * Bound the volume query the way the installer already bounds WMI * Never answer the free-space question from a snapshot * Answer nothing rather than guess, and confirm a removal the link-aware way * Do not promise an ARM64 copy the flag is about to delete * Ask st_dev whether uv can hardlink, not the df label * Vary every message that promises a copy the flag may delete * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * A caller who asked uv to copy still needs room for two * A symlinked venv costs nothing, wherever the cache sits * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Measure the blocks a discard would actually free * Let the measurement decide on POSIX instead of this run's cache mode * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Judge the tree by the cache it was built against * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Never let a pre-rename diagnostic abort the rename * Record what the building run knew instead of inferring it later * Keep the old tree's verdict out of this run's stamp * [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 * Confine the disk work to --no-rollback and the failure path * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep the stricter removal check to the path that asked for it * Diagnose a full disk wherever the install failed, not only at studio setup * Advise from what became of the old environment, not from the flag * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Restore the environment before diagnosing why the install failed * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Measure the disk before the restore empties it * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Remove comments left behind by the removed helpers --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
|
37a66c8a78
|
Installers: require x64 Python on Windows on ARM, and respect active conda (#11024)
* Installers: require x64 Python on Windows on ARM, and respect active conda Windows on ARM: pyarrow and hf-transfer publish no win_arm64 wheel, at any version, so a native ARM64 interpreter source-builds both and fails on CMake minutes into the install. install.ps1 already swapped a freshly selected ARM64 interpreter for x64, but it warned and continued when no x64 interpreter could be installed, and it never re-checked the interpreter of an environment it reused, so a venv migrated from an older install stayed on ARM64 CPython. Both now stop or rebuild, with UNSLOTH_ALLOW_ARM64_PYTHON as the opt-out. Conda: every persistent PATH write prepended, which writes an ordering that outlives the conda activation it was made under, so from the next shell on our directory sits ahead of the active environment's own entries. install.ps1, studio/setup.ps1, install.sh and studio/setup.sh now register the same directory at the back while a conda environment is active, and a system Python installed from python.org gets AppendPath instead of PrependPath. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honour the ARM64 opt-out before installing x64, and agree with the shell installers on conda Three corrections. UNSLOTH_ALLOW_ARM64_PYTHON was read only after Install-X64Python had already failed, so on any Windows on ARM machine that can reach python.org, which is most of them, setting it did nothing: an x64 interpreter the user had asked not to have was downloaded, installed and then used. It is read first now, and the warning says how to undo it. Inside an active conda environment the python.org route runs before winget, and the winget arm's own python.org fallback then ran it a second time when both had failed. On an offline machine that is the same failing download twice, announced as a fallback that had already been tried. install.sh and studio/setup.sh decided "are we inside conda" from CONDA_PREFIX alone while install.ps1 and studio/setup.ps1 read CONDA_PREFIX and CONDA_DEFAULT_ENV, so a shell hook that exports only the second gave the same user a prepend on Linux and an append on Windows. Both shell installers share one _unsloth_conda_env_active predicate now, and a test asserts the two halves name the same variables in either direction. Cases added: the opt-out with x64 installable, the single python.org attempt, CONDA_DEFAULT_ENV on its own for all three POSIX writers, and a zsh rc file. * Append to PATH in fish for real, and honour the ARM64 opt-out on a reused environment Two defects in this branch's own new code. fish: -a alone is not an append to PATH. Without --path, fish_add_path edits $fish_user_paths, and fish's own documentation says that variable 'is itself prepended to PATH, so they still stay ahead of the system paths'. Its worked example for exactly this case is 'fish_add_path --append --path /opt/fallback/bin'. So the conda arm wrote a line that left the directory in front of the active conda environment, which is the defect #5871 reports, arriving through fish instead of an rc file. Both installers now write 'fish_add_path -a -P'. The drop-in lives in conf.d, which is read alphabetically after conda.fish and sourced by every shell, and fish_add_path leaves an already-included directory in place, so re-sourcing and re-running the installer are both no-ops. The bare -a spelling still counts as present, so an install from before this fix does not collect a second line. UNSLOTH_ALLOW_ARM64_PYTHON: Resolve-WindowsOnArmX64Python never runs when the ordinary probe already found an x64 Python on the machine, so on an ARM64 box that has both interpreters the variable was never read, and the migrated-environment re-check then moved aside the very native ARM64 environment the user had asked to keep. One Test-Arm64PythonOptOut helper is now read at both sites. The rebuild also names where the old environment went and that its extra packages are not carried over, which a rebuild on a different architecture cannot do. Sources: https://fishshell.com/docs/current/cmds/fish_add_path.html Nine pwsh cases and three shell cases added; they fail on the previous head of this branch. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reposition an existing PATH entry when conda downgrades the prepend A machine that ran the installer once outside conda has our directory at the front of the User PATH. Rerunning inside an activated environment downgraded the prepend to an append, but the append-idempotent early return then returned without rewriting the registry, so the front entry survived and the run reported that conda keeps priority while it did not. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Take the python.org route for the x64 bootstrap under conda, and keep the ARM64 environment * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep an active conda ahead of the User PATH in the refreshed session * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Preserve the session PATH for a name-only conda activation * Remove conflict markers left in the installer PATH test * Keep an active conda environment in front during setup PATH refreshes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reposition a persisted prepend, and read the venv architecture before the move * Do not move the venv twice, keep what the opt-out promises, repoint before the guards * Anchor the ARM64 swap test on the call, not the shape of the if The guard is multi-line now that the native CUDA torch check joined it, and an anchor on "$DetectedPython -and" matched an unrelated earlier block instead of failing. * Let the ARM64 opt-out choose the interpreter, not only keep it UNSLOTH_ALLOW_ARM64_PYTHON was read at the swap that follows the selection, but on a host that already has an x64 interpreter the selection is x64 and that guard never fires, so the variable did nothing: a fresh install built an x64 venv and a reinstall replaced the native ARM64 one. Rank ARM64 first when the opt-out is set on an ARM64 host, leaving Install-X64Python's own -X64Only lookup alone. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Commit the rc rewrite by rename, and enumerate every stacked conda prefix cat staged > rc truncated the user's profile before writing a byte back, so an interrupt or an I/O error left it half written and the failure branch then deleted the only complete copy. Stage beside the real file, carrying its permission bits, and rename onto it; the symlink chain is resolved by hand first, since renaming over a link managed by chezmoi or stow would replace it with a regular file. conda activate --stack records the outer environments as CONDA_PREFIX_1 upward and CONDA_SHLVL counts them, so a fixed list ending at _3 dropped everything past the fourth and left those prefixes sorting after Machine and User. Also gives the ARM64 resolver harness the two real functions it calls, without which three of its four cases were failing on an unknown command rather than on an answer. * Hand the rewrite values to awk untouched, and repoint the $HOME spelling too POSIX awk decodes backslash escapes in a -v assignment, so a line holding a backslash -- which the writers escape before building it -- matched the literal grep and then matched nothing in awk: the helper renamed an unchanged file and reported the stale prepend as moved with conda still in front of it. ENVIRON hands the value over as it is. And the uv repoint pass was built only from the expanded path, so on every ordinary machine, where the destination IS ~/.local/bin, it could not match the export PATH= "$HOME/.local/bin:$PATH" line the shim block writes unexpanded. That pass already visits every startup file astral wired, so both spellings go through it; the shim's own repoint reaches only whichever profile is selected now. * Repoint the $HOME spelling in the standalone setup path too studio/setup.sh runs on its own during an update, after install.sh wrote the shim line unexpanded. _SETUP_LOGIN_PATH holds the expanded directory, which is what selects the repoint-only branch, while the line in the profile says $HOME, so the rewrite never fired and the stale prepend stayed ahead of the active conda environment. Both spellings go through the repoint, in that branch and in the already-present one. * Run the repoint helper in the shell harness and fix the spelling it uncovered * Move the fish entry rather than append one fish will ignore * Let the ARM64 opt-out find a native interpreter, and only a native one * Move only a line the installer put there * Bring the repointer fixtures to the ownership marker * Keep the rc file's permissions through the repoint The rewrite renames a staged copy onto the user's profile, so the copy's mode becomes the profile's mode for good, and a plain cp does not carry the source bits across: POSIX creates the destination with the source's mode as the mode argument, and a mode argument is always modified by the file creation mask. A 0644 .bashrc rewritten under umask 077 came back 0600. Both copies of the helper stage with cp -p now, which is specified to duplicate the permission bits, and fall back to the plain copy only where -p is refused outright. * Keep the ARM64 opt-out's environment on an ARM64 interpreter The selection prefers ARM64 under UNSLOTH_ALLOW_ARM64_PYTHON, but only among the interpreters discovery can see. With the ARM64 CPython that built the venv gone, there was nothing left to prefer: an x64 interpreter was selected, the reinstall had already moved the native environment to its rollback path, and the new environment really was built on x64 while the opt-out's own message said it would not be. pyvenv.cfg still names that base interpreter, so it is tried before the swap, and where it cannot be found the message now says plainly that the new environment is built on x64 and the ARM64 one is kept. * Keep the x64 bootstrap fixture parseable on the lint target A backslash inside an f-string expression is a 3.12 syntax addition and the repo's ruff target is 3.11, so the whole-repo check failed on the Windows paths in the stub preamble. Bind them before the template. * Cut the commentary down Same code, fewer words. The product diff went from 446 comment lines to 290, and the whole PR from 3381 added lines to 3099. Every reason survives; the paragraphs explaining them do not. --------- Co-authored-by: danielhanchen <unslothai@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |