m-fsdp: wire use_precision_aware_optimizer from ddp_config to ParamAn… - #4024
Conversation
|
This PR has been automatically converted to draft because all PRs must start as drafts. When you are ready for review, click Ready for Review to begin the review process. This will:
See the contribution guide for more details. |
|
/ok to test 74ddff5 |
cspades
left a comment
There was a problem hiding this comment.
Nit: Needs README / PyPI package documentation for the new argument, and possibly any arguments in training.py should be set during DDPConfig init.
Can handle in a subsequent PR if not done here, thanks!
| """If true, Megatron-FSDP's ParamAndGradBuffer uses the precision-aware optimizer | ||
| gradient path (e.g. `decoupled_grad` on optimizer parameters) instead of casting | ||
| main gradients to parameter dtype for `.grad`. Should match the training optimizer | ||
| setting (e.g. `--use-precision-aware-optimizer`). |
There was a problem hiding this comment.
command line args are only one way these dataclasses get populated, we shouldn't reference them in these docs. Is there an OptimizerConfig dataclass that has this option that should match?
There was a problem hiding this comment.
I do see this - https://github.com/NVIDIA/Megatron-LM/blob/main/megatron/core/optimizer/optimizer_config.py#L187:
use_precision_aware_optimizer: bool = False """If true, allows optimizer-related tensors (master_param, gradients and optimizer states) to be set to lower precision. Defaults to False. """
There was a problem hiding this comment.
Currently I think Megatron-Bridge and Megatron-FSDP only have this DDPConfig to shove all arguments into.
I think let's remove the reference to --use-precision-aware-optimizer to avoid confusing MLM vs. MBridge vs. etc. users of MCore, but I think having the same name as the attr in the OptimizerConfig might be helpful since they should be used together (when relevant, of course). Generally DDP-type classes need to know where to dump the grad.
There was a problem hiding this comment.
I've gone ahead and updated the docstring.
74ddff5 to
e9a02e3
Compare
|
/ok to test e9a02e3 |
e9a02e3 to
7ef00ed
Compare
7ef00ed to
9e9aaab
Compare
… MLM --use-precision-aware-optimizer. Signed-off-by: Cory Ye <cye@nvidia.com>
9e9aaab to
cd0bf5e
Compare
|
/ok to test 4d8cf30 |
|
/ok to test 038b141 |
jaredcasper
left a comment
There was a problem hiding this comment.
Approving, but can we make a plan for how to remove the megatron_fsdp_* args from ddpconfig?
Yeah, I'll start a design doc with Megatron-Bridge and see if we can pass in DDPConfig and FSDPConfig separately, that's the main blocker at this point. Discussions can be had there. |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/23869171336 |
NVIDIA#4024) Signed-off-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <44509866+cspades@users.noreply.github.com>
NVIDIA#4024) Signed-off-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <44509866+cspades@users.noreply.github.com> Signed-off-by: yhgalaxy <yhgalaxy@outlook.com>
NVIDIA#4024) Signed-off-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <44509866+cspades@users.noreply.github.com> Signed-off-by: Jon Barker <jbarker@aws-cmh-slurm-1-vscode-02.cm.cluster>
# Description NVIDIA#4024 NVIDIA#3746 NVIDIA#2799 NVIDIA#3676 NVIDIA#4133 NVIDIA#3295 NVIDIA#3831 NVIDIA#3754 ## Type of change - [ ] New feature (non-breaking change which adds functionality) - [ ] Infra/Build change (changes to CI/CD workflows or build scripts) - [ ] Code refactoring - [ ] Documentation change - [x] Bug fix - [ ] Breaking change ## Changes - Content 1 - Content 2 - Content 3 - Content 4 ## Checklist - [ ] I have read and followed the contributing guidelines - [ ] The functionality is complete - [ ] I have commented my code, particularly in coverage report uploading steps - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added/updated tests that prove my feature works - [ ] New and existing unit tests pass locally --------- Signed-off-by: Cory Ye <cye@nvidia.com> Signed-off-by: yexiaochuan <yexiaochuan@baidu.com> Signed-off-by: Akshat Kumar <akshat230405@gmail.com> Signed-off-by: Youngeun Kwon <youngeunk@nvidia.com> Signed-off-by: jinliangl <jinliangl@nvidia.com> Co-authored-by: Ritesh Patel <ripatel@nvidia.com> Co-authored-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <44509866+cspades@users.noreply.github.com> Co-authored-by: Xiaochuan Ye <tap91624@gmail.com> Co-authored-by: yexiaochuan <yexiaochuan@baidu.com> Co-authored-by: Akshat8510 <akshat230405@gmail.com> Co-authored-by: Kunlun Li <94586211+kunlunl@users.noreply.github.com> Co-authored-by: eternally-z <zzywzj@gmail.com> Co-authored-by: Philip Petrakian <ppetrakian@nvidia.com> Co-authored-by: Youngeun Kwon <youngeunk@nvidia.com> Co-authored-by: xuwchen <xuwenc@nvidia.com> Co-authored-by: Li Jinliang <jinliangl@nvidia.com>
NVIDIA#4024) Signed-off-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <cye@nvidia.com> Co-authored-by: Cory Ye <44509866+cspades@users.noreply.github.com>
…dGradBuffer
What does this PR do ?
Contribution process
Pre-checks
Code review
Feel free to message or comment the @mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!
All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.
Step 1: Mark PR as "Ready for Review"
.github/CODEOWNERS.Final Review might get declined if these requirements are not fulfilled.
Step 2: Final Review
For PRs that change
megatron/core, once all expert reviewers have approved, theFinal Reviewlabel is applied automatically and final reviewers are assigned.For PRs outside
megatron/core, this step is skipped.Step 3: Approved
Once all required reviewers have approved, the
Approvedlabel is applied automatically.Merge
Any member of mcore-engineers will be able to merge your PR.
For MRs into `dev` branch
The proposed review process for `dev` branch is under active discussion.MRs are mergable after one approval by either
eharper@nvidia.comorzijiey@nvidia.com.