Skip to content
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

register fused rmsnorm as pytorch custom op #296

Draft
wants to merge 1 commit into
base: gh/tianyu-l/11/base
Choose a base branch
from

Conversation

tianyu-l
Copy link
Contributor

@tianyu-l tianyu-l commented May 2, 2024

tianyu-l added a commit that referenced this pull request May 2, 2024
ghstack-source-id: 401d968feaa2e58eedb573c07739694358a8d4a6
Pull Request resolved: #296
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 2, 2024
@tianyu-l tianyu-l marked this pull request as draft May 2, 2024 21:55
tianyu-l added a commit that referenced this pull request Aug 16, 2024
ghstack-source-id: 401d968feaa2e58eedb573c07739694358a8d4a6
Pull Request resolved: #296
@tianyu-l tianyu-l force-pushed the gh/tianyu-l/11/base branch from 17cda29 to e34d2ac Compare August 16, 2024 21:00
@tianyu-l tianyu-l force-pushed the gh/tianyu-l/11/head branch from 1959d5f to c3daa9a Compare August 16, 2024 21:00
@msaroufim
Copy link
Member

n00b but is there any benefit in making a triton function a custom op? User defined triton functions should just work with compile

@tianyu-l
Copy link
Contributor Author

tianyu-l commented Dec 4, 2024

@msaroufim Hmm I don't know much. Maybe it depends on the way the triton function is wrapped? In this PR there is an autograd.function wrapping the triton function. cc: @Chillee @lessw2020 for more context.

Beyond compile, making it a custom op can be helpful for other things, e.g. registering customized DTensor sharding propagation rules, allow PP tracing to work (although we no longer have it in torchtitan) etc.

@msaroufim
Copy link
Member

should we perhaps just remove this custom kernel? I believe in pytorch nightlies rms norm should now work

@tianyu-l
Copy link
Contributor Author

tianyu-l commented Dec 6, 2024

I believe in pytorch nightlies rms norm should now work

@msaroufim oh which rms norm are you referring to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants