Skip to content

fix(megatron): synchronize parameters before inference refit - #4086

Open
wdykas wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
wdykas:fix/sync-params-before-refit
Open

fix(megatron): synchronize parameters before inference refit#4086
wdykas wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
wdykas:fix/sync-params-before-refit

Conversation

@wdykas

@wdykas wdykas commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Complete deferred distributed-optimizer parameter all-gathers before non-colocated inference refit reads model weights.

With overlap_param_gather=True, updated compute-weight shards need not be assembled until the next training forward. Neither the collective HF export nor native Megatron reshard runs that forward. Consequently, refit can read partly stale parameters even though the transfer itself succeeds. The existing gather in prepare_for_generation covers the colocated path, not these source-worker exports.

This patch adds synchronization before collective broadcast and before refit buffer offload. The existing shared-buffer preparation is preserved, and device synchronization makes the gathered weights ready for side-stream exporters. Training forward hooks remain unchanged.

Scope: one file, 13 added lines. No configuration, dependency, submodule, parser, or kernel changes.

Validation

  • Four-GPU BF16 precision-aware distributed Adam reproduction, TP2/DP2 with parameter-gather overlap: native-planner destinations differ before gather and become exactly equal afterward. The fixed rank-0 export source also contains stale values before gather.
  • Three consecutive refit points: copy/gather/device-sync ordering verified; forward hooks unchanged; subsequent parameter updates bit-identical to the synchronous-gather control.
  • Exact final inline blocks validated for DDP/non-DDP and overlap-enabled/disabled guards.
  • Ruff lint and format checks pass.

These are isolated local checks; diagnostic scripts are intentionally not included in this minimal experiment commit. Full RL convergence and quantized shared-buffer behavior have not been validated.

Signed-off-by: wdykas <wdykas@nvidia.com>
@wdykas
wdykas requested review from a team as code owners September 10, 2026 19:44
@wdykas wdykas added the CI:L2 Run doctests, unit tests, functional tests, and convergence tests label Sep 10, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@wdykas

wdykas commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 7ef3d21

@wdykas wdykas closed this Sep 10, 2026
@wdykas wdykas reopened this Sep 10, 2026
@wdykas

wdykas commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 7ef3d21

Signed-off-by: wdykas <wdykas@nvidia.com>
@wdykas
wdykas requested review from a team as code owners September 11, 2026 02:11
@wdykas

wdykas commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test e521a5b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:L2 Run doctests, unit tests, functional tests, and convergence tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant