import_fixes + drift detectors: cover transformers 5.x drift (unblocks PR #5376)#5423
Conversation
PR #5414's drift detectors and the corresponding import_fixes helpers were written against transformers 4.x. The Repo tests (CPU) step (which installs transformers>=4.51,<5.5 and currently resolves to 5.4.0) surfaces three real predicate gaps: * test_pretrained_model_enable_input_require_grads_uses_old_pattern fires DRIFT DETECTED whenever the source contains "for module in self.modules()". But the unsloth replacement that patch_enable_input_require_grads installs ALSO uses that pattern -- deliberately, just wrapped in try / except NotImplementedError. So the predicate cannot distinguish broken upstream from the working patch. Accept either pre-HF#41993 shape (no self.modules() loop) or the post-patch shape (loop + NotImplementedError handler). * test_transformers_torchcodec_available_flag_is_present asserts the pre-5.x module-level _torchcodec_available flag. transformers 5 replaced it with an lru_cache'd is_torchcodec_available() callable. Accept either symbol. Also update disable_torchcodec_if_broken to actually disable on 5.x: clear the cache and rebind the function to return False. Local verification: * transformers 4.57.6 + trl 0.25.1 + peft 0.19.1 + triton 3.5.1 + vllm 0.15.1+cu130 (the Core HF=4.57.6 cell shape): 18 passed. * transformers 5.8.1 + peft 0.19.1 + torch 2.9 CPU (the Repo tests (CPU) shape, drop trl / vllm / datasets / xformers): 12 passed, 6 skipped on missing optional libs, 0 failed. PR #5376's Repo tests (CPU) failure was a triple: - triton + enable_input_require_grads: fixed by merging current main (PR #5421's relaxed triton predicate + conftest 'import unsloth'). - torchcodec: fixed by THIS PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d9a42b226
ℹ️ 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".
| is_avail.cache_clear() | ||
| except AttributeError: | ||
| pass | ||
| tf_import_utils.is_torchcodec_available = lambda: False |
There was a problem hiding this comment.
Update the cached backend mapping when stubbing torchcodec
On transformers 5.x, this only rebinds transformers.utils.import_utils.is_torchcodec_available, but BACKENDS_MAPPING is built at import time with the original function object for the "torchcodec" backend. In environments where torchcodec is installed but its native libraries/FFmpeg are broken, any path that checks requires_backends(..., "torchcodec") still calls the old cached function and reports torchcodec as available, so the later audio decode import can still crash instead of falling back. Please also update the BACKENDS_MAPPING["torchcodec"] entry (and any re-exported checker, if present) to point at the false stub.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d9a42b226
ℹ️ 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".
| is_avail.cache_clear() | ||
| except AttributeError: | ||
| pass | ||
| tf_import_utils.is_torchcodec_available = lambda: False |
There was a problem hiding this comment.
Update the cached backend mapping when stubbing torchcodec
On transformers 5.x, this only rebinds transformers.utils.import_utils.is_torchcodec_available, but BACKENDS_MAPPING is built at import time with the original function object for the "torchcodec" backend. In environments where torchcodec is installed but its native libraries/FFmpeg are broken, any path that checks requires_backends(..., "torchcodec") still calls the old cached function and reports torchcodec as available, so the later audio decode import can still crash instead of falling back. Please also update the BACKENDS_MAPPING["torchcodec"] entry (and any re-exported checker, if present) to point at the false stub.
Useful? React with 👍 / 👎.
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
…3) (unslothai#642) The two transformers-targeting drift detectors in tests/test_upstream_ import_fixes_drift.py had predicates written against transformers 4.x only; on the Core matrix's HF=latest cell (transformers>=5,<6) they fire DRIFT DETECTED even when the upstream / fix combination is healthy. * test_pretrained_model_enable_input_require_grads_uses_old_pattern fired whenever the source contains "for module in self.modules()". But the unsloth replacement that patch_enable_input_require_grads installs deliberately ALSO uses that pattern, just wrapped in try / except NotImplementedError. The predicate could not distinguish broken upstream from the working patch. Accept either pre-HF#41993 shape (no self.modules() loop) or the post-patch shape (loop + NotImplementedError handler). * test_transformers_torchcodec_available_flag_is_present asserted the pre-5.x module-level _torchcodec_available flag. transformers 5 replaced it with an lru_cache'd is_torchcodec_available() callable. Accept either symbol -- the patch in unsloth/import_fixes.py now targets both (PR #5423 on unslothai/unsloth). Local verification: * transformers 4.57.6 + trl 0.25.1 + peft 0.19.1 + triton 3.5.1 + vllm 0.15.1+cu130 (the Core HF=4.57.6 cell shape): 18 passed. * transformers 5.8.1 + peft 0.19.1 + torch 2.9 CPU (no trl / vllm / datasets / xformers installed): 12 passed, 6 skipped on missing optional libs, 0 failed. The corresponding import_fixes.py changes already shipped on unslothai/unsloth#5423. Zoo only carries the detectors so this PR is test-only.
Resolve conflict in studio/backend/core/inference/llama_cpp.py: take main's load_model body wrapped in the serial_load_lock (unslothai#5401 / unslothai#5161) and the unslothai#5347 mmproj guard, layered with this PR's two intended edits: - New @staticmethod _build_windows_path_dirs(binary_dir, prefix, cuda_path) next to _windows_pip_nvidia_dll_dirs, returning the win32 PATH ordering binary_dir, pip-nvidia wheels, CUDA_PATH/bin, CUDA_PATH/bin/x64. - start_llama_server's win32 branch now calls the helper instead of re-implementing the same ordering inline. The PR's own install_llama_prebuilt.py nit (f"llama-" -> "llama-") and the new test file under studio/backend/tests/ auto-merged cleanly. Picks up PR unslothai#5423's transformers 5.x drift-detector predicate fixes via the merge, which is what was failing on the three Core CI jobs.
Summary
PR #5414's drift detectors and the corresponding import_fixes helpers were written against transformers 4.x. The
Repo tests (CPU)step (studio-backend-ci.yml) installstransformers>=4.51,<5.5and currently resolves to 5.4.0, which surfaces three real predicate gaps. PR #5376's redRepo tests (CPU)cell is the canonical reproducer.Two of those gaps land here; the third (
tritonpredicate strictness) was already fixed in PR #5421 and just needs PR #5376 to merge main to pick it up.What this PR changes
1.
test_pretrained_model_enable_input_require_grads_uses_old_patternThe detector fires DRIFT DETECTED whenever the source contains
for module in self.modules(). But the unsloth replacement thatpatch_enable_input_require_gradsinstalls also uses that pattern (deliberately, just wrapped intry / except NotImplementedError). So the predicate cannot distinguish broken upstream from the working patch. The fix accepts either:self.modules()loop in the source), orNotImplementedErrorhandler in the source).2.
test_transformers_torchcodec_available_flag_is_present+disable_torchcodec_if_brokenDetector currently asserts the pre-5.x module-level
_torchcodec_availableflag. transformers 5 replaced it with anlru_cache'dis_torchcodec_available()callable. Two changes:disable_torchcodec_if_brokenlearns to actually disable on 5.x: clear the function'slru_cacheand rebind it tolambda: False. Pre-5.x path (flag flip) is preserved verbatim.Local verification
How this unblocks PR #5376
PR #5376's
Repo tests (CPU)is failing on three drift detectors:Once this PR merges, PR #5376 just needs to merge main and CI goes green.
Test plan
pytest tests/test_import_fixes_drift.pyis 18/18 on the HF=4.57.6 cell shape.