Skip to content

Conversation

@SumanthRH
Copy link
Member

#690 breaks main:

           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/ray/session_2025-11-10_23-41-08_808638_5823/runtime_resources/working_dir_files/_ray_pkg_70495ea0ae6ec02c/skyrl_train/workers/fsdp/fsdp_worker.py", line 66, in init_model
    wrapped_model = HFModelWrapper(
                    ^^^^^^^^^^^^^^^
  File "/tmp/ray/session_2025-11-10_23-41-08_808638_5823/runtime_resources/working_dir_files/_ray_pkg_70495ea0ae6ec02c/skyrl_train/model_wrapper.py", line 117, in __init__
    self.model = model_class.from_pretrained(
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ray/.cache/uv/builds-v0/.tmpxGZQ4v/lib/python3.12/site-packages/transformers/models/auto/auto_factory.py", line 604, in from_pretrained
    return model_class.from_pretrained(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ray/.cache/uv/builds-v0/.tmpxGZQ4v/lib/python3.12/site-packages/transformers/modeling_utils.py", line 277, in _wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/ray/.cache/uv/builds-v0/.tmpxGZQ4v/lib/python3.12/site-packages/transformers/modeling_utils.py", line 4971, in from_pretrained
    model = cls(config, *model_args, **model_kwargs)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Qwen2ForCausalLM.__init__() got an unexpected keyword argument 'rope_parameters'

…formers) w. backward-compatible support for vLLM (NovaSky-AI#690)"

This reverts commit ad2b975.
@SumanthRH SumanthRH merged commit 098674d into NovaSky-AI:main Nov 21, 2025
3 checks passed
Copy link
Contributor

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

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 reverts a previous change that introduced a breaking TypeError in the main branch. The revert appears to be correct and complete, touching documentation, configuration files, and multiple source code files to remove the rope_parameters configuration and restore the previous rope_scaling and rope_theta implementation. I've identified a minor type inconsistency in the reverted code and provided a suggestion to fix it. Otherwise, this PR looks good to merge to resolve the issue on main.

if "rope_scaling" not in trainer_cfg:
return {}
if trainer_cfg.rope_scaling is None:
return None
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The function get_rope_scaling_config is type-hinted to return dict[str, Any], but it can return None here, which is a type inconsistency. To fix this and align with the type hint, please return an empty dictionary instead of None. An empty dictionary is handled correctly by the callers to mean 'no rope scaling'.

Suggested change
return None
return {}

li-boxuan pushed a commit to li-boxuan/SkyRL that referenced this pull request Nov 23, 2025
…ormers) w. backward-compatible support for vLLM (NovaSky-AI#690)" (NovaSky-AI#695)

NovaSky-AI#690 breaks main:

```python
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/ray/session_2025-11-10_23-41-08_808638_5823/runtime_resources/working_dir_files/_ray_pkg_70495ea0ae6ec02c/skyrl_train/workers/fsdp/fsdp_worker.py", line 66, in init_model
    wrapped_model = HFModelWrapper(
                    ^^^^^^^^^^^^^^^
  File "/tmp/ray/session_2025-11-10_23-41-08_808638_5823/runtime_resources/working_dir_files/_ray_pkg_70495ea0ae6ec02c/skyrl_train/model_wrapper.py", line 117, in __init__
    self.model = model_class.from_pretrained(
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ray/.cache/uv/builds-v0/.tmpxGZQ4v/lib/python3.12/site-packages/transformers/models/auto/auto_factory.py", line 604, in from_pretrained
    return model_class.from_pretrained(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ray/.cache/uv/builds-v0/.tmpxGZQ4v/lib/python3.12/site-packages/transformers/modeling_utils.py", line 277, in _wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/ray/.cache/uv/builds-v0/.tmpxGZQ4v/lib/python3.12/site-packages/transformers/modeling_utils.py", line 4971, in from_pretrained
    model = cls(config, *model_args, **model_kwargs)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Qwen2ForCausalLM.__init__() got an unexpected keyword argument 'rope_parameters'
```
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.

1 participant