[bugfix] encoders: keep Qwen2.5-VL importable on transformers 5 - #1791
SolitaryThinker merged 2 commits into
Conversation
transformers 5.0 removed cache_utils.SlidingWindowCache; a sliding window is a property of the cache's layers now (DynamicSlidingWindowLayer / StaticSlidingWindowLayer). The import in qwen2_5_vl_custom was unguarded and at module scope, so on transformers>=5.0.0 - what pyproject asks for - the module could not be imported at all, taking Reason1TextEncoder with it. Nothing is an instance of the removed class, so the fallback makes the two isinstance() checks answer False instead of raising. This encoder runs with use_cache=False, so past_key_values is None on that path regardless.
Merge Protections🔴 1 of 1 protections blocking · waiting on 👀 reviews and 🤖 CI
🔴 PR merge requirementsWaiting for
This rule is failing.
|
|
Correction to this PR's description, and it matters for anyone trying to reproduce it. The body says transformers 5 removed
It was removed in 5.0 and restored in 5.6 as an alias for Two consequences:
That makes it safer than the description implies: it cannot change behaviour on any version where the import currently works. The test asserts Sorry for the imprecision — the original claim was written against the version I had, and I did not check whether it held across the declared floor. Status: |
…rengthen its test (hao-ai-lab#1791)
|
Pushed c9cd284: corrected the transformers version range in the cache-guard comment and made the new test able to fail. Fixed
Verified
Left open
Note on the PR description
|
|
Thanks — both fixes look right to me. I had read the 5.0 removal as covering all of 5.x and missed that 5.6 brought the name back as an alias, and you are right that the encoder lane resolves ≥5.6, so the fallback arm was never reached in CI. Mergify still shows blocked; as far as I can tell it is waiting on a review, not on anything in the diff. |
Summary
fastvideo/models/encoders/qwen2_5_vl_custom.pycannot be imported on any transformers thisrepo asks for.
pyproject.tomlrequirestransformers>=5.0.0; the module has, at module scopeand unguarded:
transformers 5.0 removed the cache-level class — a sliding window became a property of the
cache's layers (
DynamicSlidingWindowLayer,StaticSlidingWindowLayer).Cache,DynamicCacheandStaticCacheare all still there; only this one went.class SlidingWindowCacheThis is not a dead file.
Reason1TextEncoderlives inregistry.py:88-89, registered forQwen2_5_VLForConditionalGeneration, and is used by the Cosmos 2.5 and Kandinsky 5 pipelineconfigs, so the failure lands on those pipelines.
It went unnoticed because
fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py— whichimports this module and would have failed at collection — is not in the explicit test list
ci-macos-mlx.ymlruns.The change
Import the removed class behind
try/except ImportError, matching how this file alreadyhandles
torch.distributed.tensor.Shardand the Qwen2.5-VL config classes:isinstance(x, ())isFalse, which is the right answer on transformers 5: with no such class,no cache object can be an instance of one. The two call sites
(
using_sliding_window_cache = isinstance(past_key_values, SlidingWindowCache)and theconfig.sliding_windowbranch below it) then behave as they did before for anything transformers5 can hand them.
On this encoder's own path the question does not arise either way —
reason1.py:311calls withuse_cache=False, and a cache is only builtif use_cache and past_key_values is None, sopast_key_valuesisNonethere.Not attempting to reconstruct the old behaviour from the new layer types: upstream Qwen2.5-VL
has had no
_update_causal_maskand noSlidingWindowCachereference since 4.57 at the latest,so this vendored copy has no upstream counterpart left to mirror. That is a bigger question than
making the module importable again.
Test
Added to the existing encoder test file:
test_sliding_window_cache_check_survives_transformers_5.Load-bearing — same test file against a clean
upstream/mainworktree, same environment:Also confirmed
from fastvideo.models.encoders.reason1 import Reason1TextEncoderimports again.Lint, at the versions pinned in
.pre-commit-config.yaml:(The 106 are pre-existing in this vendored file; I have not touched them.)