VisualBERT
Author: gchhablani
Pull request: yes
Labels: name: WIP

This PR adds VisualBERT (See Closed Issue #5095).

Comments:
Hi @gchhablani 

This is great! You can ping me if you need any help with this model.


Also, we have now added a step-by-step doc for how to add a model, you can find it here 
https://huggingface.co/transformers/add_new_model.html

Also have a look at the `cookiecutter` tool, which will help you generate lots of boilerplate code.
https://github.com/huggingface/transformers/tree/master/templates/adding_a_new_model

Hi @patil-suraj

Thanks a lot.

I wanted to add this model in 2020, but I faced a lot of issues and got busy with some other work. Seems like the code structure has changed quite a bit after that. I'll check the shared links and get back.

Hi @patil-suraj,

I have started adding some code. For comparison, you can see: https://github.com/uclanlp/visualbert/blob/master/pytorch_pretrained_bert/modeling.py.

Please look at the `VisualBertEmbeddings` and `VisualBertModel`. I have checked these using dummy tensors. I am adding/fixing other kinds of down-stream models. Please tell me if you think this is going in the right direction, and if there are any things that to need to be kept in mind.

I skipped testing the original repository for now, they don't have an entry point, and require a bunch of installs for loading /handling the dataset(s) which are huge.

There are several different checkpoints, even for the pre-trained models (with different embedding dimensions, etc.). For each we'll have to make a separate checkpoint, I guess.

In addition, I was wondering if we want to provide encoder-decoder features (`cross-attention`, `past_key_value`, etc.). I don't think it has been used in the original code, but it will certainly be a nice feature to have in case we have some task in the future which involves generation of text given an image and a text (probably there is something already).

Thanks :)

Hi @patil-suraj

I would appreciate some feedback :) It'll help me know if I am going in the right direction,

Thanks,
Gunjan

Hey @gchhablani 

I'm thinking about this and will get back to you tomorrow :)

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md) are likely to be ignored.

Unstale

Hi @LysandreJik 
Thanks for reviewing.

I'll make the suggested changes in a few (4-5) hours.

The `position_embeddings_type` is not exactly being used by the authors. They use absolute embeddings. They do have an `embedding_strategy_type` argument but it is unused and kept as `'plain'`.

Yes, almost all of it is copied from BERT. The new additions are the embeddings and the model classes.

Initially, I did have plans to add actual examples and notebooks initially in the same PR :stuck_out_tongue: I guess I'll work on it now.

@LysandreJik  Do you wanna take a final look?

:') Thanks a lot for all your help @patil-suraj @LysandreJik @sgugger @patrickvonplaten

Edit:
I will be working on another PR soon to add more/better examples, and to use the `SimpleDetector` as used by the original authors. Probably will also attempt to create `TF`/`Flax` models.

The PR looks awesome! I still think however that we should add `get_visual_embeddings` with a Processor in this PR to have it complete. @gchhablani @patil-suraj - do you think it would be too time-consuming to add a `VisualBERTFeatureExtractor` here? Just don't really think people will be able to run an example after we merge it at the moment

@patrickvonplaten  here `visual_embeddings` comes from a cnn backbone (ResNet) or object detection model. So I don't think
 we can add `VisualBERTFeatureExtractor`. The plan is to add the cnn model and a demo notebook in `research_projects` dir in a follow-up a PR.

> @patrickvonplaten here `visual_embeddings` comes from a cnn backbone (ResNet) or object detection model. So I don't think
> we can add `VisualBERTFeatureExtractor`. The plan is to add the cnn model and a demo notebook in `research_projects` dir in a follow-up a PR.

Ok! Do we have another model that could give us those embeddings? ViT maybe?

@patrickvonplaten, I am not sure about ViT as I haven't used or read about it yet. The VisualBERT authors used Detectron-based (MaskRCNN with FPN-ResNet-101 backbone) features for 3 out of 4 tasks. Each "token" is actually an object detected and the token features/embeddings from the detectron classifier layer. In case of VCR task, they use a ResNet on given box coordinates. 

Unless ViT has/is an extractor similar to this, if we could use ViT, it'd be very different from the original and might not work with the provided pre-trained weights. :/


Adding a common Fast/Faster/MaskRCNN feature extractor, however, will help with LXMERT/VisualBERT and other models I'm planning to contribute in the future - ViLBERT (#11986), VL-BERT, (and possibly MCAN). 

**Edit**: There's already an example for LXMERT: https://github.com/huggingface/transformers/blob/master/examples/research_projects/lxmert/modeling_frcnn.py which I'll build upon.

Ok! Good to merge for me then

maybe not promote it yet

@patrickvonplaten Yes, won't be promoted before adding an example.

The plan forward is to add a detector and example notebook in  `research_projects` dir in a follow-up PR.

Verified that all slow tests are passing :)

Merging!

Great model addition @gchhablani. Small note: can you fix the code examples of the HTML page?

Currently they look like this:

![image](https://user-images.githubusercontent.com/48327001/120484408-548cad80-c3b3-11eb-9238-4cb53126cc2f.png)

The Returns: statement should be below the Example: statement in `modeling_visual_bert.py`. Sorry for nitpicking ;)