Idefics: enable generation tests #34062
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zucchini-nlp
requested review from
gante and
ArthurZucker
and removed request for
gante
ArthurZucker
approved these changes
Oct 10, 2024
| @@ -4261,7 +4262,8 @@ def _assisted_decoding( | |||
| added_len, | |||
| is_decoder_attention=True, | |||
| ) | |||
| else: | |||
| # some (V)LLMs have hard requirement on SDPA and thus never return attn | |||
| elif outputs.attentions[0] is not None: | |||
gante
approved these changes
Oct 14, 2024
tests/generation/test_utils.py
Outdated
| @@ -1567,13 +1572,13 @@ def test_generate_from_inputs_embeds_decoder_only(self): | |||
| # This test is for decoder-only models (encoder-decoder models have native input embeddings support in the | |||
| # decoder) | |||
| if config.is_encoder_decoder: | |||
| continue | |||
| self.skipTest("This test is for decoder-only models") | |||
There was a problem hiding this comment.
We need a continue here :P
With a skip, we skip for all other models in self.all_generative_model_classes, and we might have encoder-decoder and decoder-only models mixed up together in that iterator
There was a problem hiding this comment.
hmm oke, didn't think we could have cases with mixed model types. Was just confused the test was passing for one model while not other, and took me a while to figure out hehe
tests/generation/test_utils.py
Outdated
|
|
||
| # Skip models without explicit support | ||
| config.is_decoder = True | ||
| model = model_class(config).to(torch_device).eval() | ||
| if "inputs_embeds" not in inspect.signature(model.prepare_inputs_for_generation).parameters.keys(): | ||
| continue | ||
| self.skipTest("This model doesn't accept `inputs_embeds` in generate") |
There was a problem hiding this comment.
Likewise, this also needs to be a continue
|
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. |
16 tasks
BernardZach
pushed a commit
to BernardZach/transformers
that referenced
this pull request
Dec 5, 2024
* add idefics * conflicts after merging main * enable tests but need to fix some * fix tests * no print * fix/skip some slow tests * continue not skip * rebasing broken smth, this is the fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Enables GenerationTesterMixin for Idefics models. Tests are passing for me locally.
There are a few PRs with idefics already and they need to be reviewed/merged in the following order: #33907 -> #34043 -> current PR