Skip to content

[CI] set max transformers version for skywork model - #42104

Merged
DarkLight1337 merged 2 commits into
vllm-project:mainfrom
ROCm:cifix_multimodal_skywork
May 13, 2026
Merged

DarkLight1337 merged 2 commits into
vllm-project:mainfrom
ROCm:cifix_multimodal_skywork

Conversation

@divakar-amd

@divakar-amd divakar-amd commented May 8, 2026

Copy link
Copy Markdown
Contributor

This PR resolves the following error:

AttributeError: 'SkyworkChatModel' object has no attribute 'all_tied_weights_keys'.
Did you mean: '_tied_weights_keys'?

The failure occurs because of change in transformers 5.x APIs which introduced all_tied_weights_keys

In transformers 5.x post_init() now sets an instance attribute all_tied_weights_keys (a dict):

This PR resolves the following tests as shown in this AMD CI run

FAILED models/multimodal/generation/test_common.py::test_single_image_models[skywork_r1v-test_case57]
FAILED models/multimodal/generation/test_common.py::test_single_image_models[skywork_r1v-test_case58]
FAILED models/multimodal/generation/test_common.py::test_single_image_models[skywork_r1v-test_case59]
FAILED models/multimodal/generation/test_common.py::test_multi_image_models[skywork_r1v-test_case53]
FAILED models/multimodal/generation/test_common.py::test_multi_image_models[skywork_r1v-test_case54]
FAILED models/multimodal/generation/test_common.py::test_multi_image_models[skywork_r1v-test_case55]

Related AMD CI issue: #42020 (comment)

Signed-off-by: Divakar Verma <divakar.verma@amd.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the SkyworkR1VChatModel registry entry in tests/models/registry.py to cap the transformers version at 4.57. The reviewer identified that including the transformers_version_reason field causes tests to be skipped unconditionally, which would unintentionally disable test coverage for compatible v4.x versions. It is recommended to remove the transformers_version_reason entry to ensure tests continue to run on supported versions while still enforcing the version cap.

Comment thread tests/models/registry.py
Comment on lines +1336 to 1343
transformers_version_reason={
"hf": (
"SkyworkChatModel.__init__ does not call self.post_init(), so "
"`all_tied_weights_keys` is never set; Transformers v5 requires "
"this attribute in _move_missing_keys_from_meta_to_device."
)
},
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In the current implementation of check_transformers_version (lines 158-163), providing a key in transformers_version_reason (like "hf") causes the corresponding tests to be skipped unconditionally, regardless of whether the version requirement is met.

Since your reason string indicates that the issue is specific to transformers v5, adding this entry will unintentionally disable HF runner tests for Skywork on all versions, including v4.x. If you want to maintain test coverage for v4.x, you should omit the transformers_version_reason entry. The max_transformers_version="4.57" setting alone is sufficient to skip the tests on v5.x with a clear version-mismatch message.

    ),

@AndreasKaratzas
AndreasKaratzas requested a review from hmellor May 8, 2026 18:19
@DarkLight1337
DarkLight1337 enabled auto-merge (squash) May 9, 2026 04:42
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label May 9, 2026
@DarkLight1337
DarkLight1337 merged commit ca7e454 into vllm-project:main May 13, 2026
18 checks passed
mfylcek pushed a commit to mfylcek/vllm that referenced this pull request May 19, 2026
Signed-off-by: Divakar Verma <divakar.verma@amd.com>
h1t35h pushed a commit to h1t35h/vllm that referenced this pull request May 21, 2026
Signed-off-by: Divakar Verma <divakar.verma@amd.com>
philippesic pushed a commit to philippesic/vllm-semantic-cache that referenced this pull request Jul 19, 2026
Signed-off-by: Divakar Verma <divakar.verma@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants