[CI] remove redundant checks in test_eager_matches_sdpa_inference
#36740
Conversation
|
Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. When it is ready for review, please click the |
| for enable_kernels in (True, False) | ||
| ] | ||
| # Extra test case: `output_attentions=True` has special attention mask handling and sdpa reverts to eager | ||
| ] + [("fp32_pad_left_output_attentions", "fp32", "left", True, True, False)] |
There was a problem hiding this comment.
padding-side left is the hardest test case: the causal mask will have 0 on the left (padding) and on the right (causal mask)
|
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. |
ydshieh
left a comment
ydshieh
left a comment
There was a problem hiding this comment.
Very nice finding.
(BTW, would you like to add a reference to where shows this fact
output_attentions=Truehas special attention mask
Might be helpful for further reader. (I have no idea, but you got my full trust. I am thinking for other people)
|
Added a code link in the PR header for the special attention mask manipulation with SDPA + |
3 participants
What does this PR do?
Follow-up to #36650
Removes redundant test cases in
test_eager_matches_sdpa_inference, related tooutput_attentions.SDPA has special attention mask preparation (e.g. see here) but, when we pass
output_attentions, SDPA reverts to eager attention. That means that mask preparation for SDPA +output_attentionsmust be the same as eager. In #30652 we added aforloop to test the cases withoutput_attentions in (True, False), resulting in 24 new subtests.That's testing overkill, all we need to check is to confirm that we're preparing the attention mask correctly with
output_attentions=True. This PR removes all but 1 of the redundant tests.@ydshieh with this PR, both CPU and GPU CI is faster than in
v4.49.0🤗