[GroundingDino] Fix grounding dino loss 🚨
Author: EduardoPach
Pull request: yes
Labels: name: run-slow

# 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 -->

Fixes https://github.com/huggingface/transformers/issues/31434

As the [original repo](https://github.com/IDEA-Research/GroundingDINO) doesn't provide the loss implementation I'm using the one implemented [here](https://github.com/longzw1997/Open-GroundingDino) as a baseline since it was mentioned by the original repo, on this issue https://github.com/IDEA-Research/GroundingDINO/issues/241, as a reliable source if one wants to train a `GroundingDino` model 

TODO: 
- [x] Test `GroundingDinoMatcher` and `GroundingDinoLoss` are working properly 

## Explanation of the Issue and Solution

So the issue was that `GroundingDinoLoss` and `GroundingDinoHungarianMatcher` were just a copy from `DeformableDetr` which is used for closed-set object detection (i.e. a fixed set of categories). Whereas in `GroundingDino` there's no limited amount of categories and the output logits are `d_model` dimensional where the first `seq_len` elements have a specified value and the subsequent are `nan`. The main differences are: 

1. `class_labels` are associated with the text prompt used 
2. The logits are asscoaited with the tokens of the text so it's not necessarily 1-to-1

For instance if an image with bounding boxes with fishes and jellyfishes using a prompt `"fish. jellyfish."` fish should have `class_label` 0 assigned to it and jellyfish should have 1 assigned. If the position of jellyfish and fish in the prompt swapped then the `class_label`s would swap as well. Moreover, jellyfish is represented by two tokens (`[20919, 7529]`) and fish by one token (`[3869]`) therefore we need to select the appropriate logits for each class.


As the [original implementation](https://github.com/IDEA-Research/GroundingDINO) doesn't provide the training loop or the loss implementation, but does recommend other implementations for training `GroundingDino` on this issue https://github.com/IDEA-Research/GroundingDINO/issues/241, I took as baseline the implementation from [Open-GroundingDino](https://github.com/longzw1997/Open-GroundingDino/tree/main) as it supports both visual grounding and object detection and they've trained their own `GroundingDino` using their code base achieving good performance.

Things added in this PR are:
- `build_label_maps` which generates a list of `torch.Tensor` with lenght `batch_size` mapping each category to its corresponding tokens based on the `input_ids`
- `build_text_mask` just expand the `attention_mask` to select the appropriate tokens when computing `GroundingDino.loss_labels`
- Added `enc_topk_proposals`, `encoder_logits` and `encoder_pred_boxes` to `GroundingDinoModelOutput` and `GroundingDinoObjectDetectionOutput` to compute first stage loss
- Added `class_loss_coefficient` (with correct default value) and `class_loss_reduction` to `GroundingDinoConfig`. `class_loss_reduction` was added because in `sigmoid_focal_loss` from the baseline implementation they reduced `loss_ce` with a simple sum, but that makes the losses imbalanced most of the time and in the original implementation they do have a `sigmoid_focal_loss` implemented, but using `mean` reduction, therefore I made I decided to make it configurable and use the `sum` one for testing reasons
- Modifications to `GroundingDinoLoss` and `GroundingDinoHungarianMatcher`


Also added a new integration test called `test_grounding_dino_loss` where I compare the loss obtained from 2 sample images with the baseline implementation from `Open-GroundingDino`.

c.c. @amyeroberts 

Comments:
@amyeroberts FYI for some reason, when testing locally, `test_cross_attention_mask` is failing on this branch, but when I tested using the `main` branch it was also failing (locally)

c.c. @amyeroberts 

@EduardoPach Thanks for the working this loss. Just sharing more well developed code for finetuning GroundingDINO https://github.com/open-mmlab/mmdetection/blob/main/configs/mm_grounding_dino/grounding_dino_swin-t_pretrain_obj365.py

c.c. @amyeroberts 

Maybe @NielsRogge could have a look?

Cough cough, c.c @amyeroberts 

![Screenshot 2024-08-23 at 9 54 40 PM](https://github.com/user-attachments/assets/304d41e0-08a6-484e-bafe-53fa4d36285e)
This is the result of current commit

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_31828). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

c.c. @amyeroberts 

@EduardoPach One additional comment for the `denoising`.

OpenGroundingDino : Has configuration file of denoising related variable but not used in training
https://github.com/longzw1997/Open-GroundingDino/blob/924bb6c4b93cae2dae582e1afaeccd408c72a31d/models/GroundingDINO/groundingdino.py#L750

MMDetection GroundingDino : Has also configuration and also training pipeline such as auxiliary or encoder.
https://github.com/IDEA-Research/GroundingDINO/blob/856dde20aee659246248e20734ef9ba5214f5e44/groundingdino/models/GroundingDINO/groundingdino.py#L384

https://github.com/open-mmlab/mmdetection/blob/cfd5d3a985b0249de009b67d04f37263e11cdf3d/mmdet/models/dense_heads/grounding_dino_head.py#L601

Denoising pipeline is for helping faster convergence so it will be okay to fine-tune without this feature. Current version of this PR is lack of denoising pipeline. We need to also aware that adding denoising pipeline as TO DO 👍🏼 

Hey! 🤗 Thanks for your contribution to the `transformers` library!

Before merging this pull request, slow tests CI should be triggered. To enable this:
- Add the `run-slow` label to the PR
- When your PR is ready for merge and all reviewers' comments have been addressed, push an empty commit with the command `[run-slow]` followed by a comma separated list of all the models to be tested, i.e. `[run_slow] model_to_test_1, model_to_test_2`
  - If the pull request affects a lot of models, put at most 10 models in the commit message
- A `transformers` maintainer will then approve the workflow to start the tests

(For maintainers) The documentation for slow tests CI on PRs is [here](https://www.notion.so/huggingface2/CI-for-pull-requests-8335bd9217d24d1e8ed1d3a016f39804).

@qubvel Hi Pavel I have got delegated the permission from @EduardoPach So I will be also working this PR to be get merged. There is one issue from the CI that is related to permission otherwise it is good to go. (Do you know about the permission error?)

Hi @SangbumChoi, got it! Can you please drop the two latest commits + `rebase` the branch to the `main` locally (instead of merging), then force push changes? Otherwise, I see that 862 files changed in this PR 😄 
Meanwhile, I will try to figure out the test fetch error.

Thanks, the diff looks better!

I suppose tests issues are in the debugging stage
 - https://github.com/huggingface/transformers/pull/33862

Are we still waiting for the slow tests? cc @SangbumChoi 

@ydshieh can you help with the tests? fetch tests is showing an Unauthorized error

Hi @EduardoPach and @SangbumChoi! Can you please **rebase** PR on the `main` branch on transformers? This should resolve an issue with tests running

Slow tests are failed 🥲 I suppose most of them in the same state on the main, but the loss test is also failed, can you have a look? At least we need to add a better message in assert to see the diff.

@qubvel Sorry, the problem was not synchronize the device type of `model` and the `input`.
Resolved by https://github.com/huggingface/transformers/pull/31828/commits/de356aeacc2cffbd253f1e8cd4e0f62205a25cf2

Strange thing is that local CLI returns follows but the CI pass on this 
```
root@ccb8b39e6c2d:/mnt/nas2/users/sbchoi/transformers# RUN_SLOW=1 pytest tests/models/grounding_dino/test_modeling_grounding_dino.py
```
```
========================================= short test summary info =========================================
FAILED tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_cross_attention_mask - AssertionError: False is not true
FAILED tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_grounding_dino_loss - AssertionError: False is not true
FAILED tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_inference_object_detection_head_equivalence_cpu_gpu - AssertionError: False is not true
==================== 3 failed, 46 passed, 109 skipped, 3 warnings in 211.76s (0:03:31) ====================
```

Can you also confirm about this? -> turns out that CI was having some delay (Local was correct)

I have these ones also failed locally, making a fix for cross_attention + batch equivalence
```
========================================= short test summary info =========================================
FAILED tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_cross_attention_mask - AssertionError: False is not true
FAILED tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_grounding_dino_loss - AssertionError: False is not true
FAILED tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_inference_object_detection_head_equivalence_cpu_gpu - AssertionError: False is not true
==================== 3 failed, 46 passed, 109 skipped, 3 warnings in 211.76s (0:03:31) ====================
```

Two tests failed in CI

```
FAILED tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_grounding_dino_loss - AssertionError: False is not true
FAILED tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_inference_object_detection_head_equivalence_cpu_gpu - AssertionError: False is not true
```

For cpu-gpu test I have pretty large diffs locally and even `nan` (on main)
```python
{
    "logits": tensor(0.0),
    "pred_boxes": tensor(0.8244),
    "last_hidden_state": tensor(0.9422),
    "init_reference_points": tensor(0.7221),
    "intermediate_hidden_states": tensor(2.5513),
    "intermediate_reference_points": tensor(0.8300),
    "encoder_last_hidden_state_vision": tensor(0.0001),
    "encoder_last_hidden_state_text": tensor(9.5665e-06),
    "enc_outputs_class": tensor(1.8090e-05),
    "enc_outputs_coord_logits": tensor(nan),
}

```

@qubvel 

> tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_inference_object_detection_head_equivalence_cpu_gpu

is failing on our CI (`main`)

which could be find in `#transformers-ci-daily-models` channel

@ydshieh @qubvel Currently I'm debugging why this failure happens

@SangbumChoi 

> tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_inference_object_detection_head_equivalence_cpu_gpu

is failing for quite long time 😅 but if you are willing to check it we appreciate a lot, but it's not necessary to merge this PR.

(but `test_grounding_dino_loss` yes)



> is failing for quite long time 😅 but if you are willing to check it we appreciate a lot, but it's not necessary to merge this PR.

@ydshieh 
For the aforemention error of this thing. I am not 100 % sure but I think it is related to somehow seed problem. The reason why I think this way is because even though I did not change any difference, somtimes the test pass.

@SangbumChoi do you mean the loss test is random seed dependant? In that case, we can either slightly increase the tolerance or mask it as `@is_flaky()`

> do you mean the loss test is random seed dependant? In that case, we can either slightly increase the tolerance or mask it as @is_flaky()

@qubvel Still not sure about it. I will debug deeper more tomorrow but the fact is that sometime the CI pass and sometime fails 😭 (Or there might be a computing issue with torch.backends.cudnn.deterministic = True)

Ok, sure!

HI @SangbumChoi , from the `test_grounding_dino_loss`, you have the `"loss_ce_enc": torch.tensor(16226.3145),`:
the scale of the loss is way to big from other loss? Isn't the trained network should have smaller scale loss or is there something wrong with the implementation?

Or does the `loss` actually need to include `loss_ce_enc` since the first stage is just to used for the regional proposal?


@stevenwudi 
> loss actually need to include loss_ce_enc since the first stage is just to used for the regional proposal?

Yeah, otherwise can you explain more detail about the reason why `"loss_ce_enc"` should be low? (I am also open to discuss this circumstances!)

> Yeah, otherwise can you explain more detail about the reason why `"loss_ce_enc"` should be low? (I am also open to discuss this circumstances!)

with the current `sum` as reduction, the `loss_ce_enc` will be several scales larger than all the rest of the loss and this will literally makes other loss neglible which will hinders training.

Maybe a better term is why we are not using `mean` as the `class_loss_reduction` default since the Open-GroundingDino seem to have a `mean`?
https://github.com/longzw1997/Open-GroundingDino/blob/main/models/GroundingDINO/utils.py#L168

@stevenwudi Yeah good point. Even though the loss scale is large, eventually it trains well at my experiment, but as you suggested it could be better if we set `mean`. Do you have some time for to experiment between the effectiveness of `mean` and `sum`? I will change that default as `mean`. cc. @EduardoPach 

> @ydshieh Yeah good point. Even though the loss scale is large, eventually it trains well at my experiment, but as you suggested it could be better if we set `mean`. Do you have some time for to experiment between the effectiveness of `mean` and `sum`? I will change that default as `mean`. cc. @EduardoPach

(Actually, I don't have any experiment stats to support, and I am happy to know that you did train well at your experiment)
But what I observer is that with the `sum` reduction method, the first stage `loss_ce` is really large where as the second stage `loss_ce` lie in a more sensible scale. Any ideas? @SangbumChoi 

> I will change that default as mean 

@SangbumChoi  I did this before but switch back to default `sum`. Why? Because using `mean` yielded a terrible result in my experiments with `aquarium` dataset and because `Open-GroundingDino` uses `sum` as default

>  with the current sum as reduction, the loss_ce_enc will be several scales larger than all the rest of the loss and this will literally makes other loss neglible which will hinders training.

@stevenwudi  first of all thanks for spending your time and looking into the code! I had the same thought, but then after some experiments (as mentioned above) I set `sum` as default. Why? Because `sum` is the default in `Open-GroundingDino` .

>  https://github.com/longzw1997/Open-GroundingDino/blob/main/models/GroundingDINO/utils.py#L168

This is not use in their code. See [here](https://github.com/longzw1997/Open-GroundingDino/blob/main/models/GroundingDINO/groundingdino.py#L498). Thus the expected value, since we're basing ourselves in `Open-GroundingDino` is pretty high, but converges reasonably fast (see attached img). 

Btw if you actually look the `encoder_logits` they're pretty similar to `logits` because the `encoder_logits` the first stage output that then gets refined on the second stage and generates the final `logits`


<img width="522" alt="image" src="https://github.com/user-attachments/assets/3761c75b-e080-47a4-a4ca-d41212932e12">



@EduardoPach thanks for the detailed explaination, code link and the training loss, this is super helpful.

nit: curious for the extremely large value of `loss_ce_enc` for a pretrained model, why do you think the scale is so much larger (hundreds of times the scale of other loss).
My understanding is the first stage and second stage `loss_ce` are doing very similar things: (quote from deformable DETR):
```
Two-Stage Deformable DETR. In the original DETR, object queries in the decoder are irrelevant
to the current image. Inspired by two-stage object detectors, we explore a variant of Deformable
DETR for generating region proposals as the first stage. The generated region proposals will be fed
into the decoder as object queries for further refinement, forming a two-stage Deformable DETR.
In the first stage, to achieve high-recall proposals, each pixel in the multi-scale feature maps would
serve as an object query. However, directly setting object queries as pixels will bring unacceptable
computational and memory cost for the self-attention modules in the decoder, whose complexity
grows quadratically with the number of queries. To avoid this problem, we remove the decoder and
form an encoder-only Deformable DETR for region proposal generation. In it, each pixel is assigned
as an object query, which directly predicts a bounding box. Top scoring bounding boxes are picked
as region proposals. No NMS is applied before feeding the region proposals to the second stage.
```

Hence, I can not help but wonder: does the high `loss_ce_enc` means that there is almost little generalization of the groundingdino wrt. the cross attention logit input. Could there be something fundamentally flaw with the grounding dino formulation?


> This is not use in their code. See [here](https://github.com/longzw1997/Open-GroundingDino/blob/main/models/GroundingDINO/groundingdino.py#L498).

Hmm, good to know, it seems that the Open-Groundingdino code is kinda really messy, glad we will have this HF implemetation soon 👍 


@ydshieh @qubvel @EduardoPach The reason of failing the CI test was the difference between cpu and gpu. There is a slight difference from the beginning part (e.g. `load_backbone` part) of the architecture. (There is also slight difference in text part also when we change atol into 1e-7) This difference keeps get larger and larger to the head part and makes the non-negligible difference. This difference also makes to pick the different topk proposal also.

```
for i in outputs.keys():
    try:
        difference = cpu_outputs[i] - outputs[i].cpu()
        print(f"Difference: {(difference>1e-6).sum():10} Name: {i:40} Size: {cpu_outputs[i].size()}")
    except:
        continue
```
```
Difference:          1 Name: loss                                     Size: torch.Size([])
Difference:      15992 Name: logits                                   Size: torch.Size([2, 900, 256])
Difference:       1839 Name: pred_boxes                               Size: torch.Size([2, 900, 4])
Difference:     201506 Name: last_hidden_state                        Size: torch.Size([2, 900, 256])
Difference:         57 Name: init_reference_points                    Size: torch.Size([2, 900, 4])
Difference:    1056660 Name: intermediate_hidden_states               Size: torch.Size([2, 6, 900, 256])
Difference:      10381 Name: intermediate_reference_points            Size: torch.Size([2, 6, 900, 4])
Difference:      84492 Name: encoder_last_hidden_state_vision         Size: torch.Size([2, 23890, 256])
Difference:          0 Name: encoder_last_hidden_state_text           Size: torch.Size([2, 21, 256])
Difference:          2 Name: encoder_topk_proposals                   Size: torch.Size([2, 900])
Difference:     223762 Name: enc_outputs_class                        Size: torch.Size([2, 23890, 256])
Difference:      40176 Name: enc_outputs_coord_logits                 Size: torch.Size([2, 23890, 4])
Difference:      14181 Name: encoder_logits                           Size: torch.Size([2, 900, 256])
Difference:         57 Name: encoder_pred_boxes                       Size: torch.Size([2, 900, 4])
```
----------------------------

```
for i in outputs.keys():
    try:
        difference = cpu_outputs[i] - outputs[i].cpu()
        print(f"Difference: {(difference>1e-3).sum():10} Name: {i:40} Size: {cpu_outputs[i].size()}")
    except:
        continue
```

```
Difference:          1 Name: loss                                     Size: torch.Size([])
Difference:       4110 Name: logits                                   Size: torch.Size([2, 900, 256])
Difference:         97 Name: pred_boxes                               Size: torch.Size([2, 900, 4])
Difference:      38021 Name: last_hidden_state                        Size: torch.Size([2, 900, 256])
Difference:          8 Name: init_reference_points                    Size: torch.Size([2, 900, 4])
Difference:     120993 Name: intermediate_hidden_states               Size: torch.Size([2, 6, 900, 256])
Difference:        329 Name: intermediate_reference_points            Size: torch.Size([2, 6, 900, 4])
Difference:          0 Name: encoder_last_hidden_state_vision         Size: torch.Size([2, 23890, 256])
Difference:          0 Name: encoder_last_hidden_state_text           Size: torch.Size([2, 21, 256])
Difference:          2 Name: encoder_topk_proposals                   Size: torch.Size([2, 900])
Difference:          0 Name: enc_outputs_class                        Size: torch.Size([2, 23890, 256])
Difference:          0 Name: enc_outputs_coord_logits                 Size: torch.Size([2, 23890, 4])
Difference:          0 Name: encoder_logits                           Size: torch.Size([2, 900, 256])
Difference:          8 Name: encoder_pred_boxes                       Size: torch.Size([2, 900, 4])
```

@EduardoPach I think the test_grounding_dino_loss is the same issue as I stated above. So for the solution we can recalculate based on the gpu, enlarge the tolerance, or remove this testing function

Additional analysis for cpu/gpu difference

```
for i in range(len(backbone[0])):
    i, j = backbone[0][i][0], cpu_backbone[0][i][0]
    try:
        difference = j - i.cpu()
        print(f"Difference: {(difference>4e-5).sum():10}")
    except:
        continue
```
Difference:          0
Difference:        118
Difference:         11

backbone also has some large difference at the second and last layer.

Also SwinEmbedding has 1e-6 difference even if it is the very beginning of the architecture
https://github.com/huggingface/transformers/blob/d087165db08a2ebae6bca29ea014784197511ebd/src/transformers/models/swin/modeling_swin.py#L234

@qubvel requesting for the review

> So for the solution we can recalculate based on the gpu, enlarge the tolerance, or remove this testing function

We are running CI on T4 GPU: we can update the expected values accordingly on our side.

updated the values for `test_grounding_dino_loss` and it pass now.
I will leave @qubvel to take a final look and merge when they are back.

Hi! I suppose we can merge it as tests fails are unrelated! 

The only thing it would be nice to do before merging: I see @ArthurZucker started the initiative of moving losses to a separate [module](https://github.com/huggingface/transformers/tree/main/src/transformers/loss), let's update this branch and move this loss there too. Thanks!

> Hi! I suppose we can merge it as tests fails are unrelated!
> 
> The only thing it would be nice to do before merging: I see @ArthurZucker started the initiative of moving losses to a separate [module](https://github.com/huggingface/transformers/tree/main/src/transformers/loss), let's update this branch and move this loss there too. Thanks!

Basically, just moving the loss implementation to a new file in the `transformers/loss` and properly set grounding dino in the `LOSS_MAPING`, right? @SangbumChoi do you have time to do this? Otherwise I can find some time during the weekend

It would be great if you can handle this weekend. @EduardoPach 

Hi @SangbumChoi @EduardoPach let us know if you need any help to finish this one :)

@NielsRogge Nothing special just have no time to look on... I will try to finish this in this weekdays.

Had to travel on the weekend I was supposed to finish this 😅 , should probably be able to give the final push this weekend. We can coordinate on discord if you want @SangbumChoi 

Hi @EduardoPach, thanks for the update! Is it ready or do you have something to finish?

> Hi @EduardoPach, thanks for the update! Is it ready or do you have something to finish?

Hey @qubvel, I'll take a final look into the test issues, and it will be ready. I should be able to do that this weekend.

@EduardoPach Any future plan for working this? Otherwise I can take a look

> @EduardoPach Any future plan for working this? Otherwise I can take a look

@SangbumChoi probably not in the near future. If you have the bandwidth I'd appreciate that 😄 

@qubvel @NielsRogge Hi team long time no see. I have finished this PR can you confirm the changes? Failure is unrelated
cc. @EduardoPach 

Hi @SangbumChoi! Thanks a lot for pushing this forward! I will review this week 👍 

@qubvel After the merge I can continue work https://github.com/huggingface/transformers/pull/32483 if this seems useful to HF team!

run-slow: grounding_dino

This comment contains run-slow, running the specified jobs: This comment contains run-slow, running the specified jobs:

models: ['models/grounding_dino']
quantizations: [] ...

Amazing! Thanks for the final push @SangbumChoi!