remove mcore0.12-mcore0.14#59
Conversation
There was a problem hiding this comment.
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)
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}
|
/gemini review |
There was a problem hiding this comment.
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.
No description provided.