chore: trim verbose comments added in PR #5416 (commit 12295c1f) (#5418)

Strictly comment / docstring trims. AST-verified against 12295c1f via
scripts/verify_trim_comment_only.py:

* unsloth/import_fixes.py: collapse the 32-line peft+transformers-4.x
  drift header to 10 lines; remove redundant per-stub docstrings and
  per-step numbered comments inside fix_peft_transformers_weight_
  conversion_import; keep one-line docstrings on helpers + on the
  public entry-point.
* unsloth/_gpu_init.py: collapse the 8-line preamble above
  fix_peft_transformers_weight_conversion_import() to 4 lines.
* tests/conftest.py: collapse the 13-line block comment above
  _apply_unsloth_peft_import_fix_for_tests to 5 lines; tighten three
  internal comments.
This commit is contained in:
Daniel Han 2026-05-14 04:25:23 -07:00 committed by GitHub
commit 6994d07f90
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 59 additions and 210 deletions

View file

@ -142,18 +142,10 @@ if not _has_real_accelerator():
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Apply unsloth-local upstream-drift fixes that need to run before pytest # Apply the peft + transformers-4.x stub-injection fix before pytest collects
# collects tests that import the affected third-party module directly. # tests that import peft.utils.transformers_weight_conversion. Production runs
# # this via unsloth/_gpu_init.py, but the GPU-free harness above skips full
# Specifically: ``from peft.utils import transformers_weight_conversion`` # package init, so we load just the standalone import-fixes module by path.
# blows up on (peft 0.19.x + transformers 4.x) because peft unconditionally
# imports two transformers-v5 submodules at module top. The production
# import path applies the stub-injection workaround via
# ``unsloth/_gpu_init.py``, but the GPU-free test harness above
# deliberately avoids triggering the full ``unsloth`` package init (which
# pulls in the CUDA / torch device chain). Load just the standalone
# import-fixes module by file path so drift detectors that probe peft
# see the same patched state a real unsloth install would.
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@ -178,11 +170,9 @@ def _apply_unsloth_peft_import_fix_for_tests() -> None:
if mod_name in sys.modules: if mod_name in sys.modules:
mod = sys.modules[mod_name] mod = sys.modules[mod_name]
else: else:
# Submodule import requires SOME parent ``unsloth`` entry in # Submodule import needs SOME parent ``unsloth`` entry; reuse or
# sys.modules. Reuse one if a sibling conftest step already # install a bare skeleton and pop on exit so later ``import unsloth``
# installed it (and don't pop in that case); otherwise install a # calls hit the real package init.
# bare skeleton and pop on the way out so subsequent
# ``import unsloth`` calls hit the real package init.
if "unsloth" not in sys.modules: if "unsloth" not in sys.modules:
pkg = types.ModuleType("unsloth") pkg = types.ModuleType("unsloth")
pkg.__path__ = list(pkg_spec.submodule_search_locations) pkg.__path__ = list(pkg_spec.submodule_search_locations)
@ -217,16 +207,11 @@ def _apply_unsloth_peft_import_fix_for_tests() -> None:
try: try:
fix() fix()
except Exception: except Exception:
# Individual fix is internally guarded; if the entry point itself # Individual fix is internally guarded; don't take pytest down.
# blows up, don't take pytest collection down.
pass pass
finally: finally:
# Drop our scratch skeleton so subsequent ``import unsloth`` # Drop scratch skeleton; import_fixes itself stays cached as
# calls hit the real package init rather than our empty # ``unsloth.import_fixes`` without an active parent.
# placeholder. The import-fixes module itself stays in
# sys.modules under ``unsloth.import_fixes`` -- python's import
# machinery is happy to find a submodule without an active
# parent entry.
if _installed_skeleton: if _installed_skeleton:
sys.modules.pop("unsloth", None) sys.modules.pop("unsloth", None)

View file

