Adding Llava to transformers
Author: shauray8
Pull request: yes

# What does this PR do?
Adds LLAVA to transformers.
author -  https://github.com/haotian-liu/LLaVA
hub - https://huggingface.co/shauray/Llava-Llama-2-7B-hf

Fixes # (issue)
#25060

## Before submitting
- [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.
- [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?
@ArthurZucker @amyeroberts 
@younesbelkada


## Results 
![image](https://llava-vl.github.io/static/images/view.jpg)
*prompt* - "How would you best describe this image? "
<br>
-- The photograph shows a wooden dock floating on the water, with mountains in the background. It is an idyllic scene that captures both natural beauty and human-made structures like docks at their most serene state of being surrounded by nature's wonders such as lakes or oceans (in case it isn’t just any body). This type of setting can be found all over North America where there are numerous bodies of freshwater available for recreational activities including fishing from piers near these locations; however, they also provide opportunities to observe wildlife




Comments:
@ArthurZucker Right now I've added Llava support directly to the MPT model as `LlavaMptForCausalLM`. Do you think I should add llava as a separate model or is this good enough?

And there's no preprocessor_config unfortunately, so do I go about making one and push it to a new hugging face repo or just integrate all the CLIP preprocessing and Tokenize in the class itself?

Okay, on it! 🫡 

Got caught up with something, but this should be ready for reviews @ArthurZucker. There are some quality errors for unrelated files, and a couple of minor errors as well, Do let me know how I go about solving them.

**A little update, It's llama-based llava and not mpt (making it work with mpt was a pain).

@shauray8 Nice work! 

Are you able to open the Circle CI pages and see the errors? 

For the [check_repo_consistency](https://app.circleci.com/pipelines/github/huggingface/transformers/72701/workflows/45ed2af4-319c-49e2-98a1-e884c8d91403/jobs/917908) tests, you'll see that `make fix-copies` needs to be run. 

For [tests_torch](https://app.circleci.com/pipelines/github/huggingface/transformers/72701/workflows/c910e0e2-b54d-457b-846b-bab9a301719e/jobs/917925) you'll see that `self.model_tester.batch_size` is referenced but not defined. Make sure to run the tests, including the slow test, locally to check if they pass.  See `8. Adding all necessary model tests` on the [add_new_model](https://huggingface.co/docs/transformers/add_new_model) page

The PR documentation tests look like there might just have been a transient timeout. I'm going to re-run them for you. 

Here's the documentation on how to run tests: https://huggingface.co/docs/transformers/testing 

Please make sure all tests are passing before asking for review. From next week, I'll be away for a few weeks. When ready please ask @rafaelpadilla for review and then we can ask for a core maintainer review. If you need any help or have questions regarding getting the tests to pass, please ping us! 

Thank you @amyeroberts for the help, PR documentation still seems to be in some kind of timeout (I'm not very sure how to fix that). @rafaelpadilla Other than that everything should more or less look green, Maybe you can help me fix this issue.

Edit -  `tests_pr_documentation_tests FAILED src/transformers/models/llava/configuration_llava.py::transformers.models.llava.configuration_llava.LlavaLlamaConfig`

@shauray8 It looks like the test runner is crashing when running the code example in the LlavaLlamaConfig class. The default params are for a 7B model, which is large. As this is the smallest model `src/transformers/models/llava/configuration_llava.py` should be added to `not_doctested.txt`

CI looks green!

Update @ArthurZucker @rafaelpadilla -
- added Llava 13B to the hub https://huggingface.co/shauray/Llava-Llama-2-13B-hf

Not sure why tf and flax tests say `E   No module named 'torch'` 

> Update @ArthurZucker @rafaelpadilla -
> 
> * added Llava 13B to the hub https://huggingface.co/shauray/Llava-Llama-2-13B-hf
> 
> Not sure why tf and flax tests say `E No module named 'torch'`

Hi @shauray8 ,

I'm going to look into this. :) 

> > Update @ArthurZucker @rafaelpadilla -
> > 
> > * added Llava 13B to the hub https://huggingface.co/shauray/Llava-Llama-2-13B-hf
> > 
> > Not sure why tf and flax tests say `E No module named 'torch'`
> 
> Hi @shauray8 ,
> 
> I'm going to look into this. :)

Seems that the error `E No module named 'torch'` is gone.

The current error [now](https://app.circleci.com/pipelines/github/huggingface/transformers/73673/workflows/0717e39e-1dd9-432e-a89a-6bd72cd5a66c/jobs/932101) is `NameError: name 'CLIPVisionModel' is not defined`.
To resolve this, you can remove the `is_openai_available()` [here](https://github.com/huggingface/transformers/blob/4baba5a8d772bf48ca3829bcbfd926267e22dc7c/src/transformers/models/llava/processing_llava.py#L32). Is `openai` package really needed?

removing `is_openai_available` results in `E   cannot import name 'CLIPVisionModel' from 'transformers.models.clip'`

Hi @shauray8 ,

Taking a look at the tests on the CI side, I noticed that in `test_processor_llava.py`, the class `CLIPVisionModel` is being called [here](https://github.com/huggingface/transformers/blob/4baba5a8d772bf48ca3829bcbfd926267e22dc7c/tests/models/llava/test_processor_llava.py#L48). However, torch is required to run `vision_model = CLIPVisionModel.from_pretrained("openai/clip-vit-large-patch14")`.

To fix that, [here](https://github.com/huggingface/transformers/blob/4baba5a8d772bf48ca3829bcbfd926267e22dc7c/tests/models/llava/test_processor_llava.py#L41C1-L43C1) you need to replace 
```python
@require_vision
class LlavaProcessorTest(unittest.TestCase):
```
by
```python
@require_torch
class LlavaProcessorTest(unittest.TestCase):
```
and import:
```python
from transformers.testing_utils import require_torch
```

Please, try to run that and see if it works now :) 

Thank you @rafaelpadilla for looking into this, CI looks green, PR's ready for review 

@rafaelpadilla LLaVA v1.5 was just released, but it has some architectural changes, should I modify the script to make it work with v1.5?

https://twitter.com/ChunyuanLi/status/1710299381335798202

![image](https://pbs.twimg.com/media/F7w0OBzXEAAHNtB?format=jpg&name=large)

Hi @shauray8 

Yep. Llava 1.5 has just come out of the oven :) 

I haven't compared the code of the old and new versions, but changes don't seem to be huge. 

As there's a big expectation to have Llava available on the library very soon, just do this move if it does not extend our releasing much longer.

So, feel free to go for Llava 1.5 by adapting what we already have in the current PR. 

The processor may not be affected, so my last review is still valid. Please, check it. 

The whole community is very excited to see Llava available in the library. Please, let me know if you need help with anything. 


@rafaelpadilla At first glance I can only see an extra Linear Layer, I have my exams going on but I'll try my best to get it done as soon as I can. 


@rafaelpadilla I think I have done everything we've discussed above and on top of that I've added support for `Llava V1.5`

@rafaelpadilla I've added all the docstrings

@rafaelpadilla CI's green, I've added support for `openai/clip-vit-large-patch14-336` alongside `openai/clip-vit-large-patch14` as Llava-v1.5 prefers `openai/clip-vit-large-patch14-336` 

> I've looked a bit into the design of this PR (as well as the LLaVa paper). First of all, I really appreciate the effort you're making into integrating it into the library 🙏 already some nice work.
> 
> However, for the model to get integrated into the library, there are some changes to be made. Specifically, I see you're using the `vision_model` (CLIP) inside the preprocessor class. This is very different from all other models in the Transformers library, and not compliant to its design. What should actually be done is defining something along the lines of:
> 
> ```
> class LlavaModel(config):
>      def __init__(self, config):
> 
>         self.vision_model = LlavaVisionModel(config.vision_config)
>         self.projection_layer = nn.Linear(...)
>         self.text_model = AutoModelForCausalLM(config.text_config)
> ```
> 
> for the base model (i.e. LLaVa without language modeling head on top), and then the head model:
> 
> ```
> class LlavaForCausalLM(config):
>     def __init__(self, config):
>              
>           self.model = LLavaModel(config)
>           self.lm_head = nn.Linear(...)
> ```
> 
> i.e. the `vision_model` is a PyTorch model, hence it needs to be part of the PyTorch implementation of LLaVa. The `LlavaProcessor` class should combine a `CLIPImageProcessor` and a `LlamaTokenizer`, which takes in text and images and produces input_ids, pixel_values which are the inputs to the model. Refer to implementations like BLIP, BLIP-2 as examples of other multimodal models which also leverage CLIP as vision encoder, combined with a language model.
> 
> The `LlavaVisionConfig` then includes all attributes regarding the vision encoder (very similar to `Blip2VisionConfig`). Since the language model is just LLaMa as a decoder-only model, one can leverage the AutoModelForCausalLM class to support any decoder-only LLM (this was also done for BLIP-2 - see [here](https://github.com/huggingface/transformers/blob/3e93dd295b5343557a83bc07b0b2ea64c926f9b4/src/transformers/models/blip_2/modeling_blip_2.py#L1571)), and specify any AutoConfig as text config (see BLIP-2 as [example](https://github.com/huggingface/transformers/blob/main/src/transformers/models/blip_2/configuration_blip_2.py#L323)). Additional attributes, like things regarding the projection layers, can be defined as part of `LlavaConfig`.

Thank you @NielsRogge for the review, I had my doubts regarding this, I'll make all the necessary changes as soon as I can.

To make sure I understand everything, rather than having a `LlavaTextModel` for LLaMA I should have it through `AutoConfig` and basically copy all the code from `CLIP` for `LlavaVisionModel` and have a `LlavaModel` for a bare base model. 

Yes, most importantly is to remove the vision encoder from the preprocessor class and instead make it part of the model.



@rafaelpadilla I'm still working on it, I'll let you know when it's ready for review

I also want to contribute to llava implementation. But I have a question, why we need to copy the vision encoder part but use AutoModel for language model part? e.g., blip, blip_2...

Hi @shauray8, I'm working on doing some Llava training experiments and hopefully contributing. 

Could you share any guidance on how to test this Llava implementation in its current state starting from scratch from just pytorch weights (https://huggingface.co/liuhaotian/llava-v1.5-7b/tree/main) and CLIP?
Like which of the weight conversion scripts to run in what order, and do inference? I've been trying the examples in docs and in the tests, but they seem to yield NaN tensors or various other errors due to processor inconsistency, etc.
Thank you!

Hi @shauray8 any updates on this model addition? We'd like to have this model merge in within the next few weeks - is this something that would fit in your timeline? 

@amyeroberts I'm done with the architectural changes @NielsRogge suggested, uploading new weights for LLaVa and LLaVa 1.5, writing new tests and documentations could take up 2-3 days as I'm pretty caught up with my placements

Hi @shauray8 - glad to hear the arch changes are done! I can see that there's still outstanding suggestions from @rafaelpadilla's PR which will also need to be addressed alongside tests etc. 

As there are currently 3 in-progress model PRs - #25001, #26360, #25789 - all of which we'd like to have in the library soon, and you mention you're busy with placements I propose that you continue with one and someone else can help finish off the other PRs. As Llava is the most complete and has already had some reviews this is the one I suggest you focus on. 

Let us know if you need any help! 

Hi @shauray8 ! 
Great work on the PR ! I am super excited about this architecture and I am happy to help you finishing up the PR by taking it over, or creating a new PR to add this architecture. I'll make sure to add you as the main author of this contribution as you did most of the work. Let me know how does that sound for you 

@younesbelkada Thank you for the positive feedback! I appreciate your willingness to help in completing the PR. Opening a new PR would be a much more cleaner way of doing it but I don't mind the approach. I'm open to discussing any details or providing additional information you might need.