A shallow copy in groundingdino

[fushh]

System Info

A bug in source code

Who can help?

No response

Information

• The official example scripts
• My own modified scripts

Tasks

• An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
• My own task or dataset (give details below)

Reproduction

please see

transformers/src/transformers/models/grounding_dino/modeling_grounding_dino.py

Lines 2529 to 2534 in d1b9236

	else:
	for _ in range(config.decoder_layers):
	_bbox_embed = GroundingDinoMLPPredictionHead(
	input_dim=config.d_model, hidden_dim=config.d_model, output_dim=4, num_layers=3
	)
	self.bbox_embed = nn.ModuleList([_bbox_embed for _ in range(config.decoder_layers)])

Expected behavior

a deep copy

[Rocketknight1]

Hi @fushh I don't understand the bug from this description. Can you explain?

[fushh]

In original implementation, each _bbox_embed in self.bbox_embed is the same no matter config.decoder_bbox_embed_share is True or False, which is a shallow copy.

However, when config.decoder_bbox_embed_share=False, a deep copy is needed. An example code is as follows.

if config.decoder_bbox_embed_share:
          _bbox_embed = GroundingDinoMLPPredictionHead(
              input_dim=config.d_model, hidden_dim=config.d_model, output_dim=4, num_layers=3
          )
          self.bbox_embed = nn.ModuleList([_bbox_embed for _ in range(config.decoder_layers)])
      else:
          model_list = []
          for _ in range(config.decoder_layers):
              _bbox_embed = GroundingDinoMLPPredictionHead(
                  input_dim=config.d_model, hidden_dim=config.d_model, output_dim=4, num_layers=3
              )
              model_list.append(_bbox_embed)
          self.bbox_embed = nn.ModuleList(model_list)

Only by switching to deep copy can the llmdet model be supported.

Further, it would be grateful if you can help us integrate LLMDet into transformers.
paper: https://arxiv.org/abs/2501.18954
code: https://github.com/iSEE-Laboratory/LLMDet/tree/main/hf_model
model: https://huggingface.co/fushh7/llmdet_swin_tiny_hf
model: https://huggingface.co/fushh7/llmdet_swin_base_hf
model: https://huggingface.co/fushh7/llmdet_swin_large_hf

[Rocketknight1]

cc @qubvel @NielsRogge for GroundingDINO!

[qubvel]

Hey @fushh, according to configs we actually never have

else:
          model_list = []
          for _ in range(config.decoder_layers):
              _bbox_embed = GroundingDinoMLPPredictionHead(
                  input_dim=config.d_model, hidden_dim=config.d_model, output_dim=4, num_layers=3
              )
              model_list.append(_bbox_embed)
          self.bbox_embed = nn.ModuleList(model_list)

path activated, so it's probably better to remove this from GrouningDINO at all and redefine it in llmdet from scratch, what do you think?

[fushh]

I think fixing this bug would be a better choice because other users might prefer to make changes or fine-tune on groundingdino.

[qubvel]

Sure, we can fix it here as well. However, we prefer to simplify the modeling code by removing unused parts in already-pretrained checkpoints 🤗