CI: avoid human error, automatically infer generative models

[gante]

What does this PR do?

This PR:

1. Replaces manual definition of generative models to test with generate with an automatic definition -- if model_class.can_generate(), then it runs tests from GenerationTesterMixin. No more human mistakes, which happened frequently in the past months 🐛 🔫
2. Now that we run generation tests for all models that can generate, there are a few (old) bad apples. Explicitly skip them i.e. overwrite the automated all_generative_model_classes and explain why certain classes are being skipped. (bad apples detected with py.test tests/models/ -k test_greedy_generate -vv) 💔

---

In a follow-up PR:

1. We need to manually define the model's main input name (e.g., pixel_values) in the model tester. Make it use model.main_input_name instead, to avoid human error Done ✅
2. Despite the changes in this PR, generate tests will only run if GenerationTesterMixin is inherited. We can easily forget to add the mixin, resulting in a false positive. Add an automated check: if any of the model classes can generate, then GenerationTesterMixin must be inherited in the tester

[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.

[zucchini-nlp]

Super cool! If VLMs start failing on generation tests, that's okay. I have a local draft for that, which needs to be reviewed after a few other PRs are merged

[gante]

Many side-fixes later, ready for review 🙌

[ydshieh]

why we don't need def all_generative_model_classes for TF ..?

[gante]

incorrectly moved

(I tried to apply the same pattern on TF, but it has too many broken models. Then I reverted the changes. This one was incorrectly reverted, I'm going to chase down the TF diff to 0)

[ydshieh]

left 3 nit question but yeah thank you!

I see a few files have style changes: I always think they should be fixed in a separate PR, but you can keep them here if you wish.

[gante]

@ydshieh all comments addressed 🤗

[ydshieh]

Thank you again.

Just to make sure the change in tests/test_modeling_tf_common.py is what you expect:

• removing all_generative_model_classes = () OK
• not adding def all_generative_model_classes(self): : if this is intended, OK, for you to check

[ydshieh]

Thank you again.

Just to make sure the change in tests/test_modeling_tf_common.py is what you expect:

• removing all_generative_model_classes = () OK
• not adding def all_generative_model_classes(self): : if this is intended, OK, for you to check

ah , I see you put it back :-), all good then

[zucchini-nlp]

Thanks, this hopefully helps us prevent cases when generation tests are not added.

I am not sure I got why audio models generation is turned off, if it was enabled before and CI was green? Same question for Blip2

[zucchini-nlp]

I think this was working on main for Blip2ForConditionalGeneration, are there many failures?

[gante]

If we remove this line (= if we don't skip the tests), py.test tests/models/blip_2/test_modeling_blip_2.py rebased on main results in 21 failures :P

I've also double-checked the other models with skips on Monday. Most of them have unique model properties that do not work well with generate

[gante]

BTW, note that there are two testers (Blip2ForConditionalGenerationDecoderOnlyTest and Blip2ModelTest), the skips are only on the latter. I don't know why the latter needs to skip, but that's beyond the scope of this PR :P

They were also being skipped before.

[ydshieh]

on main, Blip2ModelTest doesn't have all_generative_model_classes. which means it doesn't run generate tests, and this PR doesn't skip any extra tests for this test class

(I don't know why however)

[zucchini-nlp]

I see, thanks! I believe something similar for audio model since for me the skip comments weren't very clear

[ydshieh]

yeah, that's why I ask frequently more (detailed) comments in many PRs I reviewed 😆

[ydshieh]

@gante you can ping me to merge if the CI persists to be red but irrelevant to your PR :-)

[gante]

@ydshieh yes, please merge 🙏

(was trying to be autonomous :D)