mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-16 04:13:54 +00:00
* Keep up with transformers 5.x, TRL 0.22.x and peft
Five compatibility failures, each of which stopped a load or a run outright.
transformers 5.x turns PretrainedConfig subclasses into dataclasses. vLLM's
transformers_utils/configs/deepseek_vl2.py declares
vision_config: VisionEncoderConfig
with no default, and a dataclass will not accept a non-default field after an
inherited default one:
TypeError: non-default argument 'vision_config' follows default argument
That fires while importing vllm.transformers_utils.configs, which takes down
import vllm and, because unsloth imports vLLM, import unsloth as well. Seen in
the wild as unsloth: "ABSENT: TypeError".
PretrainedConfig.__init_subclass__ is patched rather than vLLM's source, so it
covers every affected class in any vLLM version instead of one file, and the
original stays reachable through __wrapped__: an irreversible monkey patch
cannot be tested against the failure it fixes, and cannot be undone by anyone
debugging a config problem downstream of it.
The window is narrower than it first looked. transformers 5.6.0 started passing
kw_only=True to the dataclass, which removes the ordering rule entirely, so the
fix now stands down on 5.6.0+ rather than monkey patching every config subclass
for no benefit. Read from the source rather than pinned to a version, since the
change could be backported. Verified live on three transformers: 4.57.6 (no-op),
5.5.0 (failure reproduces, fix works), 5.14.1 (upstream handles it, fix stands
down).
offload_embedding = True raised NotImplementedError on a tied-embedding model.
It is a VRAM optimisation, not a correctness switch, and on a tied model
embed_tokens IS lm_head, so offloading strands the output projection on CPU and
frees nothing. Turned off with a message instead, the way fast_inference already
degrades. Resolved before _attach_bnb_multidevice_hooks, which returns early
while the flag is still True; resolving it later would silently skip hook
attachment for exactly these models. WSL and Windows pass through unchanged,
since the offload block skips those platforms and probing there would be a code
path that never used to run.
peft imports symbols transformers no longer ships. Backfilling whole missing
modules was not enough: transformers 5.0.0.dev0 has conversion_mapping but not
_MODEL_TO_CONVERSION_PATTERN, so individual names are backfilled rather than a
real module being replaced wholesale.
On TRL 0.22.x a VLM skips dataset preparation and picks the vision collator from
_is_vlm alone. Fine-tuning a VLM on a text-only dataset then reaches the trainer
with a raw text column and transformers strips everything:
ValueError: No columns in the dataset match the model's forward method
signature ... The following columns have been ignored: [text]
Magistral_(24B)-Reasoning-Conversational fails exactly this way; it pins
trl==0.22.2 and trains a VLM on a plain text dataset. TRL 0.25.1+ fixed this by
keying off _is_vision_dataset, so that is what gets back-ported, and it no-ops
where TRL already defines the flag.
* Stop the dataclass backfill shadowing an inherited default, and say when a peft stand-in is not equivalent
Four things, all narrowing rather than widening.
_backfill_dataclass_defaults decided "no default yet" with `name not in
cls.__dict__`, which is class-local. A config subclass that re-annotates an
inherited field without assigning to it already has a default, through the MRO,
and None overwrote it. Reproduced directly: a child re-annotating a parent field
whose default is 7 came back None. dataclass reads defaults with getattr, so
hasattr is the test that matches what dataclass will do.
_backfill_missing_peft_symbols installs inert donors: an empty conversion
pattern, mapping lookups that return None. That is the truth on transformers 4
and on a transformers 5 that never had the symbol, and it is not the truth for a
transformers 5 that has conversions and renamed one, where peft would silently
skip work. Checked every released wheel from 5.0.0 to 5.6.0: all eleven names
are present, so this only fires on a dev build. It now warns for anything beyond
the pattern rather than repairing the import in silence.
The live control test observed PretrainedConfig before anything had imported
unsloth, so run on its own it had nothing to unwrap and skipped itself instead
of proving the failure is real. It installs the patch first now.
find_spec imports the parent packages, so `find_spec("trl.trainer.sft_trainer")`
raises rather than returning None when trl is absent, and the skipif marker
failed collection instead of skipping.
Tests: 4384 passed against 4377 on the same tree with these changes stashed,
same 1310 environment failures either way (version_compat and vllm_compat fetch
upstream sources per tag; the GPUs here are busy), and zero failures present on
this branch that are not also on its base.
* Load import_fixes by file spec in the dataclass backfill test
Importing unsloth.import_fixes goes through unsloth/__init__.py, which reaches
_gpu_init and pulls torch, numpy and unsloth_zoo: measured at 10.6s and 8288
modules. The helper under test is plain Python, and import_fixes.py has only
stdlib plus packaging at module level, so the file-spec load the adjacent
test_peft_symbol_backfill.py already uses works here too, at 0.01s with none of
that stack resident. A dependency-light run failed before reaching the code
under test; under a meta-path finder that raises on torch, numpy, unsloth_zoo,
unsloth, transformers, peft and trl, the three tests now pass.
* Correct four version claims in the compat comments
Comment-only. Checked against the published wheels and the commits that
introduced each change:
- kw_only=True on config dataclasses landed in 5.5.1, not 5.6.0. It was
backported: 5.5.1 on the 5.5 branch, 5.6.0 on main, both from
transformers#45139.
- The window needing the bare-annotation fix is 5.4.0 to 5.5.0. 5.0.0-5.3.0
have no PretrainedConfig.__init_subclass__ at all.
- vllm/__init__.py is lazy at every tag from 0.11.2 on, so importing vllm
never reaches transformers_utils.configs. The guard is still worth
installing early, but not for the reason the comment gave.
- _is_vision_dataset landed in TRL 0.24.0, not 0.25.1 (trl#4080). 0.24.0's
sft_trainer.py line is byte-identical to 0.25.1's, which is what the
backport's no-op guard matches on.
* Move the live kw_only threshold to 5.5.1 as well
The previous commit corrected the comments but left this test asserting the
probe returns False until 5.6.0, so it would fail on 5.5.1 through 5.5.4,
which is exactly where the fix correctly stands down. Verified against the
extracted wheels: 5.4.0 and 5.5.0 have no kw_only=True in
configuration_utils.py, 5.5.1 and 5.6.0 both do.
* Fall back to the version window when the config source cannot be read
A stripped or frozen install ships bytecode only, and inspect.getsource then
raises. Answering False there patched a 5.5.1+ install that needs nothing, and
since the backfill runs before the upstream hook, fields upstream means to be
required keyword arguments would silently gain a None default.
Falls back to 5.4.0 <= v < 5.5.1, the only window with the ordering rule and
no kw_only=True. Half open so 5.5.0.post1 and other 5.5.0 rebuilds stay
inside. Readable source still decides, since the backport means the version
alone cannot see the truth. An unreadable or unparseable version stands down:
without positive evidence the install needs the patch, the better failure is
the loud TypeError the patch would have prevented, not a config that quietly
accepts a missing required field. Uses packaging's Version directly rather
than this module's wrapper, which raises on anything its regex misses and
rewrites pre-release suffixes upwards, moving dev builds across the edge.
* 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.
* Detect the bare-annotation config rule by probing, not by version
`_transformers_needs_bare_annotation_fix` pinned the window to
`5.4.0 <= v < 5.5.1`. The rule arrived in 5.4.0 and `kw_only=True` retired it
in 5.5.1, but that landed as a backport on the 5.5 branch and on main
separately, so a version number mislabels any build that carries it early or
late, and says nothing about a future release that brings the rule back.
It now defines a throwaway subclass in exactly the shape vLLM uses, a default
followed by a bare annotation, and reports whether `__init_subclass__` raises.
Same answer on every version I checked, derived rather than tabulated:
4.57.6 False 5.4.0 True 5.5.1 False
5.3.0 False 5.5.0 True 5.6.0 False
The unreadable-source fallback keeps the same conservative default: without
positive evidence the install needs patching, answer False, since the better
failure is the loud TypeError the patch would have prevented rather than a
config that quietly accepts a missing required field.
The test that faked `transformers.__version__` to reach the fallback is
replaced. Faking a version deliberately no longer changes the answer, so that
is now asserted as the regression, alongside a test that a config whose
`__init_subclass__` raises is detected whatever it calls itself.
* Tighten the comments across the compatibility changes (#7866)
* Trim comments and docstrings across the transformers 5 / TRL compat changes
* Disable embedding offload on Windows and WSL for PR #7866
* Shorten comments and docstrings across the compat changes
* Gate the live bare-annotation test on the behaviour, not the version
---------
Co-authored-by: danielhanchen <unslothshared@gmail.com>
Co-authored-by: danielhanchen <unslothai@gmail.com>
174 lines
5.7 KiB
Python
174 lines
5.7 KiB
Python
"""_backfill_dataclass_defaults must not shadow an inherited default.
|
|
|
|
Deciding "no default yet" with `name not in cls.__dict__` was wrong: a subclass
|
|
re-annotating an inherited field already has one, via the MRO. import_fixes.py
|
|
is loaded by file spec because `import unsloth.import_fixes` would run
|
|
unsloth/__init__.py first, pulling in torch, numpy and unsloth_zoo.
|
|
"""
|
|
|
|
import importlib.util
|
|
import sys
|
|
import types
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[1]
|
|
IMPORT_FIXES = REPO_ROOT / "unsloth" / "import_fixes.py"
|
|
|
|
|
|
def _load_module():
|
|
spec = importlib.util.spec_from_file_location(
|
|
"_unsloth_import_fixes_dataclass_under_test", IMPORT_FIXES
|
|
)
|
|
module = importlib.util.module_from_spec(spec)
|
|
spec.loader.exec_module(module)
|
|
return module
|
|
|
|
|
|
_MODULE = _load_module()
|
|
_backfill_dataclass_defaults = _MODULE._backfill_dataclass_defaults
|
|
_transformers_configs_are_kw_only = _MODULE._transformers_configs_are_kw_only
|
|
ifx = _MODULE # for monkeypatching its internals
|
|
|
|
|
|
def test_an_inherited_default_is_not_shadowed():
|
|
"""Absent from cls.__dict__ but present via the MRO: None would overwrite it."""
|
|
|
|
class Base:
|
|
window: int = 7
|
|
|
|
class Child(Base):
|
|
window: int # re-annotated, no assignment
|
|
|
|
assert _backfill_dataclass_defaults(Child) == []
|
|
assert Child.window == 7
|
|
|
|
|
|
def test_a_genuinely_new_field_is_still_backfilled():
|
|
"""The narrowing must not disarm the fix."""
|
|
|
|
class Base:
|
|
window: int = 7
|
|
|
|
class Child(Base):
|
|
vision_config: object # new, and bare: the case that raises
|
|
|
|
assert _backfill_dataclass_defaults(Child) == ["vision_config"]
|
|
assert Child.vision_config is None
|
|
|
|
|
|
def test_an_inherited_method_of_the_same_name_counts_too():
|
|
"""getattr resolves through the MRO, so a method counts as a default too."""
|
|
|
|
class Base:
|
|
def helper(self):
|
|
return 1
|
|
|
|
class Child(Base):
|
|
helper: object
|
|
|
|
assert _backfill_dataclass_defaults(Child) == []
|
|
assert Child().helper() == 1
|
|
|
|
|
|
class _KwOnlyConfig:
|
|
"""Stands in for transformers 5.5.1+, whose hook passes kw_only=True."""
|
|
|
|
def __init_subclass__(cls, **kwargs):
|
|
_fake_dataclass(cls, kw_only = True)
|
|
|
|
|
|
class _OrderedConfig:
|
|
"""Stands in for 5.4.0 to 5.5.0, whose hook does not."""
|
|
|
|
def __init_subclass__(cls, **kwargs):
|
|
_fake_dataclass(cls)
|
|
|
|
|
|
def _fake_dataclass(cls, **kwargs):
|
|
return cls
|
|
|
|
|
|
def _config_without_readable_source():
|
|
"""A hook whose source cannot be read, as on a stripped or frozen install:
|
|
compiled under a filename not on disk, so `inspect.getsource` fails."""
|
|
namespace = {}
|
|
exec(
|
|
compile(
|
|
"class Config:\n def __init_subclass__(cls, **kwargs):\n pass\n",
|
|
"<unsloth-test-no-source-on-disk>",
|
|
"exec",
|
|
),
|
|
namespace,
|
|
)
|
|
return namespace["Config"]
|
|
|
|
|
|
def _pretend_transformers_is(monkeypatch, version):
|
|
module = types.ModuleType("transformers")
|
|
module.__version__ = version
|
|
monkeypatch.setitem(sys.modules, "transformers", module)
|
|
|
|
|
|
def test_the_source_beats_the_version_fallback(monkeypatch):
|
|
"""The change was backported, so a readable source decides over the version."""
|
|
_pretend_transformers_is(monkeypatch, "5.4.0")
|
|
assert _transformers_configs_are_kw_only(_KwOnlyConfig) is True
|
|
_pretend_transformers_is(monkeypatch, "5.5.1")
|
|
assert _transformers_configs_are_kw_only(_OrderedConfig) is False
|
|
|
|
|
|
def test_unreadable_source_on_5_5_1_stands_down(monkeypatch):
|
|
"""False here would give required keyword fields a None default on 5.5.1+."""
|
|
_pretend_transformers_is(monkeypatch, "5.5.1")
|
|
assert _transformers_configs_are_kw_only(_config_without_readable_source())
|
|
|
|
|
|
def test_unreadable_source_falls_back_to_probing_the_behaviour(monkeypatch):
|
|
"""With no source to read, ask the installed transformers by trying it."""
|
|
monkeypatch.setattr(
|
|
ifx,
|
|
"_transformers_needs_bare_annotation_fix",
|
|
lambda: True,
|
|
)
|
|
assert not _transformers_configs_are_kw_only(_config_without_readable_source())
|
|
monkeypatch.setattr(
|
|
ifx,
|
|
"_transformers_needs_bare_annotation_fix",
|
|
lambda: False,
|
|
)
|
|
assert _transformers_configs_are_kw_only(_config_without_readable_source())
|
|
|
|
|
|
def test_the_probe_answers_from_the_class_not_the_version(monkeypatch):
|
|
"""Faking `transformers.__version__` must not change the answer."""
|
|
for version in ("5.4.0", "5.5.0", "5.5.0.post1", "4.57.6", "9.9.9"):
|
|
_pretend_transformers_is(monkeypatch, version)
|
|
assert ifx._transformers_needs_bare_annotation_fix() is False, version
|
|
|
|
|
|
def test_the_probe_detects_a_config_that_raises(monkeypatch):
|
|
"""An `__init_subclass__` rejecting the shape is what the fix exists for."""
|
|
|
|
class Raising:
|
|
def __init_subclass__(cls, **kwargs):
|
|
raise TypeError("non-default argument follows default argument")
|
|
|
|
module = types.ModuleType("transformers.configuration_utils")
|
|
module.PretrainedConfig = Raising
|
|
monkeypatch.setitem(sys.modules, "transformers.configuration_utils", module)
|
|
assert ifx._transformers_needs_bare_annotation_fix() is True
|
|
|
|
|
|
def test_an_unreadable_version_does_not_raise(monkeypatch):
|
|
"""A version packaging cannot parse, or no transformers at all, must not raise."""
|
|
for version in ("not-a-version", "", None):
|
|
_pretend_transformers_is(monkeypatch, version)
|
|
assert _transformers_configs_are_kw_only(_config_without_readable_source()), repr(version)
|
|
monkeypatch.setitem(sys.modules, "transformers", None)
|
|
assert _transformers_configs_are_kw_only(_config_without_readable_source())
|
|
|
|
|
|
if __name__ == "__main__":
|
|
raise SystemExit(pytest.main([__file__, "-q"]))
|