unsloth/tests/test_offloaded_parameter_hint.py
Daniel Han 5da63be967
Report the real cause when a save or GGUF export fails (#7861)
* Report the real cause when a save or GGUF export fails

Four related failures where saving reported something other than what
actually went wrong.

Kaggle blamed the disk for every GGUF failure. save_to_gguf is wrapped in a
bare `except Exception` that, whenever IS_KAGGLE_ENVIRONMENT is set, reported
the 20GB disk space limit no matter what failed. An unsupported architecture,
a missing tokenizer and a bad quant method all told the user to free up space.
bert_classification exposed it: ModernBERT for sequence classification has no
llama.cpp converter and cannot produce a GGUF at any size, so the notebook
reported a disk problem that did not exist. The message is now gated on
_gguf_failure_looks_like_disk, which takes either of two independent signals:
the error text or errno naming ENOSPC, or free space under 2GB on the save
directory or the cwd. Either alone is enough, because each can be absent for a
good reason. The check never raises. Both branches now chain with `from e`, so
the original traceback survives either way.

That helper originally landed in the MIDDLE of unsloth_save_pretrained_gguf
instead of after it, and this commit carries the corrected placement. The
function still parsed and still ran, but it now ended right after building the
quantization method list, so it did the 16-bit merge and returned without ever
calling save_to_gguf. Everything below the insertion point became dead code
stranded inside the helper, under its return. save_pretrained_gguf raised
nothing, printed its usual progress, left a populated merge directory behind
and produced no .gguf, which is indistinguishable from success unless you go
looking for the file. It cost a full sweep before it was noticed, and no
ordinary unit test would have caught it since both halves are valid Python and
the import kept working. tests/test_save_entrypoints_reach_converter.py checks
the structural properties directly against the AST instead: the converter call
is present, reachable, and not stranded after a return anywhere in save.py.

A save that failed on an offloaded model said nothing about the offload.
A model too large for its GPU is partially offloaded, and accelerate leaves
those parameters on the meta device with the real data in a weights_map.
Saving then walks into accelerate internals that assume the map is populated,
so the user sees "'NoneType' object is not subscriptable" or "Cannot copy out
of meta tensor; no data!". Qwen3_MoE, Qwen3_5_MoE and Nemotron-3-Nano-30B all
failed to export this way. The hint is appended to the existing error, never
substituted for it, the exception type is unchanged, and it stays empty unless
a meta parameter is really present. It caps at three parameter names. The
helper sits above the @_normalize_tied_weights_keys_for_save decorator so the
decorator still applies to unsloth_save_pretrained_gguf.

SentenceTransformer had a different save_pretrained_merged signature.
FastLanguageModel takes (save_directory, tokenizer = None, save_method =
"merged_16bit", ...) and every notebook and doc calls it positionally, but
FastSentenceTransformer bound the same public name to (self, save_directory,
**kwargs), so that call answered with "takes 2 positional arguments but 4 were
given". Five embedding notebooks hit it. Keyword callers were unaffected,
which is why it went unnoticed. Both closures that bind the name now take
tokenizer and save_method positionally with the same defaults, so every
previous keyword call behaves exactly as before. save_method is honoured
rather than accepted and dropped: the merge_and_unload path can only produce a
16-bit merge, so it refuses anything else, and refuses before writing anything
so no half-finished directory is left behind. The other path forwards to the
FastLanguageModel merge, which understands every method.

* Say when a GGUF conversion was killed for running out of host RAM

The converter is a subprocess and llama.cpp holds tensors in host RAM, so a
large model exceeds what a free Colab or Kaggle VM has. The kernel OOM-killer
takes it and subprocess reports only

    Command '[...]' died with <Signals.SIGKILL: 9>

which names no resource at all. The user's first guess is VRAM, and on Kaggle
the existing branch would have called it a disk problem, which is the wrong
advice twice over.

Gemma3N_(4B)-Audio is the case, measured today: it trains, infers and merges
cleanly (15.7GB written, 155GB disk free, freed before the conversion) and the
converter is then SIGKILLed. It happens on the high-RAM T4 as well as the plain
one, so this is not a lane that can be configured around.

SIGKILL alone is the signal: a converter that fails on its own raises and exits
non-zero, so a kill is either the OOM-killer or someone stopping the run by
hand, and both are worth naming. Checked before the Kaggle disk branch so a
SIGKILL on a fullish disk is not misreported, and chained with `from e` so the
original survives.

7 tests, including that an ordinary converter failure and a real disk failure
are both still reported as themselves.

* Honour save_method on the no-modules path and gate the inner Kaggle messages

Three follow-ups to the review of the previous two commits.

The no_modules fallback in the second _save_pretrained_merged accepted a
save_method and then dropped it: the branch strips the Unsloth-specific kwargs
and calls merge_and_unload() unconditionally, having already deleted the
adapter files save_pretrained had just written. So save_pretrained_merged(dir,
tokenizer, "lora") returned merged full weights for a request to write
adapters, silently. That path is reached whenever no modules.json is found,
which is any plain HF encoder checkpoint loaded through FastSentenceTransformer.
It now refuses before writing anything, exactly as the fast-encoder definition
above it already did. The modules.json path is unchanged and still forwards
every method to the FastLanguageModel merge.

save_method spellings now mean the same thing on every model type.
unsloth_save_model lowercases and strips spaces before validating, so
save_method = "MERGED_16BIT" and "merged 16bit" have always worked; the new
signature check compared the raw string and rejected them, which broke keyword
callers the previous commit promised were unaffected. Both closures normalize
first.

save_to_gguf's own missing-output and quantize handlers still added Kaggle's
20GB disk explanation for every failure, and they bake it into the exception
message, so the outer gate cannot recover the real cause afterwards. Both now
take the same _gguf_failure_looks_like_disk check, and the quantize branch
chains with from e like the rest. A broken llama.cpp build with 19GB free is
what that handler used to call a disk problem: see #835, where the reporter was
told about Kaggle's disk limit while not on Kaggle.

Tests: 6 new in test_st_save_merged_signature.py (28) covering the no_modules
refusal, that it refuses before merging or writing, that the forwarding path
still accepts "lora", and the case/space variants on both closures; 3 new in
test_kaggle_gguf_error_message.py (25) asserting no 20GB message is left
ungated anywhere in save.py. Each fails without its fix.

* Add the AGPL-3.0 header to the new test files

* Use this repository's Apache-2.0 header on the new test files

unsloth is Apache-2.0 (LICENSE, and pyproject license = "Apache-2.0"), and 303
of its 314 test files carry no header at all while 10 carry the Apache one. The
new files here went in with an AGPL-3.0 notice attributed to Unsloth Zoo, which
is the wrong licence, the wrong project and against the convention here.

* Recognise a shell-wrapped exit 137 as an OOM kill in the GGUF diagnostics

* Read save.py as UTF-8 in the converter reachability test

* State the free-space premise the non-disk tests are written under

_gguf_failure_looks_like_disk has a second signal independent of the message:
a filesystem under 2GiB free is a disk failure whatever the exception says. It
probes save_directory and then os.getcwd(), so on a host whose working
directory is that full, every 'this is NOT a disk problem' test inverts and
fails for a reason unrelated to the message it asserts about.

Report 99GiB rather than pinning _DISK_HEADROOM_BYTES to 0, so the real
comparison still executes and a nonsensical headroom would still be caught.

* Apply ruff kwarg-spacing formatting

pre-commit.ci's ruff-format-with-kwargs hook reported "files were modified by
this hook" and could not push the result back, so the check stayed red.
Applied locally instead.

Three source-asserting tests had to be made robust rather than re-pinned,
since the reformatting is legitimate and will happen again:

- test_the_flag_is_recorded_beside_the_dtype counted the literal
  "self._autocast_enabled = (", which the formatter collapsed onto one line.
  Now pairs each _autocast_dtype initialiser with a nearby flag assignment by
  position, so it still catches a genuinely missing one.
- test_the_original_error_is_still_reported anchored on a whole f-string
  literal, which the formatter merged with the hint that follows it. Anchors
  on the message text alone now; either shape satisfies the intent.
- test_the_source_beats_the_version_fallback exposed a real fragility rather
  than a test problem: the detector matched the literal "kw_only=True", so any
  whitespace in transformers' own source would have read an install that needs
  nothing as one that needs patching. Made whitespace-tolerant.

* Apply the repo ruff-format hook (ruff 0.6.9) to files this PR touches

* Fix MLX device type detection under UNSLOTH_ALLOW_CPU for PR #7861

get_device_type() checked the UNSLOTH_ALLOW_CPU escape hatch before the MLX
check, so an Apple Silicon host with MLX reported "cuda" and get_device_count()
then dereferenced torch, which device_type.py never imports on MLX:

  unsloth/device_type.py:99: in get_device_count
      return torch.cuda.device_count()
  E   NameError: name 'torch' is not defined

unsloth/__init__.py and unsloth_zoo.device_type both pick MLX first, so the two
also disagreed about the runtime inside one process. Non-MLX hosts are
unaffected: _IS_MLX is False there, so the ordering never applies.

* Pin the CPU-fallback env in the device helper tests for PR #7861

---------

Co-authored-by: danielhanchen <unslothshared@gmail.com>
2026-08-06 00:12:32 -07:00

159 lines
5 KiB
Python

# Copyright 2023-present Daniel Han-Chen & the Unsloth team. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""A save that fails because the model was offloaded should say so.
Offloaded parameters sit on the meta device, and saving then dies inside
accelerate with an error that names neither the model nor the offload. The
hint is appended to that error, never substituted for it, and stays empty
unless a meta parameter is really present.
"""
import sys
from pathlib import Path
import pytest
ROOT = Path(__file__).resolve().parents[1]
sys.path.insert(0, str(ROOT))
import torch # noqa: E402
from unsloth.save import _offloaded_parameter_hint # noqa: E402
class _Model:
def __init__(self, params):
self._params = params
def named_parameters(self):
return iter(self._params)
def _p(device):
return torch.nn.Parameter(torch.zeros(2, device = device), requires_grad = False)
# ---- fires when it should --------------------------------------------------
def test_a_meta_parameter_produces_a_hint():
m = _Model([("model.layers.0.mlp.down_proj.weight", _p("meta"))])
hint = _offloaded_parameter_hint(m)
assert hint
assert "meta device" in hint
def test_the_hint_names_the_offending_parameter():
"""So the reader can tell which part of the model was offloaded."""
m = _Model([("model.layers.31.mlp.experts.w1", _p("meta"))])
assert "model.layers.31.mlp.experts.w1" in _offloaded_parameter_hint(m)
def test_the_hint_states_the_remedy():
m = _Model([("a", _p("meta"))])
hint = _offloaded_parameter_hint(m)
assert "did not fit" in hint
assert "device_map" in hint or "large enough" in hint
def test_it_reports_a_few_names_not_all_of_them():
"""A 30B MoE has thousands of offloaded tensors; pasting them all would
bury the actual error."""
m = _Model([(f"layer.{i}.weight", _p("meta")) for i in range(500)])
hint = _offloaded_parameter_hint(m)
assert hint.count("layer.") <= 3
assert len(hint) < 600
def test_a_mix_of_real_and_meta_still_fires():
"""Partial offload is the normal case -- only some layers move."""
m = _Model([("good", _p("cpu")), ("bad", _p("meta"))])
assert _offloaded_parameter_hint(m)
# ---- stays silent when it should ------------------------------------------
def test_a_fully_resident_model_gets_no_hint():
"""The mislabelling risk. An unrelated save failure must not be blamed
on an offload that never happened."""
m = _Model([("a", _p("cpu")), ("b", _p("cpu"))])
assert _offloaded_parameter_hint(m) == ""
def test_a_model_with_no_parameters_gets_no_hint():
assert _offloaded_parameter_hint(_Model([])) == ""
def test_a_model_without_named_parameters_gets_no_hint():
class Odd:
pass
assert _offloaded_parameter_hint(Odd()) == ""
def test_none_gets_no_hint():
assert _offloaded_parameter_hint(None) == ""
def test_a_raising_named_parameters_gets_no_hint():
"""A diagnostic must never replace the real error with its own."""
class Boom:
def named_parameters(self):
raise RuntimeError("model is in a bad state")
assert _offloaded_parameter_hint(Boom()) == ""
def test_a_parameter_with_no_device_does_not_crash():
class NoDevice:
device = None
m = _Model([("weird", NoDevice())])
assert _offloaded_parameter_hint(m) == ""
# ---- wiring ---------------------------------------------------------------
SRC = (ROOT / "unsloth" / "save.py").read_text(encoding = "utf-8")
def test_both_save_failure_paths_use_it():
"""The GGUF export can fail at the merge step or at the plain save step,
and offloading breaks both."""
assert SRC.count("_offloaded_parameter_hint(self)") == 2
def test_the_original_error_is_still_reported():
"""The hint is added TO the error, never instead of it.
Anchored on the message text alone, not on a whole string literal: the
repo's ruff hook may merge the hint into the same f-string or split it out
again, and either shape satisfies what this is actually checking.
"""
for anchor in ("Failed to save/merge model: {e}", "Failed to save model: {e}"):
i = SRC.index(anchor)
assert "_offloaded_parameter_hint" in SRC[i : i + 200]
def test_it_still_raises_runtimeerror():
"""Callers catch RuntimeError; changing the type would break them."""
i = SRC.index("_offloaded_parameter_hint(self)")
assert "raise RuntimeError(" in SRC[max(0, i - 300) : i]
if __name__ == "__main__":
raise SystemExit(pytest.main([__file__, "-q"]))