@ -178,14 +178,10 @@ patch_vllm_for_notebooks()
patch_torchcodec_audio_decoder() patch_torchcodec_audio_decoder()
disable_torchcodec_if_broken() disable_torchcodec_if_broken()
disable_broken_wandb() disable_broken_wandb()
# Must run BEFORE patch_peft_weight_converter_compatibility: on peft 0.19.x # Must run before patch_peft_weight_converter_compatibility -- stubs the
# + transformers 4.x, ``from peft.utils import transformers_weight_conversion`` # transformers v5 submodules peft 0.19.x imports unconditionally, so the
# raises ModuleNotFoundError because peft unconditionally imports # next patch can actually wrap build_peft_weight_mapping instead of being
# ``transformers.conversion_mapping`` and ``transformers.core_model_loading`` # swallowed by its bare ImportError except.
# at module top, but neither exists on transformers <5. Stubbing those two
# submodules first lets the converter compat patch actually wrap
# ``build_peft_weight_mapping`` instead of silently no-opping in its bare
# ``except (ImportError, AttributeError): return``.
fix_peft_transformers_weight_conversion_import() fix_peft_transformers_weight_conversion_import()
patch_peft_weight_converter_compatibility() patch_peft_weight_converter_compatibility()

View file

@ -1373,55 +1373,25 @@ def disable_broken_wandb():
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# peft 0.19.x + transformers 4.x drift # peft 0.19.x + transformers 4.x drift
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# # peft 0.19.x's ``peft/utils/transformers_weight_conversion.py`` unconditionally
# peft 0.19.x ships ``peft/utils/transformers_weight_conversion.py`` with a # imports ``transformers.conversion_mapping`` and ``transformers.core_model_loading``
# top-of-file ``from transformers.conversion_mapping import ...`` AND a # at module top. Neither submodule exists on transformers <5, so the import
# ``from transformers.core_model_loading import ...``. Neither submodule # explodes with ModuleNotFoundError -- silently swallowed by the bare except
# exists on transformers < 5.x. The peft module's own header is explicit # in ``patch_peft_weight_converter_compatibility`` below. Fix: when (and only
# ("don't import from this module unless transformers v5+ is used"), and # when) the import is broken, stub the two missing submodules with the symbols
# peft itself only triggers the import at RUNTIME inside an # peft pulls at module top. The stubs are inert at runtime because peft itself
# ``if is_transformers_ge_v5:`` branch (``peft/tuners/tuners_utils.py``). # only calls into them behind ``if is_transformers_ge_v5:`` gates.
# However any code that does the obvious
# ``from peft.utils import transformers_weight_conversion`` -- including
# Unsloth's own ``patch_peft_weight_converter_compatibility`` below
# (which wraps ``build_peft_weight_mapping``) -- still tries to import the
# module unconditionally and explodes with
#
# ModuleNotFoundError: No module named 'transformers.conversion_mapping'
#
# on the 4.x stack. The bare ``except (ImportError, AttributeError)`` guard
# inside ``patch_peft_weight_converter_compatibility`` then catches that
# and silently no-ops, leaving downstream consumers to crash later with
# the same ModuleNotFoundError the first time anything imports
# ``peft.utils.transformers_weight_conversion``.
#
# Fix: when (and only when) the import is broken AND the underlying
# transformers really is missing those two submodules, inject minimal stub
# modules into ``sys.modules`` with exactly the symbols peft pulls in at
# its module top. The stubs are dead inert on transformers 4.x because
# peft never calls into them on that branch (its own ``is_transformers_ge_v5``
# gate keeps them unreachable at runtime).
#
# On transformers v5+, both submodules exist for real, this function is a
# strict no-op (the first-import probe passes and we return immediately)
# and we never touch ``sys.modules``.
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Sentinel attribute stamped on stub modules so a second call is a strict # Stamped on stub modules so a second call is a strict no-op and so third
# no-op and so third parties can introspect ``__unsloth_stub__`` to detect # parties can introspect ``__unsloth_stub__`` to detect our patch.
# our patch.
_UNSLOTH_STUB_SENTINEL = "__unsloth_stub__" _UNSLOTH_STUB_SENTINEL = "__unsloth_stub__"
def _peft_stub_module_importable(name): def _peft_stub_module_importable(name):
"""True iff ``import {name}`` would succeed without ImportError. """True iff ``import {name}`` would succeed without side effects."""
Uses ``find_spec`` rather than a raw ``import`` to avoid triggering
arbitrary module-level side effects when we're only probing. Also
treats an already-cached ``sys.modules`` entry as importable.
"""
if name in sys.modules and sys.modules[name] is not None: if name in sys.modules and sys.modules[name] is not None:
return True return True
try: try:
@ -1431,7 +1401,6 @@ def _peft_stub_module_importable(name):
def _make_peft_stub_module(fullname): def _make_peft_stub_module(fullname):
"""Create a fresh stub module marked with our sentinel."""
import types as _types import types as _types
mod = _types.ModuleType(fullname) mod = _types.ModuleType(fullname)
@ -1442,25 +1411,7 @@ def _make_peft_stub_module(fullname):
def _install_transformers_conversion_mapping_stub(): def _install_transformers_conversion_mapping_stub():
"""Synthesise a ``transformers.conversion_mapping`` module. """Stub the 3 symbols peft 0.19.x imports from this module at top level."""
Provides exactly the three symbols peft 0.19.x imports at module top:
* ``_MODEL_TO_CONVERSION_PATTERN`` -- a real ``dict`` (peft calls
``.copy()`` on it at module top and then does keyed assignment).
* ``get_checkpoint_conversion_mapping(model_type)`` -- returns
``None`` (i.e. "no v5 conversion registered for this model type").
peft only invokes this at runtime inside
``convert_peft_config_for_transformers`` /
``convert_peft_adapter_state_dict_for_transformers``, and both
early-return on ``None``.
* ``get_model_conversion_mapping(model)`` -- returns ``None``. Same
runtime guard story.
On transformers 4.x peft's own gate (``is_transformers_ge_v5``) means
these callables never actually fire, but we make them well-behaved
just in case some caller invokes them directly.
"""
name = "transformers.conversion_mapping" name = "transformers.conversion_mapping"
existing = sys.modules.get(name) existing = sys.modules.get(name)
if existing is not None and getattr(existing, _UNSLOTH_STUB_SENTINEL, False): if existing is not None and getattr(existing, _UNSLOTH_STUB_SENTINEL, False):
@ -1468,57 +1419,38 @@ def _install_transformers_conversion_mapping_stub():
mod = _make_peft_stub_module(name) mod = _make_peft_stub_module(name)
# peft does ``_MODEL_TO_CONVERSION_PATTERN = _MODEL_TO_CONVERSION_PATTERN.copy()`` # peft does ``.copy()`` + keyed assignment at module top; real dict suffices.
# at module top, then keyed assignment. A real dict is sufficient.
mod._MODEL_TO_CONVERSION_PATTERN = {} mod._MODEL_TO_CONVERSION_PATTERN = {}
def get_checkpoint_conversion_mapping(model_type, *args, **kwargs): def get_checkpoint_conversion_mapping(model_type, *args, **kwargs):
# ``None`` is peft's "no conversion registered" sentinel; both # ``None`` = peft's "no conversion registered"; both callsites
# callsites in peft early-return on it. # early-return on it.
return None return None
def get_model_conversion_mapping(model, *args, **kwargs): def get_model_conversion_mapping(model, *args, **kwargs):
# Same story: peft treats ``None`` / empty list as "nothing to do".
return None return None
mod.get_checkpoint_conversion_mapping = get_checkpoint_conversion_mapping mod.get_checkpoint_conversion_mapping = get_checkpoint_conversion_mapping
mod.get_model_conversion_mapping = get_model_conversion_mapping mod.get_model_conversion_mapping = get_model_conversion_mapping
sys.modules[name] = mod sys.modules[name] = mod
# Attach to the parent package as well so ``import transformers; # Attach to parent so attribute-style access matches a real submodule.
# transformers.conversion_mapping`` works just like a real submodule.
parent = sys.modules.get("transformers") parent = sys.modules.get("transformers")
if parent is not None and not hasattr(parent, "conversion_mapping"): if parent is not None and not hasattr(parent, "conversion_mapping"):
try: try:
parent.conversion_mapping = mod parent.conversion_mapping = mod
except Exception: except Exception:
# Defensive: a frozen / read-only parent still leaves the # Frozen parent: sys.modules entry is enough for ``from ... import``.
# sys.modules entry in place, which is enough for
# ``from transformers.conversion_mapping import ...``.
pass pass
return mod return mod
def _install_transformers_core_model_loading_stub(): def _install_transformers_core_model_loading_stub():
"""Synthesise a ``transformers.core_model_loading`` module. """Stub the 8 symbols peft 0.19.x imports from this module at top level.
Provides the eight symbols peft 0.19.x imports at module top: ``Concatenate`` and ``ConversionOps`` MUST be real classes (peft
subclasses them at module top); the rest only appear in runtime
Classes: ``ConversionOps``, ``Concatenate``, ``MergeModulelist``, ``isinstance`` / construction calls gated behind ``is_transformers_ge_v5``."""
``Transpose``, ``WeightConverter``, ``WeightRenaming``.
Callables: ``dot_natural_key``, ``rename_source_key``.
Peft subclasses ``Concatenate`` and ``ConversionOps`` at module top
(``PeftConcatenate``, ``FlattenDims``, ``PermuteDims``), so those two
MUST be real classes -- not callables, not ``object()`` -- or class
creation will fail at import. The remaining classes only appear in
``isinstance`` checks / runtime construction calls that are gated
behind ``is_transformers_ge_v5`` upstream and never fire on the 4.x
branch, but we still make them real classes so any third party that
does ``from transformers.core_model_loading import WeightConverter``
after this patch sees a sensible (if inert) class.
"""
name = "transformers.core_model_loading" name = "transformers.core_model_loading"
existing = sys.modules.get(name) existing = sys.modules.get(name)
if existing is not None and getattr(existing, _UNSLOTH_STUB_SENTINEL, False): if existing is not None and getattr(existing, _UNSLOTH_STUB_SENTINEL, False):
@ -1527,8 +1459,6 @@ def _install_transformers_core_model_loading_stub():
mod = _make_peft_stub_module(name) mod = _make_peft_stub_module(name)
class ConversionOps: class ConversionOps:
"""Stub base class. Subclassing is permitted (peft does this)."""
def convert(self, *args, **kwargs): # pragma: no cover - inert stub def convert(self, *args, **kwargs): # pragma: no cover - inert stub
raise NotImplementedError( raise NotImplementedError(
"unsloth stub: transformers.core_model_loading.ConversionOps " "unsloth stub: transformers.core_model_loading.ConversionOps "
@ -1541,35 +1471,25 @@ def _install_transformers_core_model_loading_stub():
raise NotImplementedError raise NotImplementedError
class Concatenate(ConversionOps): class Concatenate(ConversionOps):
"""Stub. Peft subclasses this as ``PeftConcatenate``."""
def __init__(self, dim = 0, *args, **kwargs): def __init__(self, dim = 0, *args, **kwargs):
self.dim = dim self.dim = dim
class MergeModulelist(ConversionOps): class MergeModulelist(ConversionOps):
"""Stub. Peft only uses this for ``isinstance(op, MergeModulelist)``."""
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
pass pass
class Transpose(ConversionOps): class Transpose(ConversionOps):
"""Stub. Peft instantiates ``Transpose(dim0=0, dim1=1)`` at runtime."""
def __init__(self, dim0 = 0, dim1 = 1, *args, **kwargs): def __init__(self, dim0 = 0, dim1 = 1, *args, **kwargs):
self.dim0 = dim0 self.dim0 = dim0
self.dim1 = dim1 self.dim1 = dim1
class WeightConverter: class WeightConverter:
"""Stub. Peft uses for ``isinstance`` and runtime construction."""
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
# Accept any signature: peft's real upstream class evolves. # Accept any signature; upstream class evolves.
self.args = args self.args = args
self.kwargs = kwargs self.kwargs = kwargs
class WeightRenaming: class WeightRenaming:
"""Stub. Peft instantiates ``WeightRenaming(source, target)``."""
def __init__( def __init__(
self, self,
source_patterns = None, source_patterns = None,
@ -1577,16 +1497,13 @@ def _install_transformers_core_model_loading_stub():
*args, *args,
**kwargs, **kwargs,
): ):
# Support both positional and keyword forms.
self.source_patterns = source_patterns self.source_patterns = source_patterns
self.target_patterns = target_patterns self.target_patterns = target_patterns
def dot_natural_key(key): def dot_natural_key(key):
"""Stub key function. Peft only calls this inside a v5-gated path."""
return key return key
def rename_source_key(original_key, renamings, converters): def rename_source_key(original_key, renamings, converters):
"""Stub. Returns ``(original_key, None)`` -- v5-gated upstream."""
return original_key, None return original_key, None
mod.ConversionOps = ConversionOps mod.ConversionOps = ConversionOps
@ -1609,64 +1526,28 @@ def _install_transformers_core_model_loading_stub():
def fix_peft_transformers_weight_conversion_import(): def fix_peft_transformers_weight_conversion_import():
"""Make ``from peft.utils import transformers_weight_conversion`` work. """Make ``from peft.utils import transformers_weight_conversion`` import
cleanly on (peft 0.19.x, transformers 4.x) by stubbing the two missing
transformers-v5 submodules. See header block above for details.
On any (peft 0.19.x, transformers 4.x) pair the import otherwise fails Must run BEFORE ``patch_peft_weight_converter_compatibility`` -- that
with ``ModuleNotFoundError: No module named 'transformers.conversion_mapping'`` function's bare ``except (ImportError, AttributeError): return`` would
because the peft module unconditionally imports two transformers v5 otherwise silently no-op.
submodules even though peft itself only USES them inside an
``if is_transformers_ge_v5:`` branch. See the block comment above for
details.
Must run BEFORE ``patch_peft_weight_converter_compatibility``: the No-op if peft / transformers missing, or if the peft module already
latter wraps ``twc.build_peft_weight_mapping`` and its bare imports cleanly. Idempotent and strictly additive (never overwrites a
``except (ImportError, AttributeError): return`` would silently real ``transformers.conversion_mapping`` / ``core_model_loading``).
no-op on the unfixed import, leaving downstream consumers to crash
later with the same ModuleNotFoundError.
Gating contract: Returns True if patched, False if no action needed, None if peft absent."""
* No-op if ``peft`` is not installed.
* No-op if ``transformers`` is not installed (unfixable -- the
real symptom would be a different ImportError on the very
first ``import peft``).
* No-op if ``peft.utils.transformers_weight_conversion`` already
imports cleanly (transformers v5+, or a peft fork that uses
non-v5 paths).
* Idempotent: a second call sees our sentinel-stamped stubs and
returns immediately.
* Strictly additive: only installs a stub for a transformers
submodule that is currently MISSING. We never overwrite a real
``transformers.conversion_mapping`` /
``transformers.core_model_loading`` module on transformers v5+.
Forwards / backwards compatibility:
* transformers 4.57.x (no submodule) -> install stubs.
* transformers 5.x (real submodule) -> first-import succeeds, return.
* TRL 0.22 / 0.27 / 1.x -- these don't import either submodule
directly; they reach the peft conversion module (if at all)
through ``peft.tuners.tuners_utils``, behind peft's own
``is_transformers_ge_v5`` gate. Our stubs are therefore
unreachable from TRL on a 4.x install, and on a 5.x install the
real submodules win the import race against our patch.
Returns ``True`` if the patch was applied (or had been applied
previously), ``False`` if no action was needed, ``None`` if peft is
not installed.
"""
# 1. Cheap exit: no peft installed.
if importlib.util.find_spec("peft") is None: if importlib.util.find_spec("peft") is None:
return None return None
# 2. Cheap exit: peft.utils.transformers_weight_conversion already # Already importable? Either we patched, or transformers is v5+.
# importable -- either we already stubbed and re-imported, or
# transformers is v5+ with real submodules. Try once and return
# on success.
try: try:
importlib.import_module("peft.utils.transformers_weight_conversion") importlib.import_module("peft.utils.transformers_weight_conversion")
return False return False
except ModuleNotFoundError as exc: except ModuleNotFoundError as exc:
# Only act on our specific drift class. Anything else surfaces # Only act on our specific drift class.
# the original exception on the next import attempt.
missing = getattr(exc, "name", "") or "" missing = getattr(exc, "name", "") or ""
if missing not in ( if missing not in (
"transformers.conversion_mapping", "transformers.conversion_mapping",
@ -1674,8 +1555,7 @@ def fix_peft_transformers_weight_conversion_import():
): ):
return False return False
except ImportError as exc: except ImportError as exc:
# Older Python only raises ImportError without `.name`, so also # Older Python ImportError has no `.name`; string-match instead.
# string-match the message for our specific drift.
msg = str(exc) msg = str(exc)
if ( if (
"transformers.conversion_mapping" not in msg "transformers.conversion_mapping" not in msg
@ -1683,9 +1563,7 @@ def fix_peft_transformers_weight_conversion_import():
): ):
return False return False
# 3. Confirm transformers is loaded; if not, try to load it so our # Need transformers loaded to attach stubs to its package.
# stub modules can be attached to the parent package. If that
# fails the user's stack is too broken for us to repair.
transformers_root = sys.modules.get("transformers") transformers_root = sys.modules.get("transformers")
if transformers_root is None: if transformers_root is None:
try: try:
@ -1693,9 +1571,7 @@ def fix_peft_transformers_weight_conversion_import():
except Exception: except Exception:
return False return False
# 4. Stub only the submodules that are genuinely missing. We do NOT # Stub only the genuinely missing submodules; never clobber real ones.
# stub a module that already exists for real -- that would
# clobber correct behaviour on transformers v5+.
patched_any = False patched_any = False
if not _peft_stub_module_importable("transformers.conversion_mapping"): if not _peft_stub_module_importable("transformers.conversion_mapping"):
_install_transformers_conversion_mapping_stub() _install_transformers_conversion_mapping_stub()
@ -1706,26 +1582,18 @@ def fix_peft_transformers_weight_conversion_import():
patched_any = True patched_any = True
if not patched_any: if not patched_any:
# Both real submodules already exist -- ``transformers_weight_conversion`` # Real submodules present; failure was for some other reason.
# must have failed for some other reason. Bail; the next import
# attempt will surface the original exception unchanged.
return False return False
# 5. Force the peft module through a fresh import now that the # Force a fresh import now that stubs are in place. Drop any cached
# stubs are in place. If a previous failed import left a ``None`` # ``None`` entry first so importlib retries.
# cache entry in ``sys.modules`` we have to drop it so importlib
# will retry.
pkg = "peft.utils.transformers_weight_conversion" pkg = "peft.utils.transformers_weight_conversion"
if pkg in sys.modules and sys.modules[pkg] is None: if pkg in sys.modules and sys.modules[pkg] is None:
del sys.modules[pkg] del sys.modules[pkg]
try: try:
importlib.import_module(pkg) importlib.import_module(pkg)
except Exception: except Exception:
# If even with the stub the module won't import (some other # Other upstream drift; stubs stay installed so a later retry succeeds.
# upstream API drift) we swallow. Callers using
# ``try / except (ImportError, AttributeError)`` will take over.
# Crucially the stubs stay installed so the NEXT import attempt
# (after whatever transient condition clears) still succeeds.
return True return True
logger.info( logger.info(