Add MOSS-TTS-v1.5 (delay) and MOSS-Audio-Tokenizer#46447
Conversation
eustlb
left a comment
There was a problem hiding this comment.
Thanks a lot for raising this PR! A few comments for a first iteration 🤗
As a general note, to minimize the number of review iterations and get this merged as quickly as possible:
- Let's leverage
modular: a lot of the functionality is already integrated into the library, and we should make use of it. This significantly reduces the maintenance burden on our side 🤗 unless there is a clear motivation for it, the rest of the implem should be adapted to match those - Regarding the
processorandgenerateimplementations: this is a bit trickier, but by looking at the implementations of other TTS (eghiggs_audio_v2) models (and other model types in general), it's usually straightforward to align with the library's patterns. Doing so will also help avoid review comments on aspects that contributors can infer directly from existing implementations.
|
Hi @eustlb, thanks for your review. I'm working on addressing your proposed changes. One question I have is that we would like to add another MossTTSLocal model, which architecturally differs from the MossTTSDelay model from this PR. Do you recommend us submitting a different PR for adding the MossTTSLocal model? Or just do it in this PR? Note that they share the same MossAudioTokenizer as their codec to construct audio. Thank you! |
… the internal helper classes like MossAudioTokenizerPatchedPretransform and MossAudioTokenizerProjectedTransformer with minimal changes for now.
…ely on @capture_outputs.
|
Hi @eustlb, thanks again for your previous round of review. I've addressed all your comments, please let me know what you think of them. The |
|
Hi @eustlb, just want to follow up on this PR. I'd appreciate your advice! |
ebezzam
left a comment
There was a problem hiding this comment.
@ted-mosi I've left some comments on the tokenizer to complement @eustlb's review. The same ideas will be applicable to the TTS model.
There are two key files are that missing:
- conversion script, to convert an existing checkpoint to a Transformers-compatible one
- feature extractor, to prepare audio before passing it to the model
I've tried to point to related models to help you update the code according to Transformers conventions. Thank you!
Important things to keep in mind:
- Avoid redefining existing components in Transformers (attention, encoder/decoder layers, etc). That's where modular and the conversion script go hand in hand. If your component has a different state dict (e.g. interleaved attention), the role of the conversion scrip is to modify the state dict so that the Transformers-compatible one can be used with existing modules in the library.
- Remove any unused code paths / modules. It's completely normal and expected that you experiment with different configurations while developing your model. However, when we merge a model into Transformers, we only keep the configuration/components necessary for the final version.
I hope the comments help, and thank you for your contribution!
|
Thank you for your contribution 🤗! CI Security Gate — automatic approval blockedThis PR was not automatically approved for CI because the security gate failed. Possible reasons:
See the workflow run for the exact violations. A maintainer can review and manually approve CI if a finding is a false positive. |
|
Thank you for your review! I have finished addressing all review comments. Please let me know what you think! One note about the CICD pipeline, I got one failure for the I don't think I have the permission to run it, so could you please re-run that test for me to see if it passes? Thank you! |
ebezzam
left a comment
There was a problem hiding this comment.
thanks for adding the conversion script and the feature extractor!
My comments are primarily on aligning with conventions of the library. I'm not exhaustive but in general:
- please remove unused configuration attributes and code paths. We only want to keep those that are relevant for the final model.
- for every module/config/etc, we should be asking ourselves how is this done elsewhere in the library. With modular, we only want to keep the algorithmic differences.
Thank you for going over my comments carefully, but also considering how they could apply to other parts of your PR.
|
[For maintainers] Suggested jobs to run (before merge) run-slow: auto, moss_audio_tokenizer, moss_tts_delay |
CI recapDashboard: View test results in Grafana |
|
Hi @ebezzam , thank you for the previous round of code review. I have carefully addressed all your comments, and extend those ideas to the entire PR. I do find the code to be greatly improved after applying your suggestions! Please let me know what you think of them this time! One note is that I don't think the pipeline failure is due to my changes, but let me know if there's anything that I can do to fix it. |
What does this PR do?
This PR adds native Transformers support for:
MossTTSDelayModelMossTTSDelayProcessorMossAudioTokenizerModelAutoModelForTextToWaveformAutoModelForAudioTokenizationArchitecture notes
MOSS-TTS-v1.5 uses a delay-pattern TTS generation format with text tokens plus multiple audio VQ channels. The processor handles user/assistant message
construction, reference-audio tokenization, delay/de-delay conversion, and waveform decoding through the native MOSS audio tokenizer.
MOSS-Audio-Tokenizer is added as a native audio-tokenizer model so MOSS-TTS no longer depends on Hub remote code for audio tokenization.
Before submitting
Pull Request section?
documentation guidelines, and
here are tips on formatting docstrings.
AI assistance disclosure
AI assistance was used while drafting and testing parts of this PR. I reviewed the changed files, checked the implementation against existing Transformers
model patterns, and ran the relevant local tests listed below. I am responsible for the final code and for responding to review feedback.
Who can review?
@eustlb @ebezzam @vasqu