Skip to content

DDP refactoring: Extract parameter layout computation into optimizer classmethod - #3812

Merged
deepakn94 merged 2 commits into
NVIDIA:mainfrom
deepakn94:dnarayanan/refactor_param_mapping
Apr 22, 2026
Merged

DDP refactoring: Extract parameter layout computation into optimizer classmethod#3812
deepakn94 merged 2 commits into
NVIDIA:mainfrom
deepakn94:dnarayanan/refactor_param_mapping

Conversation

@deepakn94

@deepakn94 deepakn94 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

At a high level, this PR refactors DistributedDataParallel (DDP). DDP's constructor now accepts a full_param_layout argument that describes how parameters and gradients should be mapped in the underlying _ParamAndGradBuffer. Distributed optimizers compute this mapping via a static compute_full_param_layout method. This allows different optimizers to control gradient buffer layout — e.g., the existing DistributedOptimizer uses a sequential layout with padding to ensure buckets are multiples of the data-parallel size, and in the future, LayerwiseOptimizer for Muon could specify a mapping such that each gradient bucket shard contains a full layer's gradients. This ensures that: a) gradient reduction can use reduce-scatter instead of all-reduce (gradient and parameter sharding follows optimizer sharding exactly), and b) no extra parameter copies are needed after the parameter all-gather (results go directly into the right place for the next forward pass).

Key changes:

  • New param_layout.py module with BufferKey, PerBufferParamLayout, and FullParamLayout dataclasses, plus shared padding utilities (pad_param_start, pad_bucket_end).
  • DistributedOptimizer.compute_full_param_layout() and _compute_per_buffer_param_layout() static methods that pre-compute padded layouts.
  • DistributedDataParallel accepts an optional full_param_layout and passes per-buffer layouts to _ParamAndGradBuffer.
  • _ParamAndGradBuffer consumes pre-computed layouts when provided, falling back to _compute_default_per_buffer_param_layout() (no-padding layout for non-distributed-optimizer cases).
  • training.py pre-computes layouts via DistributedOptimizer.compute_full_param_layout() when using the distributed optimizer with DDP.
  • New unit tests in test_param_layout.py for layout computation functions.

In the future, training.py can assemble layouts from multiple distributed optimizers (this should be straightforward since each optimizer will have disjoint buffers).

@copy-pr-bot

copy-pr-bot Bot commented Mar 11, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Comment thread megatron/core/distributed/param_and_grad_buffer.py
Comment thread megatron/core/distributed/param_and_grad_buffer.py Outdated
@deepakn94
deepakn94 force-pushed the dnarayanan/refactor_param_mapping branch from c0918d7 to 9dcf047 Compare March 18, 2026 19:51
@deepakn94

Copy link
Copy Markdown
Contributor Author

/claude review

@deepakn94
deepakn94 marked this pull request as ready for review March 18, 2026 22:56
@deepakn94
deepakn94 requested review from a team as code owners March 18, 2026 22:56
@deepakn94

Copy link
Copy Markdown
Contributor Author

/claude review

@svcnvidia-nemo-ci svcnvidia-nemo-ci added this to the Core 0.16 milestone Mar 18, 2026
@svcnvidia-nemo-ci
svcnvidia-nemo-ci requested a review from a team March 18, 2026 22:57
Comment thread megatron/training/training.py
Comment thread megatron/core/distributed/param_layout.py Outdated
Comment thread megatron/training/training.py
Comment thread megatron/core/distributed/param_layout.py Outdated
Comment thread tests/unit_tests/distributed/test_param_and_grad_buffer.py Outdated
@deepakn94
deepakn94 marked this pull request as draft March 18, 2026 23:07
@deepakn94

Copy link
Copy Markdown
Contributor Author

/claude review

Comment thread megatron/core/distributed/param_and_grad_buffer.py Outdated
@deepakn94

Copy link
Copy Markdown
Contributor Author

/claude review

@claude claude 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.

LGTM

@deepakn94
deepakn94 force-pushed the dnarayanan/refactor_param_mapping branch 2 times, most recently from de016f8 to 807c80a Compare April 2, 2026 20:33
@deepakn94

Copy link
Copy Markdown
Contributor Author

/claude review

@claude claude 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.

LGTM

@deepakn94
deepakn94 marked this pull request as ready for review April 2, 2026 21:33
@deepakn94
deepakn94 force-pushed the dnarayanan/refactor_param_mapping branch from 5b64fa2 to 056189f Compare April 19, 2026 21:19
@deepakn94

Copy link
Copy Markdown
Contributor Author

/claude review

@claude claude 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.

LGTM

@deepakn94
deepakn94 force-pushed the dnarayanan/refactor_param_mapping branch from 056189f to 556aa44 Compare April 19, 2026 21:58
@deepakn94
deepakn94 force-pushed the dnarayanan/refactor_param_mapping branch from 556aa44 to 2aaf8f7 Compare April 19, 2026 22:52
Comment thread megatron/core/optimizer/distrib_optimizer.py Outdated
Comment thread megatron/core/distributed/param_and_grad_buffer.py Outdated
@deepakn94
deepakn94 force-pushed the dnarayanan/refactor_param_mapping branch from 2aaf8f7 to 3df5114 Compare April 19, 2026 23:35
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Approved All necessary approvals have been made label Apr 20, 2026
deepakn94 and others added 2 commits April 20, 2026 17:46
…classmethod

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add deprecation warning for DDP auto-compute fallback, pass param_indices
directly to _compute_per_buffer_param_layout instead of mutating after
construction, remove nonlocal from _finalize_bucket, add type annotation
for param_layout parameter, and add new tests for shared_embedding +
bucket_size interaction and layout regression.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@deepakn94
deepakn94 force-pushed the dnarayanan/refactor_param_mapping branch from 3df5114 to b94d35e Compare April 21, 2026 00:47
@deepakn94
deepakn94 added this pull request to the merge queue Apr 22, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/24801777359

Merged via the queue into NVIDIA:main with commit 55b8111 Apr 22, 2026
67 of 68 checks passed
@deepakn94
deepakn94 deleted the dnarayanan/refactor_param_mapping branch April 22, 2026 23:28
maanug-nv added a commit to maanug-nv/Megatron-LM that referenced this pull request Apr 30, 2026
Signed-off-by: Maanu Grover <maanug@nvidia.com>
yangbofun pushed a commit to xlm-research/Megatron-LM that referenced this pull request May 22, 2026
…classmethod (NVIDIA#3812)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
yhgalaxy pushed a commit to yhgalaxy/Megatron-LM that referenced this pull request Jun 17, 2026
…classmethod (NVIDIA#3812)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: yhgalaxy <yhgalaxy@outlook.com>
jon-barker pushed a commit to jon-barker/Megatron-LM that referenced this pull request Jul 10, 2026
…classmethod (NVIDIA#3812)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jon Barker <jbarker@aws-cmh-slurm-1-vscode-02.cm.cluster>
terminator123 pushed a commit to 021ai/Megatron-LM that referenced this pull request Aug 3, 2026
…classmethod (NVIDIA#3812)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved All necessary approvals have been made complexity: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants