-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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 support for custom checkpoints in MusicGen #30011
Add support for custom checkpoints in MusicGen #30011
Conversation
"`['facebook/musicgen-stereo-small', 'facebook/musicgen-stereo-medium', 'facebook/musicgen-stereo-large']` " | ||
"for the stereo checkpoints.", | ||
) |
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.
IMO we just have to update the doc here no?
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.
To mention that it can also be a path to a custom checkpoint
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.
I think decoder_config_from_checkpoint
will throw an error with a custom checkpoint
transformers/src/transformers/models/musicgen/convert_musicgen_transformers.py
Lines 105 to 109 in 3cc28d1
raise ValueError( | |
"Checkpoint should be one of `['small', 'medium', 'large']` for the mono checkpoints, " | |
"or `['facebook/musicgen-stereo-small', 'facebook/musicgen-stereo-medium', 'facebook/musicgen-stereo-large']` " | |
f"for the stereo checkpoints, got {checkpoint}." | |
) |
Is it possible to infer the config?
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.
We can update that error as well, the conversion script is not meant to only support official checkpoints.
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.
That makes sense. The script should be flexible enough to support all checkpoints. Can we expect the custom checkpoints to follow the same naming scheme? i.e. musicgen-stereo-[small|medium|large]
Hey! Can you make sure the CI is green and ping me again for a reveiw! |
Hey @ArthurZucker! The CI is green now. |
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.
Delay but thanks for the PR! 🔥
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
* feat: support custom checkpoint * update: revert changes and add TODO * update: docs and exception handling * fix: ah, extra space
What does this PR do?
Fixes #29873 (issue)
Add a new argument for custom checkpoint and pass it into
get_pretrained
Who can review?
@ElizavetaSedova and @ArthurZucker