Skip to content

Don't pass masks in model_inputs when not needed - #47951

Closed
SunMarc wants to merge 19 commits into
mainfrom
fix-mask-creation
Closed

SunMarc wants to merge 19 commits into
mainfrom
fix-mask-creation

Conversation

@SunMarc

@SunMarc SunMarc commented Aug 13, 2026 •

Copy link
Copy Markdown
Member

CPU CI GPU run-slow

What does this PR do?

This PR optimizes a bit the generation loop. _ignore_causal_mask_sdpa already decides that a padding mask with no padding can be ignored, but it decides it on every forward: fast_all(padding_mask) reduces the mask on the accelerator and reads the result back on the host, which stops the CPU from running ahead.

Decoding only ever appends ones to the mask, so padding can only come from the prompt and the answer
is the same on every step. Settle it once, and don't hand the model a mask that says nothing:

  generation_config._can_drop_attention_mask = (
        attention_mask is not None
        and not self.config.is_encoder_decoder
        and not is_tracing(attention_mask)
        and not self._valid_auto_compile_criteria(model_kwargs, generation_config)
        and bool(fast_all(attention_mask))
    )

Benchmark

MPS, 256 tokens, unpadded prompt

model before after speedup
SmolLM2-135M-Instruct 77.6 – 87.3 tok/s 99.8 – 100.7 tok/s 1.15× – 1.29×
Qwen3.5-0.8B 41.3 – 44.2 tok/s 46.0 – 49.0 tok/s 1.06× – 1.15×

Why not drop the mask from model_kwargs?

Tried it: 607 CI failures. Its length and presence are load-bearing beyond masking.

  • _prefill compares input_ids.shape[1] == attention_mask.shape[1] to tell whether the inputs are
    the full sequence or only the new tokens — [generate] Completely stop relying on cache_position to prepare inputs #44130 removed cache_position and promoted the mask's
    length into that role.
  • _prepare_position_ids_for_generation switches convention on its presence; without it, positions
    overshoot by past_length. That is silently wrong output, not an exception.
  • Hub custom_generate recipes and the candidate generators index it directly, and live in other repos.

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

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.

@SunMarc
SunMarc requested review from Cyrilvallez and vasqu August 13, 2026 17:32
@SunMarc
SunMarc requested a review from ArthurZucker August 18, 2026 15:46
@SunMarc SunMarc mentioned this pull request Aug 18, 2026
4 tasks done
Comment thread src/transformers/masking_utils.py Resolved
@Cyrilvallez

Copy link
Copy Markdown
Member

Hey! I don't think putting a runtime only attribute on the cache is a very good idea in general. If we want to do it, the way to go is at the very start of generate, in _prepare_attention_mask_for_generation and equivalent -> do NOT create the mask in the inputs if no padding

@SunMarc

SunMarc commented Sep 1, 2026 •

Copy link
Copy Markdown
Member Author

Thanks for the inputs, I'll try to come up with a better solution ! At least i tried to not create the masks at the very beginning and the CI was not happy at all ...

@vasqu

vasqu commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Unsubscribing for a bit, feel free to ping me when it's ready 🫡

@SunMarc
SunMarc marked this pull request as draft September 9, 2026 16:33
@SunMarc

SunMarc commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

switching to draft for now

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this new update looks better! I don't think we need that much comments and etc, let's make this minimal!

@remi-or
remi-or marked this pull request as ready for review September 14, 2026 06:24
@remi-or
remi-or marked this pull request as draft September 14, 2026 06:26
@Cyrilvallez

Copy link
Copy Markdown
Member

Please ping me when ready for review @SunMarc!

@SunMarc
SunMarc marked this pull request as ready for review September 14, 2026 17:13
@SunMarc SunMarc changed the title Ignore causal masks a bit earlier for sdpa Don't pass masks in model_inputs when not needed Sep 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 34871985082:1
Result: success | Jobs: 16 | Tests: 189,706 | Failures: 0 | Duration: 12h 52m

@SunMarc

SunMarc commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

closed in favor of #48814 !

@SunMarc SunMarc closed this Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants