Fix tests due to dropping attn mask by SunMarc · Pull Request #48903 · huggingface/transformers

SunMarc · 2026-09-17T14:55:07Z

What does this PR do?

This PR fixes the following tests that were broken due to removing the attn mask early from this PR #48814

ImageGPT — tests/models/imagegpt/test_modeling_imagegpt.py

ImageGPTModelTest::test_onnx_export_generate_dynamic
ImageGPTModelTest::test_torch_export_generate_dynamic

RuntimeError: The size of tensor a (s70 + s85) must match the size of tensor b (s70) at non-singleton dimension 3)"

Its create_bidirectional_mask() call didn’t receive past_key_values. Previously, the 2D attention mask supplied the full length: cached tokens plus new tokens. Once that mask became None, the helper inferred the length from only the new embeddings. Attention scores still included cached tokens, producing the reported s70 + s85 versus s70 mismatch.

Moshi — tests/models/moshi/test_modeling_moshi.py

MoshiDecoderTest::test_cached_decode_matches_cacheless (flaky tests btw)

For Moshi, the issue is that we were using the eager attn implementation that requires a mask.

Previously, the 2D mask reached Moshi but now, there is no mask. The controlled comparison was 100/100 passing seeds before the PR, 97/100 after, and 100/100 with the fix. The apparent flakiness comes from which random inputs expose the incorrect behavior.

Three ways to solves this: always create the mask or we don't drop the attn mask when we use eager. Last solution would be to fix eager to deal with the case where the mask is None but I don't think we should assume that the mask is all ones ?

Qwen3-Omni — tests/models/qwen3_omni_moe/test_modeling_qwen3_omni_moe.py

Qwen3OmniModelIntegrationTest::test_small_model_integration_test_batch_audio_matches_single
Qwen3OmniModelIntegrationTest::test_small_model_integration_test_w_audio

We need to refactor to use the new api with _prepare_position_ids_for_generation that is in generate where the mask is still not dropped. the changes are similar to this PR #47872. Some arch might also need this fix ? @zucchini-nlp

SunMarc · 2026-09-17T14:56:09Z

        if (
            not self.config.is_encoder_decoder
            and accepts_attention_mask
+            and self.config.get_text_config()._attn_implementation != "eager"


should do that @Cyrilvallez ?

hmm I know flex attn was never actually used for inference but was it considered at some point? 😢

SunMarc · 2026-09-17T14:57:00Z

-                attention_mask=attention_mask,
-                past_key_values=past_key_values,
-                position_ids=position_ids,
-            )


otherwise the fix is this one but i think the issue with eager might happen with other models as well

should be fine, no reason to do this prev pattern

okay then we can probably drop the eager check no ?

The eager check was only needed for this?

Imo, we should never do

if mask is none: create_mask

that portion should properly handle the skips and creation and is an anti pattern to the create fn

ydshieh · 2026-09-17T15:03:48Z

remember to run-slow and check results on the job run page 🙏

vasqu

Imo fine to fix this way, I'd like Raushan's opinion on the qwen part because im wondering where this came from 👀

Other than that, for the future: Was flex attn considered?

vasqu · 2026-09-17T15:02:14Z

        if (
            not self.config.is_encoder_decoder
            and accepts_attention_mask
+            and self.config.get_text_config()._attn_implementation != "eager"


hmm I know flex attn was never actually used for inference but was it considered at some point? 😢

vasqu · 2026-09-17T15:02:31Z

-                attention_mask=attention_mask,
-                past_key_values=past_key_values,
-                position_ids=position_ids,
-            )


should be fine, no reason to do this prev pattern

vasqu · 2026-09-17T15:03:26Z

+
+        return position_ids
+
+    def _expand_inputs_for_generation(self, expand_size=1, is_encoder_decoder=False, input_ids=None, **model_kwargs):


ig this comes from other qwen vls? at least the pattern

would like Raushan's opinion on this

HuggingFaceDocBuilderDev · 2026-09-17T15:06:29Z

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

thanks, i have only one concern on text-only generation

zucchini-nlp · 2026-09-17T15:07:39Z

+    def _prepare_position_ids_for_generation(self, inputs_tensor, model_kwargs):
+        # Overwritten -- requires 3D position ids
+


oh i love qwen!

zucchini-nlp · 2026-09-17T15:09:57Z

+        if is_input_ids and attention_mask is not None:
+            vision_positions, rope_deltas = self.get_rope_index(
+                inputs_tensor,
+                image_grid_thw=model_kwargs.get("image_grid_thw"),
+                video_grid_thw=model_kwargs.get("video_grid_thw"),
+                attention_mask=attention_mask,
+                use_audio_in_video=model_kwargs.get("use_audio_in_video") or False,
+                audio_seqlens=model_kwargs.get("audio_feature_lengths"),
+                second_per_grids=model_kwargs.get("video_second_per_grid"),
+            )


and if there are multimodal inputs, i think it will throw an error otherwise trying to pop inputs from empty grid_thw

indeed, fixed !

zucchini-nlp · 2026-09-17T15:10:57Z

+    def _expand_inputs_for_generation(self, expand_size=1, is_encoder_decoder=False, input_ids=None, **model_kwargs):
+        position_ids = model_kwargs.pop("position_ids", None)
+        input_ids, model_kwargs = super()._expand_inputs_for_generation(
+            expand_size=expand_size,
+            is_encoder_decoder=is_encoder_decoder,
+            input_ids=input_ids,
+            **model_kwargs,
+        )
+        if position_ids is not None:
+            batch_dim = 1 if position_ids.ndim == 3 else 0
+            model_kwargs["position_ids"] = position_ids.repeat_interleave(expand_size, dim=batch_dim)
+        return input_ids, model_kwargs


this defi doesn't work well for images/videos 😆

old hidden bug, but let it be - this model is almost never used atm, and we'll remove all overriden expansion soon 🤞🏻

zucchini-nlp · 2026-09-17T15:12:09Z

+        if past_length == 0 or self.rope_deltas is None:
+            if attention_mask is None:
+                attention_mask = torch.ones((batch_size, seq_length), dtype=torch.long, device=inputs_tensor.device)
+            position_ids, rope_deltas = self.get_rope_index(


same here, might need a check has_multimodal_data

SunMarc · 2026-09-17T15:38:18Z

Other than that, for the future: Was flex attn considered?
cc @Cyrilvallez for this !

SunMarc · 2026-09-17T15:46:13Z

run-slow: imagegpt, kyutai_speech_to_text, moshi, qwen3_omni_moe

github-actions · 2026-09-17T15:47:28Z

AMD CI

Workflow Run ⚙️

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

models: ["models/imagegpt", "models/kyutai_speech_to_text", "models/moshi", "models/qwen3_omni_moe"]

github-actions · 2026-09-17T15:47:29Z

Nvidia CI

Workflow Run ⚙️

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

models: ["models/imagegpt", "models/kyutai_speech_to_text", "models/moshi", "models/qwen3_omni_moe"]
quantizations: []

github-actions · 2026-09-17T16:16:11Z

CI Results (AMD)

Workflow Run ⚙️

Commit Info

Context	Commit	Description
RUN	8f0e49a5	workflow commit (merge commit)
PR	868c6d5c	branch commit (from PR)
main	da58b892	base commit (on `main`)

Model CI Report

❌ 2 new failed tests from this PR 😭

qwen3_omni_moe:
tests/models/qwen3_omni_moe/test_modeling_qwen3_omni_moe.py::Qwen3OmniModelIntegrationTest::test_small_model_integration_test_batch_flashatt2 (❌ ⟹ ❌)
tests/models/qwen3_omni_moe/test_modeling_qwen3_omni_moe.py::Qwen3OmniModelIntegrationTest::test_small_model_integration_test_batch_audio_matches_single (❌ ⟹ ❌)

zucchini-nlp

Ig slow ci is failing because of audio, otherwise lgm

zucchini-nlp · 2026-09-17T16:28:08Z

+    ) -> torch.Tensor:
+        batch_size, seq_length = inputs_tensor.shape[:2]
+        past_length = 0 if past_key_values is None else past_key_values.get_seq_length()
+        has_multimodal_data = any(grid is not None and grid.numel() > 0 for grid in (image_grid_thw, video_grid_thw))


or audio_seqlens is not None :)

github-actions · 2026-09-17T16:29:46Z

CI Results (Nvidia)

Workflow Run ⚙️

Commit Info

Context	Commit	Description
RUN	8f0e49a5	workflow commit (merge commit)
PR	868c6d5c	branch commit (from PR)
main	da58b892	base commit (on `main`)

✅ No failing test specific to this PR 🎉 👏 !

vasqu

let's remove the eager attn check as well no? like mentioned the is mask is none pattern is an anti pattern the create mask fn should handle it imo

SunMarc · 2026-09-17T17:12:25Z

Okay I will fix this !

Cyrilvallez

Would like to understand why we don't drop if eager please before merge?

Cyrilvallez · 2026-09-18T08:05:44Z

If it's for a specific model that explicitly uses the 2D mask all the time, IMO needs to be fixed on the model side directly! (i.e. something along mask = torch.ones() if mask is None else mask)

molbap · 2026-09-18T10:06:32Z

