[GroundingDino] Fix grounding dino loss 🚨 #31828

EduardoPach · 2024-07-07T12:20:56Z

What does this PR do?

As the original repo doesn't provide the loss implementation I'm using the one implemented here as a baseline since it was mentioned by the original repo, on this issue IDEA-Research/GroundingDINO#241, as a reliable source if one wants to train a GroundingDino model

TODO:

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:

class_labels are associated with the text prompt used
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_labels 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 doesn't provide the training loop or the loss implementation, but does recommend other implementations for training GroundingDino on this issue IDEA-Research/GroundingDINO#241, I took as baseline the implementation from Open-GroundingDino 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

src/transformers/models/grounding_dino/modeling_grounding_dino.py

EduardoPach · 2024-07-14T12:10:29Z

@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)

EduardoPach · 2024-07-19T13:06:02Z

c.c. @amyeroberts

amyeroberts

Thanks for fixing!

Overall looks good. A breaking change, so we should append the PR title with 🚨, but I think this is acceptable as it's aligning ourselves with the recommended loss calculation.

tests/models/grounding_dino/test_modeling_grounding_dino.py

amyeroberts · 2024-07-17T20:05:16Z

tests/models/grounding_dino/test_modeling_grounding_dino.py

+        input_ids = torch.tensor([101, 3869, 1012, 11420, 1012, 1012, 102])
+        input_ids = input_ids.unsqueeze(0).expand(self.batch_size, -1)


Why switch to hard coded input ids?

Otherwise tests that use labels and, therefore, compute a loss would complain as build_label_maps (here) would return None

Perhaps you can add a comment to explain this

src/transformers/models/grounding_dino/modeling_grounding_dino.py

SangbumChoi · 2024-07-20T04:16:47Z

@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

src/transformers/models/grounding_dino/modeling_grounding_dino.py

EduardoPach · 2024-08-01T13:01:24Z

c.c. @amyeroberts

EduardoPach · 2024-08-05T12:21:25Z

Maybe @NielsRogge could have a look?

NielsRogge

Thanks for fixing!

src/transformers/models/grounding_dino/configuration_grounding_dino.py

NielsRogge · 2024-08-05T12:42:02Z

tests/models/grounding_dino/test_modeling_grounding_dino.py

+        input_ids = torch.tensor([101, 3869, 1012, 11420, 1012, 1012, 102])
+        input_ids = input_ids.unsqueeze(0).expand(self.batch_size, -1)


Perhaps you can add a comment to explain this

EduardoPach · 2024-08-10T00:17:13Z

Cough cough, c.c @amyeroberts

amyeroberts

Thanks for the continued work on this!

Main comments are about the precision and clarity of language in the docstrings and comments. It's important to write messages such that someone new to the code can understand.

tests/models/grounding_dino/test_modeling_grounding_dino.py

src/transformers/models/grounding_dino/modeling_grounding_dino.py

SangbumChoi

Overall it looks good. Let me check also in this PR #32483 for stable training convergence.

SangbumChoi · 2024-08-23T04:54:40Z

src/transformers/models/grounding_dino/modeling_grounding_dino.py

