Open NielsRogge opened 2 years ago
Sounds like a great idea!
What do you think would be best? I could make a patch for a couple of the most popular implementations like I did for timm and swag. Or ToMe could be a separate model in the library.
Since there is code duplication, making a patch has the possibility of support more models, since I'm assuming a lot of the implementations share the same stem.
What do you think would be best? I could make a patch for a couple of the most popular implementations like I did for timm and swag. Or ToMe could be a separate model in the library.
I'd say both! Regarding a patch of existing implementations, those could be handled in this repo (if you want so, of course) and then we could add ToMe as a separate model (with weights) as well in the library.
@NielsRogge see #21 , it looks like not implementing attention from scratch but using the torch
built-in attention results in better performances. So, assuming I am correct, using torch.nn.MultiHeadAttention
should have been the right choice in transformers
even if it hurts "readability". Thus, it would make more sense to integrate that directly and/or find a way to make ToMe work with it.
Not valid for training since the fast path of nn.MultiHeatAttention
is used only during inference.
@dbolya I am currently looking at ways to get bost of both worlds (Hannah Montana throwbacks :smiley: )
Eager to know your thoughts and happy to help to make these models faster for the community
Hi,
Thanks for this great work!
In 🤗 Transformers, we support the Vision Transformer (ViT) - among many other models like MAE, BEiT, ConvNeXt, Swin Transformer, Swin Transformer v2, etc. Recent additions also include Transformer-based video models, like VideoMAE and X-CLIP.
As can be seen on the hub, the 2 most popular ViT models have +500k and +300k downloads respectively the last month. Would be great if people can leverage this speed up in performance! An increase in throughtput would be very beneficial for people putting these algorithms in production.
As models in the Transformers library are implemented very independently (we duplicate code rather than inheriting, for the sake of readability + independence among the models), we could add ToMe as a separate model in the library.
Let me know your thoughts :)
Best,
Niels ML Engineer @ HuggingFace