Yes, will fail on mllama and idefics currently, in cases where the random test sets the mask to all ones (happens) will be dropped, so the query and image mask will have different lenghts and it will crash

molbap · 2026-09-18T10:20:05Z

for idefics a block like this in the crosss attention mask computation

            padding_length = seq_length - image_attention_mask.shape[1]
            if padding_length > 0:
                image_attention_mask = torch.cat(
                    [
                        image_attention_mask,
                        image_attention_mask[:, -1:].expand(-1, padding_length, -1),
                    ],
                    dim=1,
                )

should work, there

transformers/src/transformers/models/idefics/modeling_idefics.py

Line 1240 in 1855613

seq_length = model_inputs["input_ids"].shape[1]

(after seq length computation)
not beautiful but should work

github-actions · 2026-09-18T15:19:01Z

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

run-slow: imagegpt, kyutai_speech_to_text, moshi, qwen3_omni_moe

SunMarc · 2026-09-18T15:24:03Z

Yes, will fail on mllama and idefics currently, in cases where the random test sets the mask to all ones (happens) will be dropped, so the query and image mask will have different lenghts and it will crash

I didn't manage to reproduce the failing test even with all ones masks (maybe bcause i'm running this on cpu/mps ?) We can have a look at this in a follow-up PR ! I'll first merge to this PR, so that the other models are fixed.

github-actions · 2026-09-18T15:28:29Z

CI recap

Dashboard: View test results in Grafana
Latest run: 35352874234:1
Result: success | Jobs: 16 | Tests: 190,602 | Failures: 24 | Duration: 13h 16m

SunMarc added 2 commits September 17, 2026 14:51

fix tests

6d3618c

fix !

2f91c6a

SunMarc commented Sep 17, 2026

View reviewed changes

SunMarc requested review from Cyrilvallez, vasqu and zucchini-nlp September 17, 2026 14:57

vasqu reviewed Sep 17, 2026

View reviewed changes

zucchini-nlp reviewed Sep 17, 2026

View reviewed changes

updated !

e7aba70

up

868c6d5

SunMarc requested a review from zucchini-nlp September 17, 2026 15:58

zucchini-nlp approved these changes Sep 17, 2026

View reviewed changes

audioo

0f77400

vasqu approved these changes Sep 17, 2026

View reviewed changes

Cyrilvallez reviewed Sep 18, 2026

View reviewed changes

Comment thread src/transformers/generation/utils.py Resolved

SunMarc and others added 2 commits September 18, 2026 15:53

remove unneeded fix

f042b2e

Merge branch 'main' into fix-no-attn-mask-test

233d055

SunMarc enabled auto-merge September 18, 2026 15:20

SunMarc added this pull request to the merge queue Sep 18, 2026

Merged via the queue into main with commit b856a67 Sep 18, 2026
114 checks passed

SunMarc deleted the fix-no-attn-mask-test branch September 18, 2026 15:38

This was referenced Sep 19, 2026

[serge] Fix 2 integration tests for model llava_next_video failing with other (other (2)) #48955

Closed

[serge] Fix 2 integration tests regressed by commit bcb9849a4e66 (PR #48714) #48957

Closed


		return position_ids

		def _expand_inputs_for_generation(self, expand_size=1, is_encoder_decoder=False, input_ids=None, **model_kwargs):

		def _prepare_position_ids_for_generation(self, inputs_tensor, model_kwargs):
		# Overwritten -- requires 3D position ids

Conversation

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

Uh oh!

What does this PR do?

ImageGPT — tests/models/imagegpt/test_modeling_imagegpt.py

Moshi — tests/models/moshi/test_modeling_moshi.py

Qwen3-Omni — tests/models/qwen3_omni_moe/test_modeling_qwen3_omni_moe.py

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!

Choose a reason for hiding this comment

Uh oh!

SunMarc Sep 17, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ydshieh commented Sep 17, 2026

Uh oh!

vasqu left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HuggingFaceDocBuilderDev commented Sep 17, 2026

Uh oh!

zucchini-nlp left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SunMarc commented Sep 17, 2026

Uh oh!

SunMarc commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 17, 2026

AMD CI

Uh oh!

github-actions Bot commented Sep 17, 2026

Nvidia CI

Uh oh!

github-actions Bot commented Sep 17, 2026

CI Results (AMD)

Commit Info

Model CI Report

Uh oh!

zucchini-nlp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 17, 2026

CI Results (Nvidia)

Commit Info

Uh oh!

SunMarc commented Sep 17, 2026 •

edited by github-actions Bot

Loading

SunMarc Sep 17, 2026 •

edited

Loading

vasqu left a comment •

edited

Loading

Cyrilvallez left a comment •

edited

Loading

SunMarc commented Sep 18, 2026 •

edited

Loading