Skip to content

Add support for SDPA for OWLViT and OWLv2#40023

Open
mihaidusmanu wants to merge 10 commits into
huggingface:mainfrom
mihaidusmanu:user/mihaidusmanu/sdpa-owlvit-owlv2
Open

Add support for SDPA for OWLViT and OWLv2#40023
mihaidusmanu wants to merge 10 commits into
huggingface:mainfrom
mihaidusmanu:user/mihaidusmanu/sdpa-owlvit-owlv2

Conversation

@mihaidusmanu

@mihaidusmanu mihaidusmanu commented Aug 8, 2025

Copy link
Copy Markdown

What does this PR do?

Add support for SDPA (scaled_dot_product_attention) for efficient attention to OWLViT and OWLv2 models.
The previous code is used in the eager attention implementation. I roughly followed the SigLIP code for inspiration.
Note that we could do a larger refactory to use the is_causal flag, but I tried to stick as close as possible to the original implementation in this first version.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
  • Did you write any new necessary tests? > I added an e2e sdpa inference test based on the fp16 one, but let me know if anything else is needed.

Who can review?

Maybe @amyeroberts @qubvel @ArthurZucker

outputs_eager = outputs_eager["language_model_outputs"]
outputs_sdpa = outputs_sdpa["language_model_outputs"]
key = "hidden_states" if "hidden_states" in outputs_eager else "decoder_hidden_states"
elif "text_model_output" in outputs_eager and "owl" in model_class.__name__.lower():

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not very happy about this one...

@qubvel qubvel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this @mihaidusmanu, great work!

Comment on lines +369 to +392
def eager_attention_forward(
module: nn.Module,
query: torch.Tensor,
key: torch.Tensor,
value: torch.Tensor,
attention_mask: Optional[torch.Tensor],
scaling: float,
dropout: float = 0.0,
**kwargs,
):
bsz, num_heads, seq_len, head_dim = query.shape
proj_shape = (bsz * num_heads, -1, head_dim)
query = query.reshape(proj_shape)
key = key.reshape(proj_shape)
value = value.reshape(proj_shape)

attn_weights = torch.bmm(query, key.transpose(1, 2)) * scaling

if attn_weights.size() != (bsz * num_heads, seq_len, seq_len):
raise ValueError(
f"Attention weights should be of size {(bsz * num_heads, seq_len, seq_len)}, but is {attn_weights.size()}"
)

if attention_mask is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we reuse one of the existing eager_attention_forward functions defined in transformers?

@mihaidusmanu mihaidusmanu Aug 8, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Going through the code, I think the BLIP2 eager_attention_forward (so SigLIP without fp32 cast on attention weights) should achieve the same thing so I replaced it by that one.

The only thing missing from the BLIP2 version seems to be the following logic.

        # For int8 compatibility, sometimes the `attn_probs` are in `fp32`
        attn_probs = attn_probs.to(value_states.dtype)

@github-actions

github-actions Bot commented Aug 8, 2025

Copy link
Copy Markdown
Contributor

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

run-slow: owlv2, owlvit

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.

2 participants