Skip to content

remove mcore0.12-mcore0.14#59

Merged
Jintao-Huang merged 1 commit intomodelscope:mainfrom
Jintao-Huang:remove_mcore012_014
May 4, 2026
Merged

remove mcore0.12-mcore0.14#59
Jintao-Huang merged 1 commit intomodelscope:mainfrom
Jintao-Huang:remove_mcore012_014

Conversation

@Jintao-Huang
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown

@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 updates the megatron-core dependency to version 0.15 or higher and removes legacy version-checking logic and conditional code paths throughout the repository. A critical regression was identified in src/mcore_bridge/model/gpts/qwen3_next.py where the removal of a local kwargs variable prevents the cp_group from being passed to the apply_rotary_pos_emb call for keys, which would break context parallel functionality.

I am having trouble creating individual review comments. Click here to see my feedback.

src/mcore_bridge/model/gpts/qwen3_next.py (280-284)

high

Removing the local kwargs variable here causes a regression in the subsequent key = apply_rotary_pos_emb(..., **kwargs) call (which is around line 304 in the updated file). Since kwargs is no longer defined locally, it will refer to the kwargs from the function signature, which does not contain the required cp_group. This will break context parallel functionality for this model.

You should retain a local kwargs variable containing the cp_group to ensure it is passed to the key's RoPE application.

        kwargs = {'cp_group': self.pg_collection.cp}

@Jintao-Huang Jintao-Huang merged commit f020e47 into modelscope:main May 4, 2026
1 check passed
@Jintao-Huang
Copy link
Copy Markdown
Collaborator Author

/gemini review

Copy link
Copy Markdown

@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 updates the minimum required version of megatron-core to 0.15 and removes legacy compatibility logic for older versions (0.12, 0.13, and 0.14) throughout the codebase. The changes include updating documentation and requirements, removing version-based conditional branching in model definitions and bridge logic, and simplifying function signatures to directly pass parameters like vp_stage and tp_group. I have no feedback to provide.

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.

2 participants