[serge] Fix 8 integration tests for model `grounding_dino` failing with `output_mismatch` (tensor values differ (6), oth by sergereview[bot] · Pull Request #49886 · huggingface/transformers

sergereview · 2026-10-22T23:23:20Z

fixes https://github.com/huggingface/transformers/issues/50465\

Original CI failure

Failure group: 8 integration tests for model grounding_dinofailing withoutput_mismatch (tensor values differ (6), other (2))
tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_cross_attention_mask [single-gpu] (output_mismatch, seen 21/21)

CI traceback —

tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_cross_attention_mask

(line 787)  AssertionError: Tensor-likes are not close!

Where to watch it:

test_cross_attention_mask — Test dashboard

Relates to #49885

text_position_ids is int64 and encode_sinusoidal_position_embedding ends with .to(pos_tensor.dtype), truncating sinusoidal values to int64 and zeroing ~93% of text position embeddings (#47674). The reported diffs (up to 2.36 abs / 0.96 rel) are far beyond the ~0.03/0.5% hardware-drift precedent, and the CPU≠GPU / single≠batched mismatches at the same index (0,143,2) confirm a numerical bug, not drift.

Cast text_position_ids to text_features.dtype before encoding, matching the arange branch's float cast and the maintainer's guidance in #47675. This restores the intended float embeddings so existing test expectations become valid again.

✅ Verified on GPU

serge ran the targeted @slow test(s) on a GPU runner and opened this PR only after they passed with this patch. Each targeted test was run 5× on both the pre-patch and patched trees to rule out flakiness — the result held on every run.

Passes with this patch: https://github.com/huggingface/transformers/actions/runs/39870923251
Failed before the fix (base commit): https://github.com/huggingface/transformers/actions/runs/39863293859

Possibly related

Existing issues/PRs mentioning test_cross_attention_mask (keyword match — not verified to share a root cause):

#47675 — Fix Grounding DINO text position embeddings being truncated to int64 (PR, open, updated 2026-10-15)
#41693 — 🚨 Refactor ViT to updated standards (PR, closed, updated 2026-05-08)
#31828 — [GroundingDino] Fix grounding dino loss 🚨 (PR, closed, updated 2025-02-27)

This change was produced automatically by serge from a CI failure report. The patch was generated by an LLM and applied by serge; review before merging.

serge v0.1.0 · model: Qwen/Qwen3.8-27B:deepinfra · 21 LLM turns · 33 tool calls · 2366.2s · 951294 in / 84763 out tokens

🤖 mlinter review state

hash: fc5a3f93
commit: 81a6f4a4
review: [serge] Fix 8 integration tests for model grounding_dino failing with output_mismatch (tensor values differ (6), oth #49886 (review)

…th `output_mismatch` (tensor values differ (6), oth

HuggingFaceDocBuilderDev · 2026-10-22T23:32:23Z

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.

zucchini-nlp

Same bug and fix as in the first "related prs list", citing @molbap 's review which is still relevant i think

#47675 (comment)

@askserge, can you check the comment and fix?

zucchini-nlp · 2026-11-09T01:48:26Z

run-slow: grounding_dino

zucchini-nlp · 2026-11-09T03:06:21Z

run-slow: grounding_dino, mm_grounding_dino

github-actions

Model linter — first pass

transformers-mlinter found 2 item(s) in the model files this PR touches. These are structural conventions a maintainer would otherwise flag by hand.

This is automated and advisory — it does not block merging.

rule	count	what it checks
`TRF058`	2	Buffers must be declared as nn.Buffer attributes, not registered with register_buffer().

zucchini-nlp · 2026-11-09T03:51:43Z

@guarin pls take a look since Pablo is off, and lets merge if it's fine 😄

guarin

Logic lgtm! But the slow tests don't seem to pass yet no? I guess they might need new expectations due to the position id fix.

A fast regression test checking get_text_position_embeddings might avoid introducing the same issue again but maybe not needed if it is already covered by slow tests.

guarin · 2026-11-09T09:18:09Z

-def generate_masks_with_special_tokens_and_transfer_map(input_ids: torch.LongTensor) -> tuple[Tensor, Tensor]:
+def generate_masks_with_special_tokens_and_transfer_map(
+    input_ids: torch.LongTensor, special_tokens: torch.LongTensor | None = None
+) -> tuple[Tensor, Tensor]:


Could make special_tokens required here as it was already broken?

guarin

Approving to not block the pr

…ations - `generate_masks_with_special_tokens_and_transfer_map` now requires `special_tokens` - drop the buffer comment - add fast tests for phrase masks / position ids and for float text position embeddings - update detection-head expectations for the position-id fix (grounding_dino + mm_grounding_dino) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tarekziade · 2026-11-09T12:14:48Z

run-slow: grounding_dino, mm_grounding_dino

zucchini-nlp · 2026-11-09T12:48:59Z

huh weird, could be the torch version or some new regression intorduced in the meanwhile 😆

I'll check and update the expectations, hoping it is just torch + numerical stuff

zucchini-nlp · 2026-11-09T14:27:25Z

run-slow: grounding_dino, mm_grounding_dino

zucchini-nlp · 2026-11-10T01:04:40Z

need to ssh to runners for that ig :(

github-actions · 2026-11-10T01:49:37Z

[For maintainers] Suggested jobs to run (before merge)

run-slow: grounding_dino, mm_grounding_dino

zucchini-nlp · 2026-11-10T01:50:17Z

Merging, i updated the test overall to not check just logits as it is super flaky. IMO we could check the outputs after post-process and sanity check for equivalence between vision/text hidden states

zucchini-nlp · 2026-11-10T02:40:13Z

ah i need an approve 😅
@guarin ?

tarekziade · 2026-11-10T09:15:00Z

run-slow: grounding_dino, mm_grounding_dino

github-actions · 2026-11-10T09:36:13Z

CI (Nvidia + AMD)

Nvidia · ✅ No new failures 🎉👏 · ⚙️ Workflow · 📊 Dashboard

AMD · ✅ No new failures 🎉👏 · ⚙️ Workflow · 📊 Dashboard

Failures (4 pre-existing)

Pre-existing failures (still failing) — 4 total:

Same error (4):

- grounding_dino:
tests/models/grounding_dino/test_modeling_grounding_dino.py::GroundingDinoModelIntegrationTests::test_grounding_dino_loss

        - mm_grounding_dino:
            tests/models/mm_grounding_dino/test_modeling_mm_grounding_dino.py::MMGroundingDinoModelTest::test_torch_export_dynamic
            tests/models/mm_grounding_dino/test_modeling_mm_grounding_dino.py::MMGroundingDinoModelTest::test_torch_export_static
            tests/models/mm_grounding_dino/test_modeling_mm_grounding_dino.py::MMGroundingDinoModelIntegrationTests::test_mm_grounding_dino_loss

[serge] Fix 8 integration tests for model grounding_dino failing wi…

28659de

…th `output_mismatch` (tensor values differ (6), oth

sergereview Bot marked this pull request as ready for review October 22, 2026 23:23

github-actions Bot requested a review from zucchini-nlp October 22, 2026 23:23

github-actions Bot mentioned this pull request Oct 23, 2026

[serge] integration failure triage - 2026-10-22 #49885

Closed

zucchini-nlp reviewed Oct 23, 2026

View reviewed changes

Fabius-Naumann mentioned this pull request Nov 7, 2026

Grounding DINO: phrase delimiters get text position id 0 since v5.0 (the remaining divergence of #47674) #50465

Closed

what pablo asked

e86de60

zucchini-nlp added 2 commits November 9, 2026 03:16

fix repo

89dfb3a

and the pos id thing seems valid

81a6f4a

github-actions Bot reviewed Nov 9, 2026

View reviewed changes

Comment thread src/transformers/models/grounding_dino/modeling_grounding_dino.py Resolved

Comment thread src/transformers/models/mm_grounding_dino/modular_mm_grounding_dino.py Resolved

trf

4f302e8

guarin reviewed Nov 9, 2026

View reviewed changes

no more flaky asserts and update loss

3581bea

add expectations from runner here

7f097da

zucchini-nlp enabled auto-merge November 10, 2026 01:48

zucchini-nlp disabled auto-merge November 10, 2026 01:48

zucchini-nlp enabled auto-merge November 10, 2026 01:50

tarekziade approved these changes Nov 10, 2026

View reviewed changes

zucchini-nlp added this pull request to the merge queue Nov 10, 2026

Merged via the queue into main with commit 9e7f5cd Nov 10, 2026
49 checks passed

zucchini-nlp deleted the serge/fix/itf-307efcffda70-5639f4f0 branch November 10, 2026 09:40

Conversation

sergereview Bot commented Oct 22, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Original CI failure

✅ Verified on GPU

Possibly related

Uh oh!

HuggingFaceDocBuilderDev commented Oct 22, 2026

Uh oh!

zucchini-nlp left a comment

Choose a reason for hiding this comment

Uh oh!

zucchini-nlp commented Nov 9, 2026

Uh oh!

zucchini-nlp commented Nov 9, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Model linter — first pass

Uh oh!

Uh oh!

Uh oh!

zucchini-nlp commented Nov 9, 2026

Uh oh!

guarin left a comment

Choose a reason for hiding this comment

Uh oh!

guarin Nov 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guarin left a comment

Choose a reason for hiding this comment

Uh oh!

tarekziade commented Nov 9, 2026

Uh oh!

zucchini-nlp commented Nov 9, 2026

Uh oh!

zucchini-nlp commented Nov 9, 2026

Uh oh!

zucchini-nlp commented Nov 10, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

Uh oh!

zucchini-nlp commented Nov 10, 2026

Uh oh!

zucchini-nlp commented Nov 10, 2026

Uh oh!

tarekziade commented Nov 10, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

CI (Nvidia + AMD)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sergereview Bot commented Oct 22, 2026 •

edited by github-actions Bot

Loading