Skip to content

Conversation

@vasqu
Copy link
Contributor

@vasqu vasqu commented Nov 21, 2025

Given that the torch compiler fuses our eager attention to sdpa automatically under compile, we should take more advantage and allow no masks on bidirectional masks without padding == fa backend. This restriction is also unnecessary and I'm not sure why I had this one to begin with.

Fuse strategies can be seen here:

Comment on lines -343 to -345
allow_torch_fix (`bool`, optional):
Whether to update the mask in case a query is not attending to any tokens, to solve a bug in torch's older
versions. We need an arg to skip it when using eager. By default `True`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doubled comment, unrelated but should be removed nonetheless. Slipped through

@HuggingFaceDocBuilderDev

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.

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

is there a test we can add for this? Otherwise lgtm!

@vasqu
Copy link
Contributor Author

vasqu commented Nov 21, 2025

Good point, let me check if we properly skip the mask in a test here.

@vasqu
Copy link
Contributor Author

vasqu commented Nov 21, 2025

run-slow: bart,bert,llama

@github-actions
Copy link
Contributor

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

models: ["models/bart", "models/bert", "models/llama"]
quantizations: []

@github-actions
Copy link
Contributor

CI Results

Workflow Run ⚙️

✅ No failing test specific to this PR 🎉 !

@vasqu vasqu merged commit bdee088 into main Nov 21, 2025
25 checks passed
@vasqu vasqu deleted the remove-eager-bidirectional-mask-restriction branch November 21, 2025 17:51
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.

4 participants