Remove @slow for test_eager_matches_sdpa_inference
#34558
Conversation
|
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
force-pushed
the
fix_more_matching
branch
4 times, most recently
from
ad647f2
to
2afd90e
Compare
ydshieh
force-pushed
the
fix_more_matching
branch
from
fcc2bfd
to
631cdec
Compare
ydshieh
changed the title
test_eager_matches_sdpa_inference less flaky@slow for test_eager_matches_sdpa_inference less flaky
ydshieh
changed the title
@slow for test_eager_matches_sdpa_inference less flaky@slow for test_eager_matches_sdpa_inference
tests/generation/test_utils.py
Outdated
| for key in ["image_token_index", "image_token_id", "video_token_index", "video_token_id", "vision_start_token_id"]: | ||
| token_index = getattr(config, key, None) | ||
| if token_index is not None and token_index < config.get_text_config().vocab_size: | ||
| logits_processor_kwargs["bad_words_ids"].append([token_index]) |
There was a problem hiding this comment.
make it more general.
vision_start_token_id is required for qwen2_vl
| @parameterized.expand([("float16",), ("bfloat16",), ("float32",)]) | ||
| @require_torch_sdpa | ||
| @unittest.skip("Albert requires `head_mask` which is currently not done in this test.") | ||
| def test_eager_matches_sdpa_inference(self): | ||
| pass |
There was a problem hiding this comment.
On skips like this, on Albert and other models: the test pulls the main input and the attention mask to manipulate them, finally sending them to the model. We could pop these items from inputs_dict, and then pass **inputs_dict to the model (e.g. model_eager(**prepared_inputs, **inputs_dict)) -- I think then we wouldn't need to skip tests due to missing inputs 🤗
There was a problem hiding this comment.
Maybe let's merge and do this in a follow-up PR. It's never worked before.
| @@ -452,7 +452,6 @@ def test_flash_attn_2_inference_equivalence_right_padding(self): | |||
|
|
|||
| @parameterized.expand([("float16",), ("bfloat16",), ("float32",)]) | |||
| @require_torch_sdpa | |||
| @slow | |||
There was a problem hiding this comment.
Can't we just delete the test? (it has # Copied from tests.test_modeling_common.ModelTesterMixin.test_eager_matches_sdpa_inference and it inherits the mixin, so it should run the original test!)
There was a problem hiding this comment.
no, it fails. I don't dive into why it's failing (input issues) though.
There was a problem hiding this comment.
Interesting -- in that case, how does # Copied from work? 👀
There was a problem hiding this comment.
# Copied from is only applied to files under src I believe :-) but people sometimes use it in the tests/ 😆
|
Tagging @zucchini-nlp to double-check VLM test changes :) |
| @@ -316,7 +316,6 @@ def prepare_pixel_values(self): | |||
| return floats_tensor([self.batch_size, self.num_channels, self.image_size, self.image_size]) | |||
|
|
|||
| @require_torch_sdpa | |||
| @slow | |||
There was a problem hiding this comment.
i think this test dont need skip anymore since we dont check if model has SDPA layers within this test anymore. But it can be skipped due to the same flakiness
There was a problem hiding this comment.
It still has input preparation issues, where I added below to another model test class
"Idefics requires both text and image inputs which is currently not done in this test."
As mentioned in a reply to Joao's comment, let's try to do it in a follow up PR
tests/models/llava_next_video/test_modeling_llava_next_video.py
Outdated
Show resolved
Hide resolved
| @@ -166,6 +166,8 @@ def prepare_config_and_inputs_for_common(self): | |||
| input_ids = ids_tensor([self.batch_size, self.seq_length], self.vocab_size) | |||
| attention_mask = torch.ones(input_ids.shape, dtype=torch.long, device=torch_device) | |||
|
|
|||
| input_ids[:, -1] = self.pad_token_id | |||
There was a problem hiding this comment.
for my understanding, any reason why last token has to be a pad?
There was a problem hiding this comment.
to avoid index error
(modeling code)
vision_tokens = input_ids[vision_start_indices + 1]
What does this PR do?
And make it less flaky