Repository navigation
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. |
|
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. |
There was a problem hiding this comment.
Thanks for the PR!
https://google.github.io/eng-practices/review/developer/small-cls.html
This PR is too large for me to review right now. Could we reduce the first iteration to under 500 LOC (including tests), even if that means cutting features or simplifying the algorithm?
|
|
||
|
|
||
| @dataclasses.dataclass(frozen=True) | ||
| class ShardPlan: |
There was a problem hiding this comment.
Apparently, most of the PR is spent on deciding the parameter-to-rank mapping. Can we separate this into another PR and make this PR focused on the most basic allgather-each-parameter-group solution? We may even keep this as a baseline for numerics testing.
Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Instead of grabbing it implicitly. Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Now we initialize once at the start, but only if any rank contains boundary parameters. Signed-off-by: janEbert <janpabloe@nvidia.com>
Accordingly, we update the function signature and add a type cast to ignore a type error. Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
We need to execute the closure in a gradient-enabled context. Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Also move `Muon` import to the top. Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
ee0ba46 to
dc05e2f
Compare
Cached so not expensive, but prevents cache errors. Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
- Fix style - Adjust line length Signed-off-by: janEbert <janpabloe@nvidia.com>
Signed-off-by: janEbert <janpabloe@nvidia.com>
|
Hi @wujingyue , this PR has been pending for a while. Do you have an estimate of when it might move forward and get merged into the main branch? BTW, I’d also like to see MFSDP v2 support functionality similar to the emerging optimizer support in the distributed optimizer path — specifically, allowing some parameters to use the Muon optimizer while others use Adam. Is this feature currently on the roadmap? |
|
We are splitting this into multiple PRs, following https://github.com/NVIDIA/Megatron-LM/blob/main/megatron/core/distributed/fsdp/src/docs/mfsdp_design.md#development-process. #6597 is the first one. |
Squashed from NVIDIA#6425. Signed-off-by: tongliu <tongliu@nvidia.com> Signed-off-by: Jack Chang <jianbinc@nvidia.com>
9bb205d to
bb9ce7e
Compare
bb9ce7e to
dc05e2f
Compare
|
(Accidentally pushed other PR branch here. Reverted now.) |
What does this PR do?
Add Muon support for M-FSDPv2 based on an experimental optimized M-FSDPv1 prototype.
No integration into MCore yet.
This version uses all-
Flatplacements and uses an owner-based design:Pseudocode:
Internal design doc with more details: https://docs.google.com/document/d/11FGBYk-hdvGBGh_s-1tGBv2JYLicRPXIHApILtD6Mzw/edit?tab=t.vd156zcgkxyj
Minor information
get_containing_parameter_groupsignature is due to it being too strict; I needed to pass rawtorch.Tensors to this function.Issue tracking
Linked issue: Ref #5533.