Add CLVP
Author: susnato
Pull request: yes

# What does this PR do?

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->

Adds CLVP, which is an integral part of `tortoise-tts`. 
Required for `tortoise-tts` integration in HF diffusers(Please see [this issue](https://github.com/huggingface/diffusers/issues/3891)). 


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [x] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case. ([link](https://github.com/huggingface/diffusers/issues/3891))
- [x] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [x] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker and @younesbelkada
- vision models: @amyeroberts
- speech models: @sanchit-gandhi
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @gante
- pipelines: @Narsil
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @sgugger

Integrations:

- deepspeed: HF Trainer/Accelerate: @pacman100
- ray/raytune: @richardliaw, @amogkam

Documentation: @sgugger, @stevhliu and @MKhalusova

HF projects:

- accelerate: [different repo](https://github.com/huggingface/accelerate)
- datasets: [different repo](https://github.com/huggingface/datasets)
- diffusers: [different repo](https://github.com/huggingface/diffusers)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Maintained examples (not research project or legacy):

- Flax: @sanchit-gandhi
- PyTorch: @sgugger
- TensorFlow: @Rocketknight1

 -->

Comments:
cc: @sanchit-gandhi, @dg845

Very cool @susnato! Let me know if you have any questions / queries - more than happy to lend a hand here!

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_24745). All of your documentation changes will be reflected on that endpoint.

Hi @sanchit-gandhi, this PR is ready for review!

Some notes related to this design I wanted to mention - 
1. Although CLIP has both tokenizer and Image Processor, in tortoise the text is encoded and pushed into both the autoregressive model and the CLVP model so I think its better to have only one tokenizer(for tortoise) and encoding the text one time and pushing it to both of the models rather than defining a seperate tokenization_clvp.py.
2. Instead of processing Images and checking which image fits the description(from text) best, CLVP compares Speech Token Candidates and text. The speech tokens come from the output of the auto-regressive model itself, so we don't need the Image Processor too!
3. CLVP uses Rotary Position Embeddings.

Hello @sanchit-gandhi I have - 
-  pushed the changed that you asked.
- implemented the tokenizer and respective tests.

For the `Feature Extractor` I wanted to ask some things before going forward - 

`CLVP` compares the text embeddings generated by the tokenizer and the latent speech embeddings generated by an autoregressive model(here `gpt2`),  So IMO we need a Feature Extractor which generates latent embeddings, in other words the feature extractor must take the tokenizer's outputs and then apply a gpt2 model on it and then give us the output. But I don't think that it complies with the design of the library. (I don't know any Feature Extractor which uses another transformer model for processing. :(  )

In CLIP it was reasonable to use a Feature Extractor to just read images but I don't think it will be straight forward in this case.

So how should we design the Feature Extractor and what are your thoughts on this?   

Hi @ylacombe thanks a lot for the guidance!

I have a small doubt/question about the `Prenet` part - 
I believe that we must add an autoregressive model in the prenet after the MEL encoder. The reasons are that 

- The speech inputs(mel spectrograms) will be passed to the prenet and then to the speech transformer(`feature extractor->prenet->speech transformer`) and since the speech transformer has an embedding layer at the front, it will give us error if we directly pass the outputs of the [mel encoder](https://github.com/neonbjb/tortoise-tts/blob/3c4d9c51316cd2421cc2dea11ac3a7a2d3394acd/tortoise/models/autoregressive.py#L198) to the transformer. So we must use the autoregressive model to convert the outputs of the `mel encoder` into `codes` and then pass it to the `speech transformer`. 
- Also the CLVP speech transformer was trained to differentiate between `text tokens `and the `codes` generated by an `gpt2` so it only makes sense to have the `gpt2` model in the pipeline.   


So to wrap it up I would suggest something like - 

```python
from transformers import GPT2LMHeadModel    # the autoregressive model

class CLVPPrenet(nn.Module):
    def __init__(cfg):
        super().__init__()
        self.mel_encoder = CLVPMelEncoder(cfg)
        self.autoreg = GPT2LMHeadModel(cfg)
   def forward(self, voice_mel_spectrograms, text_tokens):
       x = self.mel_encoder(voice_mel_spectrograms)
       # similar to this line - https://github.com/neonbjb/tortoise-tts/blob/3c4d9c51316cd2421cc2dea11ac3a7a2d3394acd/tortoise/api.py#L416
       codes = self.autoreg.generate(x, text_tokens)
       
       return codes
```

And the `CLVPFeatureExtractor` will only care about outputting `mel spectrograms` as you said.

Please let me know what do you think of this design.

Hi @susnato,
Indeed, it makes sense to add all the submodels that are used by the model, so I agree to add them to your modeling code.

One note though: you shouldn't mix forward and generate in your forward pass, as this mixes two different concepts and as users might want to pass some specific generate kwargs when generating.

A possible solution for that is to bypass the use of the `CLVPPrenet` and to directly use `CLVPMelEncoder` and `GPT2LMHeadModel` in your final model.  (and at the end of the day, using them during the final model.generate)

Two last things to consider is that:
1. you should check that the gpt2 model of the original code indeed corresponds to our `GPT2LMHeadModel` before using it
2. If it corresponds, you might want to load it with the `AutoModel` class (which will load `GPT2LMHeadModel` if the config is correctly set!)


Let me know if you have more questions!


Hi @ylacombe first of all apologies for the huge delay. I have pushed some major changes - 

- Added Feature Extractor and the tests
- Added Processor and the tests
- Added `CLVPAutoRegressiveLMHeadModel` which is just a `GPT2LMHeadModel` but with changes to make sure that the outputs are same
- Added a `generate()` method to the `CLVPModel` which calls the `CLVPAutoRegressiveLMHeadModel`'s generate method to get `speech_ids` and then processes it using the speech model.
- reworked the tests.

Hi @ylacombe , I have addressed all of your comments except these 3 - 
- For [this](https://github.com/huggingface/transformers/pull/24745#discussion_r1313185072) and [this](https://github.com/huggingface/transformers/pull/24745#discussion_r1314639079) I am still waiting for the views/thoughts of  @ArthurZucker, @sanchit-gandhi and @amyeroberts before proceeding.
- For the [comment](https://github.com/huggingface/transformers/pull/24745#discussion_r1312992773) related to adding `CLVPTokenizerFast`, I have asked some additional questions in that thread.

Please review it and let me know if more changes are needed or not other than those 3.

Hi @ylacombe, I have pushed the changes you asked in your last review and answered the questions in their respective thread.
> Anyways, I left some small comments, mostly nits. My last request would be to verify that your model also work when passing multiple audio inputs, since I only code snippets with mostly one sample. Does batching correctly work with CLVP ?

As I see this, there are 4 possible batching scenarios :
- [x] 1 Text vs N Audios 
Here we just repeat the text tokens N times, such that we are generating different responses of the same text with different voices.
- [x] N Texts vs 1 Audio
Here we just repeat the audio N times, such that we are generating responses for multiple texts with the same voice.
- [x] N Texts vs N Audios
Here we generate the response in a way that the 2nd response will be generated using the 2nd text and the 2nd audio.
- [ ] N Texts vs M Audio
This is where it gets messy. We throw a [Value error here](https://github.com/susnato/transformers/blob/fbbff32b5d611a08804c1be7a63498901040753a/src/transformers/models/clvp/modeling_clvp.py#L520) because we don't know which text correspond to which audio. We can improve this by considering that each text corresponds to every audio and generating N*M speech candidates and comparing them with N Texts.

So , yes the batching works for CLVP except the last scenario(as of now).

Hi @ArthurZucker, just to be clear on the naming part - 
We will have 3 configs - `ClvpTextAndSpeechConfig`, `ClvpDecoderConfig` and `ClvpConfig` to bind them. 

For the models we will have `ClvpTextAndSpeechModel` (which is the `CLVPTransformerWithProjection`), `ClvpDecoder`,  `ClvpDecoderLMHead` and will keep the `ClvpModel` as it is.

Do you approve these naming? 

Let's forget about the `TextAndSpeech` it's just a general transformers. We could just have a `ClvpEncoderConfig`  (encoder that can be used for both the speech and the text encoder)  `ClvpDecoderConfig` and the `ClvpConfig`. 
Regarding the modeling, `ClvpEncoder`, `ClvpDecoder` ,  `ClvpPretrainedModel`, ,`ClvpModel` and `ClvpModelForConditionalGeneration` . You should really have a look at `MusicGen`'s modelling code 😉 

Hi @ArthurZucker, I have refactored both the `modeling` code and `checkpoint conversion` code and worked on your comments. 
For the modeling code I have taken inspiration from the `MusicGen` and for the checkpoint script I have followed the `Whisper` (as you suggested). 

Hi @ylacombe, I have pushed the changes and answered the remaining question [here](https://github.com/huggingface/transformers/pull/24745#discussion_r1340236251). 

Hey @susnato, let's discuss the remaining question [here](https://github.com/huggingface/transformers/pull/24745#discussion_r1340236251) before asking @ArthurZucker to review!

Hey @susnato - would you mind marking all completed conversations as 'resolved'? This greatly helps the next reviewer know which parts of the PR are still pending, and which are complete. I'll endeavour to provide you with a review as soon as possible after this! Ping me as soon as you've done so and I'll get you a review 🤗

Hi @sanchit-gandhi , I have marked all completed conversations as 'resolved'. Here is a brief explanation about the changes from your last review - 
- Feature Extractor and Tokenizer of `tortoise-tts` have been implemented for this model.
- we have incorporated the `autoregressive model` and `conditional_encoder` from `tortoise-tts` here. They are called `ClvpDecoder` and `ClvpConditioningEncoder` respectively. Also we have introduced `ClvpDecoderConfig` to store the configs for decoder.
- The Clvp text and speech model's have been squeezed into a single `ClvpEncoder` to reduce reduncancy also their configs(`clvptextconfig` and `clvpspeechconfig`) have been reduced to `ClvpEncoderConfig`.
- We have implemented a single model `ClvpForConditionalGeneration` which takes care for the decoder and encoder models. `ClvpForConditionalGeneration.generate()` first calls the generate of the `ClvpDecoder`(`autoregressive_model`) and generates the `speech_candidates` and right after that it applies the both text and speech encoders (`ClvpEncoder`) to filter out the best candidates (just as clvp does.)

Hi @sanchit-gandhi , I have pushed the changes except [this](https://github.com/huggingface/transformers/pull/24745#discussion_r1344603740), [this](https://github.com/huggingface/transformers/pull/24745#discussion_r1344246928) and [this](https://github.com/huggingface/transformers/pull/24745#discussion_r1343917195).
I have mentioned the reasons in their respective threads.

Also I have marked all implemented comments as resolved.

Hi @sanchit-gandhi , I have modified the tokenizer to output two sets of `input_ids`. Let me know if you are ok with the change.

Hi @ArthurZucker, I have pushed the changes that you requested in the last review.

Okay I'll review again then 

Hello @ArthurZucker, I have simplified the logic of the tokenizer.(I have reverted back to the previous version where we only pass `input_ids` and add bos and eos for `ClvpConditioningEncoder`)
Also I have worked on your comments 

Please review it and let me know if this works with you. 

Gentle ping @ArthurZucker for a follow up review. :)


Yes!

Hi @ArthurZucker, I have pushed the changes that you requested in the last review. 

Let's just fix the CIs and we can merge

Hello @ArthurZucker, the CI is green now! :hugs:  

Hi @ArthurZucker, I have pushed some changes related to using `_prepare_4d_causal_attention_mask` and slightly reworked the `.md` file.

Please let me know if we need any more changes before we can merge. 

Awesome, I'll review these and wait for the CI to go green! 🚀 

 The CI is red but the error seems to be unrelated to this PR.
Rebasing again.

(Failing tests are most probably not related our ci has some trouble today! )

> (Failing tests are most probably not related our ci has some trouble today! )

Ok, should I still rebase one more time or are we good to go? :) @ArthurZucker 

One more time, please! Apologies for the disruption this has caused in getting this merged. 

Hi @ArthurZucker, @amyeroberts I just rebased and the CI is green now! 

@susnato 🙌  

Thanks for your patience regarding the failing CI and your work adding the model!