Skip to content

Conversation

remi-or
Copy link
Contributor

@remi-or remi-or commented Jul 3, 2025

This PR changes the order of priority of Expectations to prefer the default expectation over the cross-device match as discussed with @ydshieh . It also changes the decorator of a FA3 test from require_flash_attn to require_flash_attn_3

@remi-or remi-or requested a review from ydshieh July 3, 2025 09:59
@remi-or remi-or changed the title General fixes Expectations re-order and corrected FA3 skip Jul 3, 2025
@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

@ydshieh ydshieh left a comment

Choose a reason for hiding this comment

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

Thanks.

If it is not clear from any where, maybe add a comment about what a default will look like.

I am using (None, None). And a comment like

A default value is usually a value that is used for some time in a test before it uses Expectations. When we modify a test to use Expectations for a specific hardware, we don't want to affect the tests on other hardware. So we add a value for that specific hardware and leave the previously existing value as the default value which will be used by the other hardwares. This is why the default value is preferred over using the values from other hardwares.

might help, if you find so.

@ydshieh
Copy link
Collaborator

ydshieh commented Jul 3, 2025

Feel free to merge 🤗

@remi-or
Copy link
Contributor Author

remi-or commented Jul 3, 2025

Good ideas! Did both. Will merge after tests pass.

@remi-or remi-or enabled auto-merge (squash) July 5, 2025 20:53
@ydshieh ydshieh disabled auto-merge July 7, 2025 09:42
@ydshieh ydshieh merged commit a325409 into huggingface:main Jul 7, 2025
23 of 25 checks passed
rjgleaton pushed a commit to rjgleaton/transformers that referenced this pull request Jul 17, 2025
* Fix Expectations and a FA3 skip

* Fixed docstring

* Added context for Default expectation
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.

3 participants