Skip to content

Fix TypeError on list-typed ignore_keys_at_rope_validation in RoPE config#46142

Open
Charly21r wants to merge 1 commit into
huggingface:mainfrom
Charly21r:fix/rope-ignore-keys-list-coercion
Open

Fix TypeError on list-typed ignore_keys_at_rope_validation in RoPE config#46142
Charly21r wants to merge 1 commit into
huggingface:mainfrom
Charly21r:fix/rope-ignore-keys-list-coercion

Conversation

@Charly21r
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #46121

RotaryEmbeddingConfigMixin.ignore_keys_at_rope_validation is a set at the class level, but JSON has no set type, so any config.json that serializes this field (e.g. checkpoints written by LoRA merge / export tooling like ms-swift) loads it back as a list instance attribute that shadows the class default. RotaryEmbeddingConfigMixin.convert_rope_params_to_dict then does:

self.ignore_keys_at_rope_validation = self.ignore_keys_at_rope_validation | {"partial_rotary_factor"}

which raises TypeError: unsupported operand type(s) for |: 'list' and 'set' whenever partial_rotary_factor is also set on the config. In practice this prevents serving such merged checkpoints (observed downstream in vLLM with merged Qwen3.5 checkpoints).

This PR coerces the attribute to a set before the union in src/transformers/modeling_rope_utils.py, and adds a regression test in tests/utils/test_modeling_rope_utils.py covering both direct attribute assignment and the from_dict round-trip path that mirrors the JSON-deserialization flow.

Code Agent Policy

  • I confirm that this is not a pure code agent PR.

Before submitting

  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you write any new necessary tests?

Who can review?

@Rocketknight1

@github-actions
Copy link
Copy Markdown
Contributor

View the CircleCI Test Summary for this PR:

https://huggingface.co/spaces/transformers-community/circle-ci-viz?pr=46142&sha=1267e6

@Rocketknight1
Copy link
Copy Markdown
Member

LGTM, CI issues seem unrelated. It's core code though, so I'll wait for a core maintainer to approve in case I'm totally wrong here! cc @ArthurZucker @Cyrilvallez @vasqu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

convert_rope_params_to_dict raises TypeError when ignore_keys_at_rope_validation is a JSON-loaded list

2 participants