Add --muon-coefficient-type argument for Muon optimizer - #3927
Conversation
Allow users to select the Newton-Schulz polynomial coefficient set (e.g. simple, quintic, polar_express, aol) via the new --muon-coefficient-type CLI flag. Supported types are discovered dynamically from the installed emerging_optimizers package so that upstream additions are picked up automatically without code changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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. |
Derive supported coefficient types from the public NSCoeffT Literal type via typing.get_args() rather than reading keys from the private _COEFFICIENT_SETS dict. Tests likewise avoid importing _COEFFICIENT_SETS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
emerging_optimizers must be installed to use Muon, so there is no need for a hardcoded fallback list. get_supported_coefficient_types() now asserts the package is present and reads NSCoeffT directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace hardcoded _NS_STEPS_FOR_COEFF_TYPE mapping with dynamic discovery via get_supported_coefficient_types() (backed by NSCoeffT). Since get_coefficient_iterator cycles/repeats coefficients, a single default step count works for all types. Remove redundant duplicate test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| @@ -288,6 +311,7 @@ def lion_init_state_fn(opt, config=None): | |||
| "use_nesterov": config.muon_use_nesterov, | |||
There was a problem hiding this comment.
BTW, should we check for certain versions of emerging_optimizers? I believe this got re-named to nesterov in the latest release.
There was a problem hiding this comment.
I feel it is better to keep the code only support one version. And I think having that version be the tagged in pyproject is reasonable. Optionally we can add a global check.
We will bump main to support v0.2.0 soon(after dev refactor+bump and main2dev sync, both are finalizing).
The Lion class moved in emerging_optimizers 0.2. Gate the import behind an explicit version check so users get a clear error instead of a silent ImportError on older versions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests were failing with AssertionError because they unconditionally called get_supported_coefficient_types() which requires emerging_optimizers >= 0.2. Added pytestmark skip conditions and guarded module-level collection code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
validate_coefficient_type was unconditionally calling get_supported_coefficient_types() which asserts HAVE_EO_V02. This caused dist_checkpointing tests to fail when emerging_optimizers < 0.2 was installed, since TensorParallelMuon.__init__ always calls validate_coefficient_type. Skip validation when HAVE_EO_V02 is False, as the coefficient_type kwarg is not passed to newton_schulz_tp in that case anyway. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…>= 0.2 "quintic" is the default coefficient type supported before emerging_optimizers 0.2, so validate against it rather than skipping validation entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ient_types
Now that validate_coefficient_type falls back to ("quintic",) without
emerging_optimizers >= 0.2, most tests only need HAVE_EMERGING_OPTIMIZERS.
Only the three tests that directly call get_supported_coefficient_types()
still require HAVE_EO_V02.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sion The coefficient_type parameter is supported in emerging_optimizers >= 0.1, so there's no need to gate it behind HAVE_EO_V02 (>= 0.2). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/ok to test 10bf5eb |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/23577425224 |
Co-authored-by: root <root@pool0-0084.cm.cluster> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: root <root@pool0-0393.cm.cluster> Co-authored-by: root <root@pool0-0154.cm.cluster> Co-authored-by: root <root@pool0-0021.cm.cluster> Co-authored-by: root <root@pool0-0331.cm.cluster> Co-authored-by: root <root@pool0-0079.cm.cluster> Co-authored-by: root <root@pool0-0137.cm.cluster>
Co-authored-by: root <root@pool0-0084.cm.cluster> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: root <root@pool0-0393.cm.cluster> Co-authored-by: root <root@pool0-0154.cm.cluster> Co-authored-by: root <root@pool0-0021.cm.cluster> Co-authored-by: root <root@pool0-0331.cm.cluster> Co-authored-by: root <root@pool0-0079.cm.cluster> Co-authored-by: root <root@pool0-0137.cm.cluster> Signed-off-by: yhgalaxy <yhgalaxy@outlook.com>
Co-authored-by: root <root@pool0-0084.cm.cluster> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: root <root@pool0-0393.cm.cluster> Co-authored-by: root <root@pool0-0154.cm.cluster> Co-authored-by: root <root@pool0-0021.cm.cluster> Co-authored-by: root <root@pool0-0331.cm.cluster> Co-authored-by: root <root@pool0-0079.cm.cluster> Co-authored-by: root <root@pool0-0137.cm.cluster> Signed-off-by: Jon Barker <jbarker@aws-cmh-slurm-1-vscode-02.cm.cluster>
Co-authored-by: root <root@pool0-0084.cm.cluster> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: root <root@pool0-0393.cm.cluster> Co-authored-by: root <root@pool0-0154.cm.cluster> Co-authored-by: root <root@pool0-0021.cm.cluster> Co-authored-by: root <root@pool0-0331.cm.cluster> Co-authored-by: root <root@pool0-0079.cm.cluster> Co-authored-by: root <root@pool0-0137.cm.cluster>
Allow users to select the Newton-Schulz polynomial coefficient set (e.g. simple, quintic, polar_express, aol) via the new --muon-coefficient-type CLI flag. Supported types are discovered dynamically from the installed emerging_optimizers package so that upstream additions are picked up automatically without code changes.