Skip to content

fix(qwen3): support rope_parameters config - #1898

Merged
michalk8 merged 2 commits into
ml-explore:mainfrom
daniellaah:fix/qwen3-rope-parameters
Sep 18, 2026
Merged

michalk8 merged 2 commits into
ml-explore:mainfrom
daniellaah:fix/qwen3-rope-parameters

Conversation

@daniellaah

@daniellaah daniellaah commented Sep 17, 2026

Copy link
Copy Markdown
Contributor
  • ☑️ I understand it is strictly prohibited to use AI to write PR description
  • AI usage disclosure: Use Claude Code for testing and review

Summary

Qwen3 checkpoints produced by transformers 5.x releases (e.g. dinghar/qwen3-0.6b-pangram-sft) fail to load in mlx_lm.models.qwen3 because ModelArgs cannot load rope_theta correctly.

The checkpoint's config.json provides:

"rope_parameters": {"rope_theta": 1000000, "rope_type": "default"}

with no top-level rope_theta field. But ModelArgs.rope_theta is a required field with no default, so loading fails with:

TypeError: ModelArgs.__init__() missing 1 required positional argument: 'rope_theta'

In addition, rope_parameters may contain RoPE configuration such as rope_type, factor, and original_max_position_embeddings. ModelArgs currently ignores rope_parameters, so this configuration can be lost without raising an error because rope_scaling is optional.

Fix

ModelArgs:

  • switch rope_theta from a required field to an optional one so ModelArgs can get it from rope_parameters
  • add an optional rope_parameters: Optional[dict].
  • resolve rope_theta from rope_parameters when the top-level value is absent, and use rope_parameters as the fallback RoPE scaling configuration when rope_scaling is absent.

Changes are backward compatible: the existing test_qwen3 (flat rope_theta, no rope_parameters) still passes unchanged.

Testing

  • Added test_qwen3_rope_parameters:
    • nested config works
    • top-level fields take precedence
    • raises ValueError when rope_theta is absent from both
  • Confirmed the new test fails with the exact TypeError above against unpatched main, and passes after the fix.
  • Verified against the real checkpoint end to end: dinghar/qwen3-0.6b-pangram-sft now loads correctly.
  • Full tests/test_models.py suite passes locally.
  • Passed uvx pre-commit run --all

Fixes #1563

@daniellaah daniellaah changed the title fix(qwen3): accept nested rope_parameters (rope_theta, rope_scaling) fix(qwen3): fix(qwen3): support rope_parameters config Sep 17, 2026
@daniellaah daniellaah changed the title fix(qwen3): fix(qwen3): support rope_parameters config fix(qwen3): support rope_parameters config Sep 17, 2026

@michalk8 michalk8 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, thank you!

@michalk8
michalk8 merged commit 29040de into ml-explore:main Sep 18, 2026
2 checks passed
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.

Qwen3 model conversion fails when rope_theta is nested under rope_parameters

2 participants