-def sigmoid_focal_loss(inputs, targets, num_boxes, alpha: float = 0.25, gamma: float = 2):
+# Similar to the one used in `DeformableDetr` but we pass `num_queries`, as `logits` are flattened
+# due to masked selection, and support different `reduction` modes.
+def sigmoid_focal_loss(


https://github.com/longzw1997/Open-GroundingDino/blob/main/models/GroundingDINO/utils.py#L138

Even though there are customization in this code, I like the current version of sigmoid_focal_loss 👍🏼

SangbumChoi · 2024-08-23T12:55:12Z

This is the result of current commit

HuggingFaceDocBuilderDev · 2024-08-27T12:38:25Z

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

tests/models/grounding_dino/test_modeling_grounding_dino.py

EduardoPach · 2024-09-03T20:52:51Z

c.c. @amyeroberts

SangbumChoi · 2024-09-04T13:56:18Z

tests/models/grounding_dino/test_modeling_grounding_dino.py

@@ -741,3 +780,53 @@ def test_cross_attention_mask(self):
        self.assertTrue(torch.allclose(outputs1.logits, outputs_batched.logits[:1], atol=1e-3))
        # For some reason 12 elements are > 1e-3, but the rest are fine
        self.assertTrue(torch.allclose(outputs2.logits, outputs_batched.logits[1:], atol=1.8e-3))
+
+    def test_grounding_dino_loss(self):
+        ds = load_dataset("EduardoPacheco/aquarium-sample", split="train")


Maybe we should move this to huggingface?

Maybe, wdyt @amyeroberts

Thanks for working on this, it is great to have it tested!
Do we actually need to load the whole dataset here? Can't we copy annotations and upload one image somewhere?

@qubvel by upload one image somewhere you mean to a hf dataset or to the tests fixtures?

yes, no, maybe?

@ydshieh can you please help, do we have any place for test assets?

@EduardoPach @qubvel I think we can do like this

transformers/tests/models/detr/test_modeling_detr.py

Line 562 in be9cf07

# We will verify our results on an image of cute cats

Hi. If IIRC, that dataset contains only 2 examples, right?

The easiest way is to have it as

hf-internal-testing/aquarium-sample

@qubvel I added you to hf-internal-testing (you have to accept the invitation) then you can create a copy of the above dataset.

amyeroberts

Thanks for fixing and all the work iterating on this!

Just two things before we're ready to merge:

This convo
Slow model tests -- Could you push an empty commit with the message [run_slow] grounding_dino?

EduardoPach · 2024-11-19T17:54:55Z

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

qubvel · 2024-11-25T12:35:23Z

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

EduardoPach · 2024-11-25T23:17:25Z

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.

SangbumChoi · 2025-01-10T05:23:57Z

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

EduardoPach · 2025-01-10T22:35:49Z

@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 😄

SangbumChoi · 2025-02-19T08:50:51Z

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

qubvel · 2025-02-19T11:15:47Z

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

qubvel

Thanks for the update @SangbumChoi! I left a few minor comments

src/transformers/loss/loss_grounding_dino.py

src/transformers/models/grounding_dino/modeling_grounding_dino.py

Co-authored-by: Pavel Iakubovskii <qubvel@gmail.com>

…ach/transformers into eduardo/fix-groundingdino-loss

SangbumChoi · 2025-02-27T04:53:41Z

@qubvel After the merge I can continue work #32483 if this seems useful to HF team!

qubvel

Thanks for fixing it @SangbumChoi! It would be super helpful for the community to continue working on the fine-tuning example 🤗

src/transformers/loss/loss_for_object_detection.py

qubvel · 2025-02-27T10:52:32Z

run-slow: grounding_dino

github-actions · 2025-02-27T10:53:45Z

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

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

EduardoPach · 2025-02-27T19:43:47Z

Amazing! Thanks for the final push @SangbumChoi!

* Starting to fix GroundingDinoLoss and GroundingDinoHungarianMatcher * More updates * More updates * fixed: GroundingDinoLoss * fixed: failing tests * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update tests/models/grounding_dino/test_modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Addressed comments * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py Co-authored-by: Sangbum Daniel Choi <34004152+SangbumChoi@users.noreply.github.com> * add: cardinality loss and make box loss as copy from * change: default for reduction loss is sum * fix: vectorized generate fake box * fix copies * Addressed comments * addressed comments * addressed one-hot * Update tests/models/grounding_dino/test_modeling_grounding_dino.py Co-authored-by: Sangbum Daniel Choi <34004152+SangbumChoi@users.noreply.github.com> * Addressed comments * fixed test * Update src/transformers/models/grounding_dino/modeling_grounding_dino.py * Update tests/models/grounding_dino/test_modeling_grounding_dino.py Co-authored-by: Pavel Iakubovskii <qubvel@gmail.com> * Starting to fix GroundingDinoLoss and GroundingDinoHungarianMatcher * More updates * More updates * fixed: GroundingDinoLoss * add: cardinality loss and make box loss as copy from * fix copies * Revert "Update tests/models/grounding_dino/test_modeling_grounding_dino.py" This reverts commit aa74c4c57c430e54cc74c414d6269edb65c73e83. * [run-slow] groundigdino * remove nestedtensor * [run-slow] groundig_dino * [run-slow] grounding_dino * [run-slow] grounding_dino * [run-slow] grounding_dino * check * check * add: enconder intermediate outputs to ImageLoss forward * add: GroundingDinoForObjectDetectionLoss in the loss directory * make style * fix the loss function * remove class_reduction since it sum is default * remove class_reduction * Update src/transformers/loss/loss_grounding_dino.py Co-authored-by: Pavel Iakubovskii <qubvel@gmail.com> * simple fix * Update src/transformers/loss/loss_grounding_dino.py Co-authored-by: Pavel Iakubovskii <qubvel@gmail.com> * minor fix * Update src/transformers/loss/loss_for_object_detection.py --------- Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> Co-authored-by: Sangbum Daniel Choi <34004152+SangbumChoi@users.noreply.github.com> Co-authored-by: Pavel Iakubovskii <qubvel@gmail.com> Co-authored-by: sangbumchoi <danielsejong55@gmail.com> Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>

zappy586 reviewed Jul 9, 2024

View reviewed changes

src/transformers/models/grounding_dino/modeling_grounding_dino.py Outdated Show resolved Hide resolved

zappy586 reviewed Jul 9, 2024

View reviewed changes

src/transformers/models/grounding_dino/modeling_grounding_dino.py Outdated Show resolved Hide resolved

EduardoPach changed the title ~~WIP - [GroundingDino] Fix grounding dino loss~~ [GroundingDino] Fix grounding dino loss Jul 14, 2024

EduardoPach mentioned this pull request Jul 14, 2024

[GroundingDino] - GroundingDinoProcessor kwargs is Broken #31952

Closed

4 tasks

amyeroberts reviewed Jul 19, 2024

View reviewed changes

EduardoPach requested a review from amyeroberts July 23, 2024 11:30

EduardoPach changed the title ~~[GroundingDino] Fix grounding dino loss~~ [GroundingDino] Fix grounding dino loss 🚨 Jul 23, 2024

SangbumChoi reviewed Jul 29, 2024

View reviewed changes

src/transformers/models/grounding_dino/modeling_grounding_dino.py Outdated Show resolved Hide resolved

NielsRogge approved these changes Aug 5, 2024

View reviewed changes

This was referenced Aug 6, 2024

Zero-shot finetuning examples #32459

Open

Adding new zero-shot examples #32483

Draft

amyeroberts reviewed Aug 15, 2024

View reviewed changes

SangbumChoi reviewed Aug 20, 2024

View reviewed changes

src/transformers/models/grounding_dino/modeling_grounding_dino.py Outdated Show resolved Hide resolved

EduardoPach requested review from amyeroberts and SangbumChoi August 22, 2024 09:53

SangbumChoi reviewed Aug 23, 2024

View reviewed changes

EduardoPach mentioned this pull request Aug 23, 2024

GroundingDino - Loss calculation exceptions #31434

Closed

4 tasks

SangbumChoi reviewed Aug 27, 2024

View reviewed changes

tests/models/grounding_dino/test_modeling_grounding_dino.py Outdated Show resolved Hide resolved

SangbumChoi reviewed Sep 4, 2024

View reviewed changes

amyeroberts added the run-slow label Sep 17, 2024

amyeroberts approved these changes Sep 17, 2024

View reviewed changes

EduardoPach added 2 commits November 23, 2024 21:33

add: enconder intermediate outputs to ImageLoss forward

75b31a7

add: GroundingDinoForObjectDetectionLoss in the loss directory

8353041

SangbumChoi and others added 5 commits February 19, 2025 16:38

Merge branch 'main' into fix-grounding-dino-loss

ae916c9

make style

7792865

fix the loss function

fa92961

remove class_reduction since it sum is default

a0961f6

remove class_reduction

890934a

qubvel self-requested a review February 25, 2025 11:18

qubvel reviewed Feb 25, 2025

View reviewed changes

SangbumChoi and others added 5 commits February 26, 2025 09:44

Update src/transformers/loss/loss_grounding_dino.py

56e5b63

Co-authored-by: Pavel Iakubovskii <qubvel@gmail.com>

simple fix

207a703

Update src/transformers/loss/loss_grounding_dino.py

40b8534

Co-authored-by: Pavel Iakubovskii <qubvel@gmail.com>

Merge branch 'fix-grounding-dino-loss' of https://github.com/EduardoP…

cb983e3

…ach/transformers into eduardo/fix-groundingdino-loss

minor fix

bc79813

qubvel approved these changes Feb 27, 2025

View reviewed changes

src/transformers/loss/loss_for_object_detection.py Outdated Show resolved Hide resolved

Update src/transformers/loss/loss_for_object_detection.py

9039a4c

qubvel merged commit 222505c into huggingface:main Feb 27, 2025
23 of 24 checks passed

		input_ids = torch.tensor([101, 3869, 1012, 11420, 1012, 1012, 102])
		input_ids = input_ids.unsqueeze(0).expand(self.batch_size, -1)

[GroundingDino] Fix grounding dino loss 🚨 #31828

[GroundingDino] Fix grounding dino loss 🚨 #31828

Conversation

EduardoPach commented Jul 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Explanation of the Issue and Solution

Uh oh!

Uh oh!

Uh oh!

EduardoPach commented Jul 14, 2024

Uh oh!

EduardoPach commented Jul 19, 2024

Uh oh!

amyeroberts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SangbumChoi commented Jul 20, 2024

Uh oh!

Uh oh!

EduardoPach commented Aug 1, 2024

Uh oh!

EduardoPach commented Aug 5, 2024

Uh oh!

NielsRogge left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EduardoPach commented Aug 10, 2024

Uh oh!

amyeroberts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SangbumChoi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SangbumChoi commented Aug 23, 2024

Uh oh!

HuggingFaceDocBuilderDev commented Aug 27, 2024

Uh oh!

Uh oh!

EduardoPach commented Sep 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qubvel Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EduardoPach commented Jul 7, 2024 •

edited

Loading

qubvel Sep 10, 2024 •

edited

Loading

ydshieh Sep 23, 2024 •

edited

Loading

amyeroberts left a comment •

edited

Loading

EduardoPach commented Nov 25, 2024 •

edited

Loading

SangbumChoi commented Feb 19, 2025 •

edited

Loading