main added nParallel to sidebar-model-config.tsx's own configSignature while
this branch moved that helper into config-signature.ts so the hub, the sidebar
and the model config page key one editor instance the same way. The behaviour
is unchanged, so the assertion follows the expression to where it now lives and
pins the sidebar to the shared key.
* Studio: show a Nudging tool calls badge while the tool-call re-prompt runs
* Guard the nudge status ordering assertion against index 0
* Tighten the nudge status comments
* Announce the nudge text instead of the generic spinner label
* Trim the nudge status comments
Collapse the multi-line notes to fewer lines and drop one that restated the assert below it. The blank-before-badge ordering reason and the keep-in-sync contract are preserved.
---------
Co-authored-by: danielhanchen <unslothai@gmail.com>
* fix(studio): show the current artifact's source after switching artifacts
The canvas source view feeds one Streamdown a fence built from the selected
artifact's code, but never keys it. Streamdown does not revise a block it has
already committed, so the panel keeps rendering the previous artifact's source.
Key the source view on the artifact ID plus a hash of its code: tool artifact
IDs are derived from the tool call, not the code, so the ID alone does not
change when a tool artifact is updated in place.
* Name the real root cause and make the source-key test load-bearing
The remount is needed because Streamdown memoizes a fenced code block on its
hast node's line/column span, which ignores the text inside the fence, so two
canvases of equal line count compare equal and the old source stays on screen.
Verified in Chromium against streamdown 2.5.0: unkeyed, 70 lines -> 70 lines
renders the previous artifact, 70 -> 71 and 70 -> 90 render correctly.
Move the key expression into the source branch so it costs nothing while the
artifact is streaming and the view is unmounted, and export the helper from
types.ts so the test exercises the shipped code instead of a local copy of the
formula (it passed before even with the key removed from the component).
* Assert the source view's Streamdown key wiring, not just the helper
The suite exercised buildArtifactSourceKey but never the component, so deleting
key={buildArtifactSourceKey(artifact)} from the Streamdown left every test
green. There is no DOM renderer available to these tests, so parse
artifact-surface.tsx with the TypeScript compiler API (already a devDependency)
and assert the source view's Streamdown carries that key.
Mutation-checked: removing the key fails 1 test, swapping it for artifact.id
fails 1, and making the helper ignore code fails 2.
* Tighten the comments added by this PR
The frontend splitter accepted any suffix after a .gguf head, while the backend
requires that suffix to be the label the scanner derives from the filename. A
colon is legal in a POSIX filename and POSIX is case sensitive, so
"/models/llama.gguf:Bar.gguf" and "/models/llama.gguf:bar.gguf" are two real,
distinct files. The one-time backfill folded both onto one override key, since
the variant half is stored lowercased, and then re-read the local configs by
that key, so the first entry was sent twice, the second file's context and KV
cache settings never left the browser, and the done flag was set anyway.
splitQuantSuffix now ports extract_quant_label for a bare filename, shard suffix
and float-precision fallback included, and takes the suffix only when it equals
that label. Checked against the backend over twenty-nine keys with identical
answers on both sides, up from twelve, and the backfill now migrates both files
with their own settings.
The identity helpers come straight from features/hub/lib/model-identity rather
than the hub barrel, which also re-exports the download manager and its React
components. Same bindings, and it puts the module within reach of a test.
The 27 new frontend assertions cover the split case by case against the
backend's answers, and pin the storage rule the backfill's re-read depends on:
two spellings of one repo id or one Windows path keep a single record, a POSIX
path keeps two, and importing the legacy load settings never adds a duplicate.
* 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>
* feat(studio): share the llama-server --parallel bounds as PARALLEL_MIN/MAX
The per-load parallel-slots field needs the same 1..64 range the CLI flag
validates, but models/inference.py cannot import run.py (run.py builds the
app that imports routes that import models). Promote the bounds into this
dependency-free module, which already owns the -np/--parallel semantics, and
record the deliberate mirrors that cannot import it (run.py, the unsloth CLI,
the web UI). The denylist entry stays: the first-class field is now the single
write path for the slot count, so a pass-through would still desync the
committed bookkeeping from llama-server.
* feat(studio): note the per-load override in the --parallel help text
--parallel is now the server-wide default that a per-load n_parallel (the
Studio Parallel Slots run setting) can override, not the definitive slot
count. Point at the new control so a user does not conclude a restart is the
only way to change slots, and record the shared PARALLEL_MIN/MAX mirror
alongside the existing CLI one.
* feat(studio): add n_parallel to LoadRequest and echo the slot counts
LoadRequest.n_parallel (optional, PARALLEL_MIN..PARALLEL_MAX) lets a load pick
its own llama-server --parallel count; omitted, the server-wide launch default
applies. ValidateModelRequest carries it too so the training-coexistence
estimate sizes the KV cache like the follow-up load rather than passing on a
smaller footprint.
LoadResponse and InferenceStatusResponse gain both requested_parallel_slots
(what the load was invoked with) and parallel_slots (what llama-server
actually runs after the fitter's slot reduction), so a client can tell an
honored request from a reduced one. Both are None where --parallel has no
meaning: non-GGUF loads and the diffusion runner.
* feat(studio): record the requested parallel-slot count on the backend
The auto GPU-memory fit may launch fewer slots than requested to keep the
model fully on GPU, so the committed effective count cannot answer "is the
live server what this request asked for?". Store the invoked count separately
(mirroring the _requested_n_ctx pattern) from the pre-reduction pending
kwargs, expose it as requested_parallel_slots, and have _already_in_target_state
compare requested-vs-requested: comparing against the effective count would
reload -- and re-reduce -- forever on an identical Apply.
The comparison sits in the non-diffusion branch, since the diffusion runner
ignores --parallel entirely. The requested value shares the effective count's
lifecycle, so every unload/kill path clears it and a stale count cannot
poison the next load's dedupe.
* feat(studio): honor a per-load parallel-slot count in /load and /validate
Resolve the slot count once per load -- the request field if set, else the
server-wide launch default -- and feed it to every consumer that must agree:
the training-coexistence guard, the llama-server load kwargs, and the reload
dedupe. Without the dedupe comparison a changed slot count would be swallowed
as already_loaded; it compares requested-vs-requested and skips the diffusion
runner, which ignores --parallel.
app.state.llama_parallel_slots is deliberately never written: it stays the
launch intent and the admission-queue fallback, so one load's override cannot
leak into later loads. /validate resolves the same way so its estimate cannot
undercount what the load then allocates.
Both /load returns and /status echo the counts through one helper, which
reports None for diffusion -- its load never commits a count, so echoing the
reset placeholder would fabricate an "invoked with 1 slot".
* feat(studio): accept nParallel in the chat-preset load config
ChatPresetLoadConfig is extra="forbid", so a preset carrying the new parallel
slots knob would 422 the whole settings sync without this field. Bounds come
from the shared PARALLEL_MIN/MAX rather than literals, so a future range
change cannot start rejecting presets the UI still allows.
* test(studio): cover the per-load parallel-slots knob
Pins the behaviors a regression would silently break: the requested-vs-effective
dedupe (comparing against the reduced count would reload forever), the diffusion
skip and its None echo, the requested count's reset lifecycle, and its commit
from the pre-reduction pending kwargs.
Also pins the three bounds mirrors that cannot import PARALLEL_MIN/MAX (run.py,
the unsloth CLI, the web UI) plus the preset model that can, so a range change
cannot leave one of them clamping or rejecting at the old limit.
* test(studio): refresh the --parallel denylist comments for the UI knob
The pinned rationale said the typer flag owns the slot count and pointed users
at a Studio restart. Parallel Slots / LoadRequest.n_parallel is now the other
managed writer, and the 1..64 guard is the shared PARALLEL_MIN/MAX -- a reader
following the old comments would conclude the UI control does not exist.
* feat(studio): note the per-load override in the CLI --parallel help
Both the plain-serve and `unsloth studio run` flags now describe a server-wide
default the Studio Parallel Slots run setting can override per load, matching
the backend help text.
* feat(studio): remember a per-model Parallel Slots override
nParallel joins the per-model config with the same null-means-follow-the-default
convention as the other knobs: null keeps the server-wide --parallel count, so
a blank control never pins a number and isDefaultConfig still deletes an
otherwise-untouched config instead of storing it.
The value is re-clamped to N_PARALLEL_MIN/MAX on every localStorage read and
write (the store is user-editable), and listing it in STORED_CONFIG_FIELDS
keeps it from being dropped as an unknown key. Legacy blobs predate the knob,
so their migration carries null. No schema-version bump: an additive optional
field, like the GPU fields before it.
* feat(studio): bridge nParallel between the per-model config and the store
The config->store, store->config and equality helpers all need the new field:
without the equality arm a slots-only edit reads as unchanged, so Apply is
dropped and the dirty state never lights up.
* feat(studio): track the parallel-slot override in the chat runtime store
nParallel holds the editable override and loadedNParallel the value the last
successful load sent, which the failed-switch rollback re-sends. Both are
per-model: they clear on unload and on a model switch, unlike the standing
preferences (GPU memory mode, speculative type) that survive one.
There is deliberately no backend-echo field for the control: the echo is the
resolved count, so adopting it would pin a blank "follow the server default"
input to an explicit number.
* feat(studio): type n_parallel and the slot-count echoes
The load request gains the optional per-load slot count, and both the load
response and the status payload gain requested_parallel_slots (invoked) and
parallel_slots (actually running after the fitter's reduction). Keys stay
snake_case: the payload is serialized as-is, with no case conversion.
* feat(studio): forward n_parallel to the validate preflight
validateModel builds its own body rather than forwarding the load payload, so
the slot count has to be listed explicitly. Slots scale the KV estimate, and
the preflight exists to refuse a load the training guard would then 409 -- an
unforwarded count would validate a smaller footprint than the load allocates.
* feat(studio): include nParallel in the active model's config
The sidebar assembles the active model's config from individually subscribed
store fields; an unsubscribed field would leave the form showing a stale value
after any external change.
* feat(studio): add the Parallel Slots control to the run settings
A numeric input in the GGUF advanced section, blank meaning "follow the server
default". It clamps on change like the Draft Tokens field rather than using
NumericValueInput, so there is no blur-draft to lose when the user types a
value and immediately clicks Load.
hasNonDefaultAdvanced counts it too, so a remembered override reopens the
advanced section instead of hiding the setting that is actually in effect.
* feat(studio): key the sidebar config form on nParallel too
The signature drives the remount that re-seeds the form; without the new field
an externally changed slot count would leave the sidebar showing the old one.
* feat(studio): send the Parallel Slots override on load
performLoad snapshots the slot count at click time (staged run-settings config
first, else the store) and sends it on both the validate preflight and the
load, so the two size the same footprint. A cross-model switch re-baselines it
like the other per-model knobs -- the previous model's count must not follow
onto the next one -- and the failed-switch rollback re-sends the previous
model's value so a rescue reload cannot silently drop to the server default.
The success path keeps the click-time value rather than the response echo: the
echo is the count the fitter resolved, so adopting it would turn a blank
"follow the server default" control into an explicit pin. Slots are GGUF-only,
so a transformers load sends and records null instead of a phantom override.
* feat(studio): carry the slot override through the compare-pane load
The compare pane builds its own load request, so it needs the field explicitly
or a pane with a remembered override would load at the server default. Its
validate preflight sends the same count, matching the comment above it that
promises validation is sized exactly as the load below.
GGUF-gated on both calls, and the store adopts the pane's own click-time value
rather than the resolved echo, mirroring the single-model path.
* feat(studio): honor the remembered slot override on startup auto-load
The auto-load path reads the per-model config and forwards every other
remembered knob, so a remembered Parallel Slots value was the one setting lost
on the "load last used model" path: llama-server came back at the server-wide
default with the control showing blank, and the first manual Apply afterwards
then forced a needless reload because the counts disagreed.
* feat(studio): seed the slot baseline from the status echo
Only the rollback baseline is seeded, never the editable control: the echo is
the resolved count, so adopting it would pin a blank "follow the server
default" input to a number. Without the seed, loadedNParallel stayed null
after a tab reload or a second tab adopting the running model, and a failed
switch then rolled the previous model back at the server default while every
other knob was restored.
* feat(studio): capture Parallel Slots in chat presets
The knob joins the preset load config end to end: captured from the store,
re-clamped when read back (persisted presets are untrusted input), applied on
switch, and summarized in the preset chip. Its default is null, so
coalesceDefaultLoadKnobs keeps a default-only preset empty rather than
persisting a no-op override.
* feat(studio): re-derive the preset state when Parallel Slots changes
Both preset memos snapshot the store through capturePresetLoadConfig, so
without the new dependency a slots-only edit left the unsaved-changes flag and
the load summary showing the previous value.
* test(studio): pin the Parallel Slots wiring end to end
Source-contract coverage for the hops a refactor can silently drop: the three
/load builders (interactive, compare pane, startup auto-load) and their
validate preflights, per-model persistence and clamping, the UI row, and the
status seed -- including the negative assertion that hydration seeds only the
rollback baseline, never the control, so the resolved echo cannot pin a blank
"server default" input.
* test(studio): pin nParallel in the preset load config
Covers capture, clamped read-back and apply on the frontend, plus the backend
field itself: ChatPresetLoadConfig is extra="forbid", so a missing or drifted
field 422s every settings sync that carries a preset.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Fall back to one slot when llama-server lacks --kv-unified for PR #7447
Without --kv-unified an explicit --parallel N makes llama-server give each slot -c/N, so on a build without the flag choosing N slots silently shrinks every context window for a feature that build cannot serve. Clamp to one slot and log why, placed after the requested count is captured so the echo still reports it and before the KV estimates so the fit matches what actually launches.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Clear the slot control on load paths that never send it, and size the training guard for diffusion
Four review findings on the per-load Parallel Slots knob.
The editable nParallel control means "follow the server default" when null, so
any success path that does not send a slot count has to clear it. Three paths
kept a value staged for a different model:
- chat-adapter.ts, cached non-GGUF auto-load: the interactive and compare
builders already clear both fields for a non-GGUF response, this third one
did not. The field never renders for a non-GGUF target, so the stale count
was invisible and unclearable from the UI yet still persisted, and it flips
isDefaultConfig so a user with no overrides silently gets a stored entry.
- chat-adapter.ts, fresh-model fallback: its request omits n_parallel but its
success state resynced every other knob and left the slots alone, so a staged
edit survived against a server running the default and the next Apply
reloaded at a count that load never sent.
- apply-inference-status-to-store.ts: on a model change underneath the tab
every sibling knob adopts the new model's status, but nParallel updated only
its baseline, so the previous model's explicit count followed onto the new
model and saving or reloading there pinned it. Clear the control and keep
seeding the baseline for the rollback.
The training-coexistence guard sized a diffusion GGUF with the requested slot
count. _estimate_kv_cache_bytes scales the SWA cache with slots
(swa_limit = swa * slots + ubatch), but load_model hands a diffusion target to
_start_diffusion_server before the slot plumbing, so that runner is always
single-slot. At the new default of 4 this inflated the estimate and could 409 a
load that fits. An unclassified GGUF keeps the requested count.
Backend base KV depends on -c alone, not on --parallel, which is why only the
SWA term is affected: llama.cpp PR 14363 and discussion 4130.
Tests: three training-guard cases in test_parallel_slots_per_load.py and one
source contract in test_model_picker_contracts.py, each mutation-checked.
174 passed across the backend slot/admission/training suites, 56 across the
frontend contract suites.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Keep the slot control when re-adopting the running model, and never record slots for a diffusion load
Two follow-ups from the latest review round.
The first is a regression from c796393. That commit cleared the slot control
whenever hydratingExistingModel was set, to stop model A's count following onto
model B. But that flag is also set on the resident-model adopt path: when the
store checkpoint is an external provider id and the user re-picks the still
loaded local model, applyActiveModelStatusToStore is called with the external
id as previousCheckpoint, so the flag is unconditionally true. The clear then
wiped the config applyPerModelConfigToRuntime had restored two lines earlier,
and it was the only knob that did, because the siblings re-adopt the status
echo while this one cleared. Gate the clear on the tab's own baseline no longer
matching the running count: a genuine A to B swap still clears, re-adopting the
same model keeps its value.
The second revises an earlier call of mine. I rejected the diffusion phantom as
cosmetic because the backend ignores the value on every send. The sharpened
report is right and my rejection was wrong: capturePresetLoadConfig records
nParallel with no model gate, a Preset carries no model id, and applying one
writes nParallel for whatever model is current. So a count recorded against a
diffusion model, which the backend never applied, rides a saved preset onto a
text GGUF and becomes a real override the user never chose. Record slots only
when the load actually committed them, on all three load builders.
Tests: two source contracts in test_model_picker_contracts.py, both mutation
checked. Frontend typecheck clean, 58 passed across the contract and preset
suites.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Clear the slot baseline when status reports a model without slots
Hydrating from a GGUF to a slotless model left loadedNParallel at the previous
model's count: the seed only runs when the echo is non-null, and the control
clear added earlier touches nParallel alone. The stale baseline is what a
failed-switch rollback re-sends, and preset capture reads it, so it could claim
slots for a model that never used them.
Clear it when status describes a model that cannot have slots. /status omits
the echo entirely for non-GGUF and sends an explicit null for the diffusion
runner, so keying on is_gguf === false or an explicit null covers both while an
absent field on a GGUF, which is how an older backend reports one, still leaves
the baseline alone.
Test mutation checked; frontend typecheck clean against a fresh npm ci.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Distinguish a same-model re-adopt from a model swap, and size the training guard at the slots that launch for PR #7447
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Keep the blank slot control across a failed-switch rollback for PR #7447
* Restore a remembered slot override when hydrating a fresh store for PR #7447
* Tighten comments for PR #7447
* Restore a remembered slot override on a model switch too for PR #7447
* Tighten comments and docstrings for PR #7447
* Take the rollback slot intent from the picker's pre-switch snapshot for PR #7447
* [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>
Co-authored-by: danielhanchen <unslothai@gmail.com>
Lifecycle rows are shared so a load or a download shows up in everyone's
monitor list, which is deliberate. Since they started carrying via_api_key
they also carry the flag the floating panel auto-opens on, and that reached
every authenticated subject: another logged-in browser sprang open for API
traffic it had nothing to do with. The row now records the caller that drove
it and reports the attribution only to them. Visibility is untouched, so the
row still appears for everybody, and a subject-scoped Clear hides a shared row
it owns rather than deleting it out of everyone else's history.
Auto-download had the flag hardcoded on, reasoning that only an API request
gets that far. Only a /v1 request does, which is not the same thing: Studio's
own chat calls those same endpoints with a session JWT, so a chat that named a
model this server does not have popped the panel open mid-chat, which is
exactly what via_api_key exists to prevent. The attribution now comes from the
request that asked for the download.
Two ways the Hub's per-model settings page could offer a resident model's
saved or default values as if they were live, and then write them back over
the running config on Apply.
ModelConfigPage seeds its editable state from loadedConfig in a useState
initializer, so it reads that prop once per mounted instance. The sidebar
entry keys its instance on a signature of the live config; the Hub's keyed on
the model and quant only. Open the page before /api/inference/status has
hydrated, or while that same target is still loading, and loadedConfig flips
from null to the live config after mount with nothing to remount on: the
editor keeps the values it seeded from and Apply reloads the model with them.
Both hosts now mount under one shared key that includes the live config, so
the arrival of that config re-seeds the editor and a repeated poll of the same
values does not.
The live config itself comes out of the chat runtime store, and landing
straight on /hub is the one entry point where nothing has applied the status
yet: useChatModelRuntime has no mount sync and the chat page is a different
route. The Hub's own status effect pinned the checkpoint and stopped there, so
the resident check passed while kv cache, speculative decoding, tensor
parallel and every GPU placement field still held their defaults. It now
applies the whole status, the same call the chat runtime's refresh makes, and
holds off when a load owns the store or an external provider is selected so it
cannot fight either.
* Pin the newer-mapper FP8 probe with tests that can fail
The two identity assertions added in #7478 compare the returned FP8 tables
against the installed ones, but the fixture serves the same mapper.py as both
the installed and the fetched source and exec always allocates fresh dicts, so
they pin allocation rather than provenance and hold for any new dict.
Replace them with two tests that drive get_model_name end to end: one splices an
FP8 entry into the fetched source only and asserts the upgrade error still fires,
the other serves a mapper.py with no FP8 tables and asserts the 4bit half of the
probe survives, which is the regression #7497 fixed.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Tighten the resolver stub for PR #7516
- Restore the fp8_block/fp8_row identity assert alongside the new provenance
test. It is weak, not vacuous: it still catches a probe that hands back the
installed table objects, and it costs nothing to keep.
- Bind Version and transformers_version in the stub namespace. Both are
unreached under the current gates, so a change to either would fail with a
bare NameError instead of the assertion.
Merged main, which clears the unrelated test_runtime_text_encoding failure the
branch inherited from its base.
* Cover the FP8 row-scaling path instead of duplicating the block one
The two tests this PR originally added were already covered by
tests/test_new_mapper_fetched_fp8.py from #7497. An 8-mutant matrix over
loader_utils.py found nothing they caught that the existing file did not, so
they are dropped and test_new_mapper_no_global_leak.py goes back to main.
Two real gaps were open, both on the row branch that load_in_fp8 = True plus
UNSLOTH_HAS_FBGEMM selects ahead of block:
- the FBGEMM row branch in __get_model_name could be deleted outright with
every test still green
- _resolve_with_mappers could ignore its fp8_row argument and silently fall
back to the installed row table
Adds two tests to the existing file, reusing its _load_resolver rather than a
second harness. The row-only fixture splices into the fetched row table alone,
since an entry the block table also knows lets the block branch answer and
masks the regression.
* [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>
* fix: add XPU device support and update hardcoded CUDA selections
* fix: add XPU device support for pytest CUDA skipped tests
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Fix device handling for PR #7401
- perplexity_eval.py: use DEVICE_TYPE_TORCH, not DEVICE_TYPE. The latter can
be "hip" or "mlx", which .to() rejects, so this regressed ROCm.
- test_batched_leftpad_generation_gpu.py: XPU diverges here today, so mark it
non-strict xfail on XPU instead of reverting to a CUDA-only guard. Keeps the
real XPU gap visible and turns green once it is fixed.
- Guard torch.xpu.is_available() with hasattr, matching device_type.py.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Re-enable the flash varlen attention test in CI for PR #7401
attention_dispatch.py now predefines flash_attn_func / flash_attn_varlen_func
as None, so test_run_attention_flash_varlen_receives_window_and_softcap no
longer needs flash_attn importable to be monkeypatched. Verified on a runner
shaped like the CPU-only one: the test fails against main's attention_dispatch
and passes at this head, so the deselect is now dead weight.
* Tighten comments for PR #7401
Drop the hasattr rationale: torch.xpu has existed since torch 2.3 and the
dependency floor is 2.4, so no supported build predates the namespace. The
guard stays as cheap defence, but the comment claimed something untrue.
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <danielhanchen@gmail.com>
A loose .gguf keys its settings by the bare path with no variant, since the path
already names the one file. The loader still derives a label from the filename
and /status reports it, so the settings page compared a derived quant against a
deliberate null, never matched, and withheld the live launch config from the very
file that was loaded. Applying from that page could then write the saved values
over what the resident model is running with.
The variant equality is skipped for that case only. Every other target, a repo
row or a directory, still has to agree on the quant, because there the variant is
what tells two loaded copies apart.
The overlay opens on API-key traffic only, and record_lifecycle never set that
flag. Auto-switch and auto-download run before the endpoint opens its request
row, so a switch or a download that is refused never reaches api_monitor.start
and its lifecycle row is the whole trace of the request. The monitor therefore
stayed shut on exactly the failures automatic observability is for.
The row now carries the attribution: a load takes it from the request that drove
it, and auto-download passes it directly, since only an API request reaches that
path at all. A manual unload and an idle unload are not API traffic and stay
unattributed, so neither pops the overlay.
The manifest-removal guard added in #7492 exits with a bare 'exit 1', so in
Tauri mode the installer never emits the [TAURI:ERROR] line and the desktop
UI falls back to a generic failure instead of naming the cause. Every other
failure path in studio/setup.ps1 goes through Exit-SetupFailure, and
tests/sh/test_tauri_retry_failure_context.sh asserts that invariant, so
'Repo tests (CPU)' has been red on main since that merge.
Co-authored-by: danielhanchen <unslothai@gmail.com>
* Bound how many approvals may park, against the executor
#7455 landed parking, which is the right shape and supersedes what this branch
was carrying. It is unbounded, though, and the thing it is unbounded against is
not the GPU.
A run stopped on an approval prompt is blocked inside the to_thread(next, gen)
call that drives it, so it holds one of asyncio's default min(32, cpu + 4)
executor threads until the user answers. The slot cap used to bound that.
Parking hands the slot back, which admits another run that can park too, so the
ceiling became the wait line: 64 deep on a 1-slot backend. Long before that, the
executor is full and nothing else in the backend runs, including generation
steps for chats that already hold slots and the stream teardown that would clean
up after a disconnect.
The pool already permits `capacity` pending prompts, and each park adds one
more, so the budget is what the executor has left after the cap and a reserve of
4. On this machine (32 workers) --parallel 4 gets 8 parks and 20 free threads,
--parallel 24 gets 4 and 4, and --parallel 28 or higher gets none: there the
prompt keeps its slot and behaves exactly as it did before parking existed.
Counted process-wide rather than per queue. There is one executor, but a
per-queue budget is the same allowance again for every backend, and base_url
carries a fresh port on every model load, so a reload would mint a queue that
knows nothing about the approvals still parked on the old one. A reset clears it
too, or a leaked claim shrinks the budget for the life of the process.
park() reports whether it took the budget, and a refusal costs nothing to undo
because the slot never left its holder. The stream reads that answer rather than
recording a refused park as parked, which would make it skip the park for every
later approval in the same run even once the budget freed up.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Size the park budget from the executor's own CPU count
Two review findings, both real.
The budget read os.cpu_count(). 3.13 sizes ThreadPoolExecutor from
os.process_cpu_count(), which honours CPU affinity and cgroup quotas, and
asyncio's default executor is a plain ThreadPoolExecutor(), so a container
pinned to one core on a 64-core host got a 5-thread executor and a budget
computed from 64. The bound was then looser than no bound at all in exactly the
environment that can least afford it. It asks the same source the executor does,
and the test compares against a real ThreadPoolExecutor rather than restating
the formula, so it stays right on 3.12 as well.
The reserve was a flat 4, which on that same 5-thread executor left nothing to
budget and turned parking off entirely. Small hosts are where a chat most needs
to keep moving while another sits on a prompt. It scales now, and the ceiling
has a floor of two: a quarter of five is one, and one park cannot cover two
chats on prompts at once, which is what #7455's own two-approvals test needs.
Without that floor, that test fails on a one or two CPU runner. `spare` still
takes the budget to zero when the pool already fills the executor, so nothing
about a 32-worker machine changes: --parallel 4 still gets 8 parks, 24 gets 4,
28 gets none.
The two behavioural budget tests pin the worker count rather than reading it off
the runner, and the property test sweeps executor sizes from one CPU to 64
instead of asserting against whatever the host happens to have. The whole suite
passes with the CPU count faked to 1, 2 and 4, which is how both of these were
reproduced.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Size the park budget from every live backend, and free it on the answer
Two review findings, both real.
The budget was global but sized from one queue's capacity. A reload mints a
queue on a new port while the old one drains, so both are live, and prompts on
both park executor threads. Eight parks on an old 1-slot queue plus a new
24-slot backend is 32 threads on a 32-thread executor, with the new backend's
prompts refused and holding their slots, which is the state the reserve exists
to prevent. It sums the capacity of every backend still serving instead. Idle
queues are skipped: those are the ones the registry is about to evict, and they
are holding nothing.
The budget also outlived the wait it was paying for. unpark_async only dropped
it after reacquiring a slot, but the generator yields its post-approval event
first, so the executor thread is already back in the pool while the resume
queues. An approved chat waiting on a slot would refuse a different chat's park,
and that chat then keeps the slot the resumer is waiting for, so an unanswered
prompt strands chats that were already approved. The budget is released when the
prompt wait ends now, and the queue's parked count still runs until the slot is
back, which is what guards idle eviction and the resume ordering.
Both are separate counters on the lease as a result, and every exit from a park
drops the budget: unpark, unpark_async and release. That last one was the mutant
that came back missed, since a client disconnecting on a prompt releases
straight out of parked and would otherwise lose a budget slot for good.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Tighten the comments on the park budget
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Three follow-ups on the last round.
The auto-switch load tried the advertised id before the concrete load path, so an
entry under the alias shadowed the settings the user had just saved and kept
shadowing them. The settings page keys every local row by the path being loaded,
while the alias is derived: the /v1/models name a hand-written overrides PUT is
written against, and for a loose .gguf only its filename stem. Each pair now
reads the path first, and the alias, the bare ids and the older filename-label
key are all still read after it, so a cached repo (keyed by its repo id, which is
the alias) resolves exactly as before.
publicModelId collapses two paths that share a filename or a directory basename
onto one id, so the resident check added last round could mark the wrong catalog
row as loaded and seed its editor with another model's live launch config, then
save that under this model's key. The Hub page now records the loadable
identifier /status reports, as every other status reader already does, so the
literal comparison names one row; the public-id pass only accepts a namespaced
repo id, the one collapse that cannot name two models.
The override map shipped before this browser mirror did, holding only
llama_extra_args and max_seq_length, so an upgraded install can have a server
entry for a model whose context, KV cache, speculative and GPU settings live only
in localStorage. The backfill read key presence as done, skipped exactly those
models and then marked itself complete, so their API loads lost the settings for
good. Filling them in from the browser would reopen the race the conditional
write just closed, so the merge is the server's: the PUT flag is now
fill_absent_fields, and studio_db merges field by field under the write's own
transaction, where a stored value always wins. A fill with an entry already there
also stops replaying that entry's stored flags through validation, so one
denylisted since it was saved cannot 400 the one-time migration.
Tests: the key order and the fill in studio/backend/tests, the collapse in
studio/frontend/tests, and the wiring in tests/studio/test_model_picker_contracts.py.
Five follow-ups on the per-model settings map.
The one-time localStorage backfill read the override map once and then wrote
each model in turn, so a save by another tab during that pass was replaced by
this browser's older copy, against the migration's own "never overwrites"
contract. Re-fetching per model would cost a round trip each; instead the PUT
takes only_if_absent and the server tests and writes under one transaction.
gpu_ids arrived unbounded and normalize_model_override de-duplicated it by
scanning the list it was building, so a large authenticated array cost roughly
20x what the same work costs with a set (4.5s against 0.27s for a million
entries). The payload now bounds the field to the number of ids the normalizer
can store, and the dedupe uses a set.
A settings target opened from the Chat model picker carried no apiLoadable, so
the isGguf fallback mirrored an Ollama GGUF to the server. Ollama's blobs reach
that picker as custom-folder GGUFs under a .studio_links / ollama_links dir,
which local_model_resolver refuses to index, so the mirror advertised a load the
API can never make. The picker, the sidebar editor and the backfill now all use
the same classification.
The Hub settings page compared the loaded model to settingsTarget.id, but a GGUF
loaded from an inactive HF cache or straight off disk loads by path while
/status reports the clean public id, so the page ignored the live launch config
and showed saved or default values. It now also matches the settings identity
and the public id the backend would report.
A standalone .gguf gets a filename-derived format_variant from the inventory, so
the Hub row menu stored its settings under <path>:Q4_K_M while the Chat picker,
the detail card and the backfill all used the bare path. The row menu now uses
the bare path too.
Tests: publicModelId / residentModelIdMatches / isOllamaLinkPath /
settingsGgufVariantForRow in studio/frontend/tests, the create-only write and
the gpu_ids bound in studio/backend/tests, and the wiring in
tests/studio/test_model_picker_contracts.py.
Comment-only pass over the quant-suffix split, the storage-shape normalization
and the wall-clock deadline in the idle-unload test. Same points, fewer lines.
savePerModelConfig normalizes before deciding whether a config is default, and
the runtime hands the settings page Speculative Decoding "auto", which
canonicalizes to null. The page judged the raw object instead, so a model sitting
at defaults looked non-default: turning on "Remember for this model" reported
saved while the local write had dropped the entry, reopening showed it as not
remembered, and the mirror sent the server a speculative_type "auto" override the
browser did not have. That disagreement between the two is the one thing the
mirror is written to avoid.
The page now normalizes once and uses that object for the default check, the
local write and the server mirror alike. Driving the real module under node: the
raw object reads as non-default, storage stores nothing, and only the normalized
reading agrees with storage.
The backfill took a key's identity by splitting on its last colon, so anything
else that ends in one was read as a quant separator. A Windows path made
"C:\models\foo.gguf" into model "C" with variant "\models\foo.gguf", and an
ordinary colon inside a POSIX filename folded "/models/foo:Bar.gguf" and
"/models/foo:bar.gguf" onto one key, so whichever of the two was already on the
server made the other look migrated and left its API loads on defaults.
splitQuantSuffix now mirrors split_quant_suffix on the backend: the suffix has to
be a known quant label, with or without a bits-per-weight modifier, or the head
has to be a .gguf carrying a stem label. Checked against the backend over twelve
keys, including every case above, with identical answers on both sides.
Settings also opens from the on-device detail card, and that constructor never
set apiLoadable, so an Ollama model reached the server mirror and the "API loads
use these settings" line from that entry point even though the auto-switch
resolver skips Ollama's scanner. It now reads the same source the row menu does.
The drive loop waited a fixed 200 iterations of a 10 ms sleep. Windows rounds
that sleep up to the roughly 15.6 ms scheduler tick, on this loop and on the
idle loop under test alike, so the unload got far less real time there than the
count suggests and the test failed on the Windows runner for being slow rather
than wrong. It now waits on time.monotonic with a generous ceiling and still
breaks as soon as the KV file is gone, so the fast path costs nothing.
The test is older than this PR; the deadline is the only change to it.
_is_abs_path_id decides whether an id is a host path that must not be published
through /v1/models, and it asked pathlib.Path, which follows the running OS. A
Windows backend therefore read "/home/me/x.gguf" as a relative name and a POSIX
one read "C:\models\x.gguf" the same way, and in both cases the path was
advertised verbatim as a model id.
Ids outlive the machine that wrote them: settings sync, a WSL session and a
copied config all carry the other platform's spelling, which is why the
model-override identity in this PR already folds Windows drive, UNC and WSL
paths. The backend now agrees with it, reading the value as both a POSIX and a
Windows path. Neither reading can misfire on a repo id, which has no leading
separator, drive letter or UNC prefix.
The two tests this fixes on Windows are older than this PR; the new one pins the
contract in both directions.
Pass over every comment this branch touches. Collapse the multi-paragraph
rationales to the point they were making, drop prop docs that only restated
the prop name, and reflow the rest onto fewer lines. No code changes.
A repo cached outside the active HF cache reports load_id as its snapshot path
(cache_inventory), which is what the loader needs, but the chat picker and the
auto-switch index both name that repo by repo_id. The new Hub settings page was
saving under the load id, so the settings landed on a key no other load reads:
the picker, an auto-load and an OpenAI-compatible request all fell back to
defaults, and a server override already stored under the repo id could win
against the save.
ModelPickTarget now carries configId for the case where the storage identity is
not the loadable one. Every read, write and server mirror in ModelConfigPage
uses it; the chat template and GGUF header probes keep target.id, since they
have to open the model. The Hub sets it for cache rows and resolves its own
load through the same helper, so a config saved from the settings page is the
one a later load finds. Rows whose load id is already their identity, which is
every local row and every repo in the active cache, are unaffected.
Verified against the real per-model-config module under node: saved under the
snapshot path, a picker read reports remembered=false and the default max
sequence length; saved under the repo id it reports remembered=true and 8192.
The backfill wrote the snapshot it took before fetchModelOverrides resolved, so
a save or a forget during that round trip was undone: the write is queued behind
the interactive one and commits last, leaving the browser showing the new
settings while an API load applied the old ones. Each write now re-reads the
model's current local config and skips it if it has gone or gone back to
defaults. Verified against the real module under node: the write carries
maxSeqLength 9999 where it previously carried the stale 1000.
target.isGguf was also standing in for "an API request can load this". It
cannot for an Ollama model: local_model_resolver skips Ollama's scanner on
purpose, so those models are never in the auto-switch index, yet the mirror ran
and the settings page told the user the API would apply them. The target now
carries apiLoadable, set from the inventory source the row already has, and both
the mirror and that sentence read it.
Extracting the labels out of the monitor page left two contracts asserting they
were still in it, so the staged run went red on all three platforms. They read
the new module now, and the page contract additionally pins that it imports from
there rather than redefining them.
Three separate reports, all confirmed against head.
listPerModelConfigs reported future-schema records. loadPerModelConfig refuses to
apply one and eviction refuses to drop one, because this client cannot interpret
that schema, so handing it to the backfill would persist a partial reading of it
server-side and let an API-triggered load apply settings the same client will not
apply locally. It is skipped there now, matching the other two paths.
The detail view's on-device card passes a null variant while its own lookup is
pending or after it failed, and this entry point opened the editor anyway. That
saves a bare-model config, which the picker never finds because it matches
variants exactly, while the API's bare-key fallback would apply it.
openModelSettings already refuses with a toast for exactly that reason; this
path now refuses the same way.
A failed detail fetch was never retried. The revision is recorded when the fetch
starts, and on failure the entry stays missing, so selectedIsMissing does not
change and a terminal row's updated_at does not advance: nothing was left to
re-run the effect, and the full prompt and reply stayed unavailable until
another row was selected. The in-flight flag settling is the trigger now, and
the attempt count bounds it, because the usual failure is an entry that has aged
out of the ring buffer and will never arrive however often it is asked for.
Two holes in the previous two commits, both found by the same review round.
The per-model write queue keyed on the literal spelling, so the backfill's
legacy casing and a UI save's normalized one opened two queues for one model and
raced exactly as before. It now keys on the folded identity, which is what the
backend resolves by.
A .gguf with no recognizable quant token is labelled by its filename stem, and
v2 storage lowercases that label while the scanner probes with the filename's
own casing. Folding only recognized quant labels therefore left the migrated
entry unreachable for precisely the files that need the stem fallback. The
suffix rule now also accepts a case-insensitive match against the label the
scanner derives for that filename, which keeps an ordinary colon out because the
head still has to be a .gguf. _bare_model_id drops onto the same shared rule
rather than repeating half of it.
Saving twice quickly, or saving while the one-time backfill is still running,
started independent requests with no sequencing, so the older response could
commit last and resurrect the entry the newer one meant to replace or remove. An
API-driven load then applies context or GPU settings the user has already
changed, with nothing in the UI showing it.
Writes now chain per override key. The chain hangs off the settled tail, so a
failed write cannot cancel the next one, and only the last writer clears the
slot so a queue that is still building keeps its order. Different models still
overlap. Verified against the real module under node: two saves for one model
with the first made slow commit oldest-first and never overlap, where the
previous version committed them in the wrong order; a rejected write still lets
the next succeed; and two models still run concurrently.
The overlay is mounted from __root.tsx and imported two label helpers from the
page, so the page and its dependency graph were pulled into the eagerly loaded
bundle and the route's lazyRouteComponent bought nothing: every route paid for
the monitor page even when it was never opened. Measured on a production vite
build, the async api-monitor chunk was 0.20 kB, meaning the implementation had
landed in the main bundle.
The helpers now live in their own module. The same build gives an 18.83 kB
api-monitor chunk and a main bundle 18 kB smaller (3.9 kB gzipped).
An explicit remove ran the launch-flag validation first, so a form still
carrying a rejected flag raised a 400 and left the override in place. Nothing is
stored on that path, so there is nothing to validate; remove now short-circuits
it, which is what the branch below already claims to do.
Clear log dropped only the caller's own rows, but a lifecycle row is shared: it
is visible to everyone and owned by no one, so those rows survived and the
reload straight after the click brought them back, leaving the button visibly
ineffective. Deleting them is not an option either, since that erases another
caller's history. They are now hidden per subject, so the clear is true for that
caller and harmless to the rest. A shared row that is still running is live
state rather than history, so it stays visible, and the hidden ids are pruned
against the ring buffer so they cannot accumulate.
Three gaps the previous round left, all in the same key-resolution rule, so the
rule now lives in one place as split_quant_suffix.
A quant label may carry a bits-per-weight modifier, because two files at the
same base quant are kept distinct by it: utils/models/model_config.py preserves
IQ4_XS-3.53bpw while hub/utils/gguf.py strips it, and both forms reach the
override keys. The known-quant pattern accepted neither, so _bare_model_id
missed the bare entry and the first qualified save dropped its launch flags.
On POSIX the browser lowercases the quant but keeps the path casing, so a
migrated "/models/Foo:q4_k_m" was unreachable from the scanner's
"/models/Foo:Q4_K_M". Only the quant suffix folds now, and only when it really
is a quant, so "/models/foo:Bar.gguf" stays a distinct filename and the path
itself stays case-sensitive.
A standalone .gguf picked directly has no quant to choose between and is stored
with a null variant. The backfill filter read that as safetensors and skipped
it, and the done flag is set on the same pass, so those settings stayed
browser-only for good while auto-switch kept loading the model with defaults.
* Keep `import unsloth` working when bitsandbytes is absent
device_type.py already prints "bitsandbytes is not installed - 4bit QLoRA
unallowed, but 16bit and full finetuning works" and clears
ALLOW_BITSANDBYTES / ALLOW_PREQUANTIZED_MODELS, but the import chain then
hard-required the module anyway, so `import unsloth` raised instead.
#7354 made this reachable: the gfx906 install path uninstalls the generic
bitsandbytes wheel (no gfx906 kernels in it), which leaves an MI50 / Radeon VII
host unable to import unsloth at all, not on the 16bit path the message
promises.
- kernels/utils.py: guard the bnb import; bind get_ptr and the five 4bit ctypes
handles to a stub that raises a clear message if a 4bit path is entered.
HAS_CUDA_STREAM stays False, which is the correct route.
- save.py, models/granite.py: guard Bnb_Linear4bit and peft's Linear4bit
(peft exports it only when bnb imported cleanly) with placeholder classes.
Both names only feed isinstance checks, so nothing matching is exact.
- _gpu_init.py: same degradation on the xpu branch as the cuda branch above.
Verified on a Strix Halo (gfx1151, DEVICE_TYPE=hip, torch 2.11.0+rocm7.13.0)
by blocking bitsandbytes with sys.modules["bitsandbytes"] = None, so
find_spec returns None and the import raises exactly as when the package is
absent. Before: ModuleNotFoundError at kernels/utils.py:136. After: import
succeeds, FastLanguageModel/FastModel import, ALLOW_BITSANDBYTES=False,
ALLOW_PREQUANTIZED=False, and the 4bit stub raises with the real cause. With
bitsandbytes present, every binding is unchanged.
New test walks the `import unsloth` module graph with ast and fails on any
unguarded bitsandbytes (or peft Linear4bit) import; verified it catches the
old code. Targeted suites: 702 passed, 18 skipped.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address the review on #7502: zoo coupling, non-hip flags, py3.9 collection
Three findings, each reproduced first and negative-controlled after.
1. The fix still needed an unreleased unsloth_zoo (P1). save.py imported
unsloth_zoo.saving_utils at module scope, and any zoo without the companion
#953 fix imports bitsandbytes there, so `import unsloth` kept failing for a
dependency set pyproject.toml allows. Raising the floor was not an option:
PyPI's newest zoo is 2026.7.6 and #953 is merged but unreleased, so a bump
would break every install today. Both names it pulled in are used only inside
functions, so the import is now lazy at those two call sites, matching what
determine_base_model_source in the same file already does. Verified against a
real pre-#953 zoo checkout with bitsandbytes blocked: import succeeds, and
restoring the eager import reproduces the failure at saving_utils.py:70.
This PR no longer depends on a zoo release.
2. Capability flags were only cleared on hip (P2). device_type.py probed
bitsandbytes inside its DEVICE_TYPE == "hip" branch, so a cuda or xpu host
without bnb imported fine but still reported ALLOW_BITSANDBYTES=True, and the
default load_in_4bit=True path in models/loader.py would select a 4bit
checkpoint before failing. Clear both flags whenever the module is absent, on
every backend, via find_spec so a working install pays nothing. A cuda host
with bnb blocked now reports False/False; with bnb present nothing changes.
3. The new test could not be collected on Python 3.9 (P2). `Path | None` is a
PEP 604 union and requires-python still allows 3.9, so pytest raised
TypeError at import. Added `from __future__ import annotations`. Checked in
real uv venvs on 3.9, 3.10 and 3.13: 2 passed each; removing the future
import reproduces "unsupported operand type(s) for |" on 3.9 only.
The xpu branch in _gpu_init.py needs no separate flag handling now that the
probe is backend-independent.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address the second review on #7502: guarded probe, and 8bit in the same guard
1. The capability probe used find_spec while the fallbacks in kernels/utils.py
and _gpu_init.py treat any import failure as unavailable, so an installed but
unusable wheel would leave ALLOW_BITSANDBYTES true while the kernels had
already bound the stub. Probe with the same guarded import instead, so all
three agree by construction. No new cost on any path: _gpu_init.py already
imports bnb before device_type is reached on cuda, and device_type's own hip
block imports it a few lines later.
Worth recording that the state this prevents is currently unreachable for an
unrelated reason: a broken wheel takes `import unsloth` down earlier, in
transformers/integrations/bitsandbytes.py:20 via
unsloth_zoo/patching_utils.py:680, whichever exception it raises (OSError also
escapes the zoo moe_utils `except ImportError`). So this is correctness for
when those imports get guarded, not an observable fix today.
2. Both loader guards printed for load_in_4bit or load_in_8bit but only cleared
load_in_4bit, so an explicit load_in_8bit=True survived and reached
Transformers, which builds the bnb quantizer and fails there. Clear both. The
message no longer says AMD either: the flag now goes false whenever bnb is
unusable on any backend.
Tests: the probe must not use find_spec, and an ast walk requires every
ALLOW_BITSANDBYTES guard in loader.py to clear both flags, so a third guard
cannot be added with the same omission. Dropping either fix reddens them (1 and
2 failures respectively). 4 passed on 3.9, 3.13 and the ROCm venv; absent and
healthy bnb both stay consistent across hip and cuda.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Drop the importlib import left over from the find_spec probe on #7502
* Address the third review on #7502: exact-name bypass and a forwarded bnb config
Both findings hold up, so both are fixed.
1. use_exact_model_name=True skipped the guard entirely. load_in_4bit defaults
to True, so on a host without bitsandbytes
FastLanguageModel.from_pretrained(name, use_exact_model_name=True) kept 4bit
set and failed downstream. That option suppresses repo-name remapping and
cannot make bitsandbytes available, so it has no business gating a capability
check. Ungated at both sites.
2. A user-supplied quantization_config survived the fallback. It sets
load_in_4bit/8bit at the top of from_pretrained and stays in kwargs, so
clearing the local flags still let Transformers rebuild the bnb quantizer.
Now dropped as part of the fallback.
One correction to the second suggestion: it cannot be dropped whenever the
fallback runs. quantization_config also carries GPTQ, AWQ, fp8 and torchao
configs, which have nothing to do with bitsandbytes and must reach the loader
untouched. The pop is gated on the config actually requesting load_in_4bit or
load_in_8bit, reusing the same dict/attr probe from the top of the function.
Behaviour, exercising the real guard block against synthetic inputs with
use_exact_model_name=True and bnb unusable:
default 4bit, no cfg 4bit=False 8bit=False
explicit 8bit, no cfg 4bit=False 8bit=False
BitsAndBytesConfig(4bit/8bit) 4bit=False 8bit=False config dropped
dict bnb config 4bit=False 8bit=False config dropped
GPTQ config 4bit=False 8bit=False config SURVIVES
fp8 dict 4bit=False 8bit=False config SURVIVES
Nothing changes when bitsandbytes works: the whole block is inside
`if not ALLOW_BITSANDBYTES`.
Tests: an ast walk requires neither guard to reference use_exact_model_name in
its test, and requires each to pop quantization_config behind a _wants_bnb
check, so an unconditional pop fails too. Re-gating one guard or removing one
pop reddens a test each. 6 passed on 3.9, 3.13 and the ROCm venv.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address the fourth review on #7502: FastModel never reached the 16bit path
Both findings are real, and the second one meant this PR did not actually
deliver what it advertises for FastModel or vision loads. Reproduced first.
1. patch_compiling_bitsandbytes() ran unguarded at the top of
FastModel.from_pretrained, and unsloth_zoo's copy imports bitsandbytes
unconditionally (patching_utils.py:40). So every FastModel call on a
bnb-less host died there, whatever the arguments:
FastModel(load_in_16bit=True) -> ModuleNotFoundError at patching_utils.py:40
FastModel(full_finetuning=True) -> ModuleNotFoundError at patching_utils.py:40
The FastLanguageModel path already wraps this call in try/except with a
warning, and its comment even says "Mirror FastModel" - FastModel was the
unwrapped one. Wrapped it the same way, so behaviour is unchanged wherever
bitsandbytes imports.
2. The mode-exclusivity check ran before the capability fallback. load_in_4bit
defaults to True, so load_in_16bit=True made
int(load_in_4bit) + int(load_in_16bit) == 2 and raised "Can only load in 4bit
or 8bit or 16bit" before the fallback could clear the unavailable 4bit
request. Moved the fallback ahead of that check.
After both, the same three calls get past every bitsandbytes gate and reach
model resolution, failing only on the deliberately fake repo name used by the
probe. Nothing changes when bitsandbytes works: the fallback is still inside
`if not ALLOW_BITSANDBYTES`, and the wrapper only swallows an import that
previously crashed the load.
Tests: the mode check must be preceded by an ALLOW_BITSANDBYTES fallback in the
same function, and no call to patch_compiling_bitsandbytes may sit outside a
try. The ordering assertion is scoped to the enclosing function on purpose - my
first version compared line numbers file-wide, so the other loader's guard
satisfied it and the negative control passed when it should have failed. With
the scoping fixed, moving the fallback back after the mode check reddens it, as
does unwrapping the patch call. 8 passed on 3.9, 3.13 and the ROCm venv.
* [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>
