import_fixes: stub transformers.conversion_mapping so peft 0.19.x imports on transformers 4.x#5416
Conversation
…orts on transformers 4.x
patch_peft_weight_converter_compatibility currently opens with
try:
from peft.utils import transformers_weight_conversion as twc
except (ImportError, AttributeError):
return
which silently no-ops on (peft 0.19.x, transformers 4.57.x): peft's
transformers_weight_conversion module unconditionally imports two
transformers-v5 submodules at module top
from transformers.conversion_mapping import ...
from transformers.core_model_loading import ...
and neither submodule exists on transformers < 5. peft itself only USES
those submodules inside an is_transformers_ge_v5 branch, but the top of
file import still explodes with
ModuleNotFoundError: No module named 'transformers.conversion_mapping'
The bare except above swallows that, so the weight converter compat
wrap never gets installed, and any downstream code that later does
from peft.utils import transformers_weight_conversion crashes with the
same ModuleNotFoundError.
Fix: synthesise minimal stub modules for transformers.conversion_mapping
and transformers.core_model_loading, install them into sys.modules, and
re-import peft.utils.transformers_weight_conversion so the kwargs compat
wrap can succeed on top. The stubs expose exactly the symbols peft 0.19.x
pulls in at module top (Concatenate / ConversionOps are real subclassable
classes since peft subclasses them as PeftConcatenate / FlattenDims /
PermuteDims), so peft's own class creation succeeds. None of the stubbed
callables actually fire on the 4.x branch because peft's runtime
is_transformers_ge_v5 gate keeps them unreachable.
Gating contract (strict no-op outside the (peft 0.19.x, transformers 4.x)
combination):
* No-op if peft is not installed.
* No-op if peft.utils.transformers_weight_conversion already imports
clean (transformers v5+, or any peft fork off the v5 path).
* Strictly additive: only stubs submodules that are currently missing
from sys.modules / find_spec. We never overwrite the real
transformers.conversion_mapping / transformers.core_model_loading
on transformers v5+.
* Idempotent: sentinel attribute (__unsloth_stub__) on the stub modules
makes a second call return False, a third call return False, etc.
* Surfaces drift unchanged: if peft fails for some reason OTHER than
these two specific missing submodules, the original ImportError is
left for the caller's own try/except to take over.
Forwards / backwards compatibility:
* transformers 4.57.6 -> install stubs.
* transformers 5.x (real submodules) -> first-import probe succeeds,
return False, never touch sys.modules.
* TRL 0.22 / 0.27 / 1.x -- none of these 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. Stubs are therefore unreachable from TRL on a 4.x install,
and on a 5.x install the real submodules win the import race.
* peft 0.18 / 0.19 / 0.20 -- the symbols stubbed cover the union of
what peft pulls at module top across the 0.19.x line; older peft
that doesn't import the v5 submodules at all hits the cheap
first-import-probe exit and we never touch sys.modules.
Wired into unsloth/_gpu_init.py to run BEFORE
patch_peft_weight_converter_compatibility (otherwise that function's
bare except would still silently no-op). Mirrors the equivalent fix
shipped in unsloth-zoo (the zoo-side stub installs itself via
apply_import_fixes() at zoo import time, but a user can run
unsloth without the zoo fix on an older unsloth_zoo, so the unsloth
side needs to own its own copy of the workaround).
tests/conftest.py is updated to pre-apply this specific fix via the
standalone import-fixes module so the GPU-free drift detector test
(tests/test_import_fixes_drift.py::test_peft_transformers_weight_conversion_importable_and_signature)
sees the same patched state that a real ``import unsloth`` would.
The pattern mirrors unsloth-zoo's tests/conftest.py
_apply_zoo_import_fixes_for_tests helper, scoped to just the peft fix.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c06ab2c3ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # transformers is v5+ with real submodules. Try once and return | ||
| # on success. | ||
| try: | ||
| importlib.import_module("peft.utils.transformers_weight_conversion") |
There was a problem hiding this comment.
Preserve the guarded AttributeError fallback
In environments where peft.utils.transformers_weight_conversion raises an AttributeError during import because of another upstream drift, import unsloth now fails here before reaching patch_peft_weight_converter_compatibility(), even though that existing patch deliberately catches AttributeError and no-ops. Please treat non-target AttributeErrors the same as before, e.g. catch it in this probe and return False, so this new workaround only changes the specific missing-transformers-submodule case.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request addresses a compatibility issue between peft 0.19.x and transformers 4.x by stubbing missing transformers submodules (conversion_mapping and core_model_loading) required for peft imports. The fix is implemented in unsloth/import_fixes.py and integrated into both the GPU initialization process and the test suite configuration. Review feedback highlights a discrepancy in the fix_peft_transformers_weight_conversion_import docstring regarding its return value when the patch is already applied. Additionally, it is recommended to dynamically generate the log message to accurately report which specific modules were stubbed.
| Returns ``True`` if the patch was applied (or had been applied | ||
| previously), ``False`` if no action was needed, ``None`` if peft is | ||
| not installed. |
There was a problem hiding this comment.
The docstring states that the function returns True if the patch "had been applied previously". However, the implementation at line 1666 returns False if the module already imports cleanly (which is the case if the patch was applied in a previous call). This contradiction should be resolved by updating the docstring to reflect that it returns False if no action was taken in the current call.
| logger.info( | ||
| "Unsloth: stubbed transformers.conversion_mapping / " | ||
| "transformers.core_model_loading so peft.utils." | ||
| "transformers_weight_conversion imports cleanly on " | ||
| "transformers <5." | ||
| ) |
There was a problem hiding this comment.
The log message unconditionally states that both transformers.conversion_mapping and transformers.core_model_loading were stubbed. However, the logic in step 4 (lines 1700-1706) allows for stubbing only one of them if the other is already present. It would be more accurate to dynamically generate the message based on which modules were actually patched, or use a more general phrasing.
if patched_any:
stubbed_names = []
if sys.modules.get("transformers.conversion_mapping").__dict__.get(_UNSLOTH_STUB_SENTINEL):
stubbed_names.append("transformers.conversion_mapping")
if sys.modules.get("transformers.core_model_loading").__dict__.get(_UNSLOTH_STUB_SENTINEL):
stubbed_names.append("transformers.core_model_loading")
logger.info(
f"Unsloth: stubbed {' / '.join(stubbed_names)} so peft.utils."
"transformers_weight_conversion imports cleanly on "
"transformers <5."
)References
- User-facing warning messages should be dynamically generated to include the specific configuration values they refer to, rather than using hardcoded examples, to ensure accuracy and avoid confusion.
|
FYI - I just opened unslothai/unsloth-zoo#639 to remove The zoo PR's |
…) (unslothai#5418) Strictly comment / docstring trims. AST-verified against 12295c1 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.
What's broken
On the (peft 0.19.x, transformers 4.57.x) combination,
raises
because peft's
transformers_weight_conversionmodule unconditionally imports two transformers-v5 submodules at module top (transformers.conversion_mappingandtransformers.core_model_loading), neither of which exists on transformers < 5. peft itself only USES those submodules inside anis_transformers_ge_v5branch, but the top-of-file import still explodes the moment anything tries to load the module.Our existing
patch_peft_weight_converter_compatibility(unsloth/import_fixes.pylines 1375-1454) opens withso the bare except silently no-ops, the kwargs compat wrap never gets installed, and any downstream code that later does
from peft.utils import transformers_weight_conversionblows up with the sameModuleNotFoundError.What this PR does
Adds
fix_peft_transformers_weight_conversion_importtounsloth/import_fixes.py. When (and only when) the import is currently broken AND the underlying transformers really is missing those two submodules, it injects minimal stub modules intosys.modulesexposing the symbols peft pulls in at module top. Then it forces a fresh import ofpeft.utils.transformers_weight_conversionso the existingpatch_peft_weight_converter_compatibility(the kwargs compat wrap) can run on top.ConcatenateandConversionOpsare real subclassable classes since peft subclasses them asPeftConcatenate/FlattenDims/PermuteDimsat module top, so peft's own class creation succeeds. None of the stubbed callables actually fire on the 4.x branch because peft's runtimeis_transformers_ge_v5gate keeps them unreachable.Wired into
unsloth/_gpu_init.pyto run BEFOREpatch_peft_weight_converter_compatibility(otherwise that function's bare except would still silently no-op).This mirrors the equivalent fix shipping in
unsloth-zoo(zoo applies its own copy atapply_import_fixes()time), but a user can run unsloth against an olderunsloth_zoothat doesn't have the workaround, so the unsloth side needs to own a copy too.Gating contract
Strict no-op outside the (peft 0.19.x, transformers 4.x) combination:
peft.utils.transformers_weight_conversionalready imports clean (transformers v5+, or any peft fork off the v5 path).sys.modules/find_spec. We never overwrite the realtransformers.conversion_mapping/transformers.core_model_loadingon transformers v5+.__unsloth_stub__) on the stub modules makes a second call returnFalse, a third call returnFalse, and so on.ImportErroris left for the caller's own try / except to take over.Compatibility matrix
False, never touchsys.modules.peft.tuners.tuners_utils, behind peft's ownis_transformers_ge_v5gate. Stubs are therefore unreachable from TRL on a 4.x install, and on a 5.x install the real submodules win the import race.sys.modules.Tests
tests/conftest.pyis updated to pre-apply this specific fix via the standalone import-fixes module so the GPU-free drift detector test (tests/test_import_fixes_drift.py::test_peft_transformers_weight_conversion_importable_and_signature, shipping in a sibling PR) sees the same patched state a realimport unslothwould. The pattern mirrors unsloth-zoo'stests/conftest.py_apply_zoo_import_fixes_for_testshelper, scoped to just the peft fix.Local verification on (peft 0.19.1, transformers 4.57.6, torch 2.9.1+cu128):
Full suite: 16/18 pass. The two remaining failures (
test_triton_compiled_kernel_has_num_ctas_and_cluster_dims,test_vllm_guided_decoding_params_or_structured_outputs_present) are independent of this PR -- they were failing before our changes for unrelated drift items (triton 3.6+CompiledKernelshape, vLLM PR #22772GuidedDecodingParamsrename) that this PR does not address. They are also handled by the equivalent zoo fixes already shipping, and can be wired into the same conftest helper in a follow-up.End-to-end smoke (
import unsloth-> existingpatch_peft_weight_converter_compatibilityactually installs):Idempotence:
Transformers v5+ simulation (real submodules pre-installed, no sentinel):