Skip to content

Conversation

@MKhalusova
Copy link
Contributor

The docs/index.md file currently contains two auto-generated parts: the list of models (same as in README), and a table of models with supported frameworks. Due to the number of models available in transformers (200+), the list and the table have become quite large, and there have been internal discussions about removing the list of models from the index.md.

This PR adds the following changes:

  • removes the autogenerated model list from the index.md and updates the script so it's no longer added
  • modifies the script that generates the table to make model names links to corresponding model_doc.

The model lists in the main README and localized READMEs remain as is.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 2, 2023

The documentation is not available anymore as the PR was closed or merged.

@MKhalusova MKhalusova marked this pull request as ready for review October 2, 2023 17:56
@MKhalusova MKhalusova requested review from ArthurZucker and stevhliu and removed request for stevhliu October 2, 2023 17:57
Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

Super nice, I think it's a lot cleaner like this! ✨

It looks like some models (like Llama 2, Flan T5, DiT) are missing though.

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Very clean thanks a lot 🤗

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is a good time to check model on the table has a corresponding model.md and vice-versa!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe check_copies.py checks consistency between model.md and the list of models in MODEL_NAMES_MAPPING. Here, I use the same MODEL_NAMES_MAPPING, so I'm not sure one more check here is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if you think it's good to merge as is. Sorry about the ping @ArthurZucker :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

Choose a reason for hiding this comment

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

A loooooot better indeed 😉

@MKhalusova
Copy link
Contributor Author

MKhalusova commented Oct 3, 2023

Super nice, I think it's a lot cleaner like this! ✨

It looks like some models (like Llama 2, Flan T5, DiT) are missing though.

The reason they are missing is that the table is based on the model classes. And in case of the models you mentioned, Llama 2 uses the same implementation as Llama, Flan T5 uses the same implementation as T5, and DiT's architecture is equivalent to that of BEiT.
I can check if I can automatically find models like this, if not, I'll add them as "special case" constants.

@ArthurZucker
Copy link
Collaborator

Yep, I think these models should be part of the table and we should make sure we can easily add more models there that share the same modeling file!

@MKhalusova
Copy link
Contributor Author

MKhalusova commented Oct 3, 2023

I've added a dict of models that use the same config as some "base" model. This way the table is complete.
I couldn't find this mapping anywhere else, so it's hardcoded here.
If these models were included in transformers_module.models.auto.configuration_auto.CONFIG_MAPPING_NAMES, it would not need to be hardcoded here, however, that dictionary is used in so many places, I'm hesitant to modify it.

cc @stevhliu

Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

Great job, and thanks for completing the table!

@MKhalusova MKhalusova merged commit 18fbeec into huggingface:main Oct 5, 2023
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.

4 participants