Skip to content

[megatron] fix: engine alignment - #4097

Merged
ISEEKYAN merged 2 commits into
verl-project:mainfrom
ISEEKYAN:mcore_engine_fix
Nov 12, 2025
Merged

ISEEKYAN merged 2 commits into
verl-project:mainfrom
ISEEKYAN:mcore_engine_fix

Conversation

@ISEEKYAN

Copy link
Copy Markdown
Collaborator

separate the bf16 part of #4086. to fix the gap between megatron and fsdp in bf16.

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

Copy link
Copy Markdown
Contributor

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 aims to fix a discrepancy between Megatron and FSDP for bf16 training by setting finalize_model_grads_func on the model configuration. The change is implemented within the _build_optimizer method, ensuring it only applies during training, which is correct as this function is related to gradient handling. The implementation is straightforward and seems to correctly address the issue described in the pull request. I did not find any issues of high or critical severity.

@vermouth1992

Copy link
Copy Markdown
Collaborator

Could you please add some comments in the code to inform the people why we should add this?

from megatron.core.distributed import finalize_model_grads
from megatron.core.utils import get_model_config

# register some callbacks for megatron training, following https://github.com/NVIDIA/Megatron-LM/blob/core_v0.15.0rc7/megatron/training/training.py#L2039-L2057

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@ISEEKYAN
ISEEKYAN merged commit 425d6bb into verl-project:main Nov 12, 2025
47 of 48 checks passed
chenjiaoAngel added a commit to chenjiaoAngel/verl that referenced this pull request Nov 14, 2025
separate the bf16 part of verl-project#4086.
to fix the gap between megatron and fsdp in bf16.
chenhaiq pushed a commit to The-Hierophant/verl-1 that referenced this pull request Nov 18, 2025
separate the bf16 part of verl-project#4086.
to fix the gap between megatron and fsdp in bf16.
wuwendyy pushed a commit to wuwendyy/verl that referenced this pull request Nov 19, 2025
separate the bf16 part of verl-project#4086.
to fix the gap between megatron and fsdp in bf16.
albertimff pushed a commit to albertimff/verl that referenced this pull request Dec 1, 2025
separate the bf16 part of verl-project#4086.
to fix the gap between megatron and fsdp in bf16.
TimurTaepov pushed a commit to giorgossideris/verl that referenced this pull request Dec 20, 2025
separate the bf16 part of verl-project#4086.
to fix the gap between megatron and fsdp in bf16.
vyomakesh0728 added a commit to vyomakesh0728/verl that referenced this pull request Jan 22, 2026
separate the bf16 part of verl-project#4086.
to fix the gap between megatron and fsdp in bf16.
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