-
Notifications
You must be signed in to change notification settings - Fork 199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Muon Optimizer to contrib
#1126
base: main
Are you sure you want to change the base?
Conversation
338357f
to
a45f7a0
Compare
contrib
Hi all! How do I restrict the tests to exclude vectors as weights? |
Hello @leloykun, Looks like an interesting contribution thank you!
I don't fully understand your question. Could you give more context? Other questions/remarks: How does this optimizer treat vector-shaped values? The"muon_iterator" could run on vectors but not return what you want, so how do you make the distinction? Should it take a mask to only apply on matrices? Should it raise value error? Also could you add the mathematical description in the doc string? That would greatly help. Finally, put references at the end of the docstring (we'll adopt this format with #1129) Thank you! |
Hi @vroulet, Muon is only defined for matrix-shaped values. I'm thinking of raising an error when the input is vector-shaped, but where's the best place to put it? If there are other optimizers here that does this, can you point me to them? |
Hello @leloykun
Thanks again and sorry for the delay! |
Hey @leloykun, Looking at the original repository, it seems that the best course of action is to:
Please let me know if you are willing to pursue the PR |
Hi @leloykun, I'll be happy to take over merging this PR on our end, let me know once you want me to take a look at the changes / start the merge process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you introduce these cosmetic changes? The PR looks really good!
Hey, I just reviewed the PR, it looks great, thanks for addressing @vroulet's comments. I left some cosmetic comments. Once you get those in, can you squash your commits please? |
I'm sorry, I mislead you. Because we're supporting Python 3.9 Union and Tuple are necessary, exactly like you had it (having Optional would be ok too, but you can leave it as Union[A, None]). I'll start the merge once you can revert those Union/Tuple changes. |
@rdyro, I've just addressed your nits and squashed the commits. I kept the |
Adds support for @KellerJordan's Muon optimizer as detailed here: https://kellerjordan.github.io/posts/muon/
This optimizer allowed us to reduce the training time of a GPT-2 level model down to just ~3.5 minutes on 8xH100s. See: https://github.com/KellerJordan/modded-nanogpt
This implementation also supports the adaptive version of Muon that can adapt to the scale of the gradients as they change during training. To enable this feature, simply set
muon(adaptive=True, ...)
. See more at: https://github.com/leloykun/adaptive-muonMuon is equivalent to Shampoo without accumulation. I.e., take Shampoo's update rule, discard the left and right preconditioners, then simplify the matrix operations. Alternatively, Muon can also be thought of as steepest descent under Spectral norm. For more details, please refer to @jxbz's paper https://arxiv.org/pdf/2409.20325.
More generally, steepest descent under Schatten-p norm, for some large p, can be thought of as variants of Muon (note that Schatten-infty norm = Spectral norm). A (rough) proof of this claim can be found here: https://x.com/leloykun/status/1846842887839125941. This implementation supports these variants by allowing the user to pass in custom coefficients for the newton-schulz iterations.