Extract args init to launch scripts - #4225
Conversation
|
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. |
676fbcb to
b94beb4
Compare
|
/claude review |
There was a problem hiding this comment.
Two issues found:
-
Bug:
parse_and_validate_argsmissingreturn args— commented inline onmegatron/training/arguments.py. -
Dead parameter:
parsed_argsininitialize_megatron— theparsed_args=Noneparameter oninitialize_megatron()is now unused. The function body no longer references it (it just callsget_args()). It should be removed to avoid confusion, along with any callers that pass it.
Code reviewFound 2 issues:
Megatron-LM/megatron/training/training.py Lines 817 to 830 in 14fedae
Lines 5 to 9 in 14fedae 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Signed-off-by: Maanu Grover <maanug@nvidia.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com>
1c0f6f0 to
71c847b
Compare
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/24422302488 |
Signed-off-by: Maanu Grover <maanug@nvidia.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com>
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/24427514360 |
|
Missing a update in "examples/multimodal/train.py" |
…init split PR NVIDIA#4225 extracted argument parsing out of initialize_megatron(); call parse_and_validate_args() separately and invoke initialize_megatron() with no arguments.
Backward-compat stubs and aliases remain for external libraries, but all internal Megatron-LM code now uses the canonical Hybrid names. Changes: - mamba_builders.py: backward-compat re-export stub with deprecation warning (delegates to hybrid_builders) - pretrain_mamba.py: backward-compat wrapper that runpy's pretrain_hybrid with deprecation warning - pretrain_hybrid.py: port upstream parse_and_validate_args refactor (from upstream NVIDIA#4225 which modified pretrain_mamba.py) - megatron/training/arguments.py: import Symbols from canonical hybrid_layer_allocation path instead of backward-compat stub - nemotron3_super_release_g200/model_config.yaml: update --spec and --mtp-spec to use hybrid_layer_specs path - test_dsa_gpt_mamba_equivalence.py: update all class references and imports to HybridModel / hybrid_stack_spec - tools/checkpoint/remap_gpt_dsa_to_mamba.py: update docstrings and help text from MambaModel/MambaStack to HybridModel/HybridStack Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…init split PR NVIDIA#4225 extracted argument parsing out of initialize_megatron(); call parse_and_validate_args() separately and invoke initialize_megatron() with no arguments.
Signed-off-by: Maanu Grover <maanug@nvidia.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Signed-off-by: yhgalaxy <yhgalaxy@outlook.com>
Signed-off-by: Maanu Grover <maanug@nvidia.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Signed-off-by: Jon Barker <jbarker@aws-cmh-slurm-1-vscode-02.cm.cluster>
Signed-off-by: Maanu Grover <maanug@nvidia.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
What does this PR do ?
Extract args parsing and validation into a dedicated function out of
initialize_megatron(), and call it in pretrain scripts. This is to facilitate separating args logic from training code.We will build the config classes from the args in the pretrain scripts and pass that to
pretrain(), in #4227 .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.