Verify interpolation of image processors

NielsRogge commented 11 months ago

Feature request

As pointed out in https://github.com/huggingface/transformers/pull/27742, some image processors might need a correction on the default interpolation method being used (resampling in Pillow). We could check this on a per-model basis.

Motivation

Interpolation methods have a slight (often minimal) impact on performance. However it would be great to verify this on a per-model basis.

e.g. ViT's image processor defaults to BILINEAR but should use BICUBIC as seen here. We can update the default values of the image processors, but can't update the configs on the hub as this would break people's fine-tuned models.

Your contribution

I could work on this, but this seems like a good first issue for first contributors.

To be checked (by comparing against original implementation):

[ ] beit
[ ] bit
[ ] clip
[ ] convnext
[ ] convnextv2
[ ] cvt
[ ] data2vec-vision
[ ] deit
[ ] dinat
[ ] dinov2
[ ] efficientformer
[ ] efficientnet
[ ] focalnet
[ ] imagegpt
[ ] levit
[ ] mobilenet_v1
[ ] mobilenet_v2
[ ] mobilevit
[ ] mobilevitv2
[ ] nat
[ ] perceiver
[ ] poolformer
[ ] pvt
[ ] regnet
[ ] resnet
[ ] segformer
[ ] siglip
[ ] swiftformer
[ ] swin
[ ] swinv2
[ ] van
[ ] vit
[ ] vit_hybrid
[ ] vit_msn

amyeroberts commented 11 months ago

@NielsRogge Thanks for opening the issue!

It's fine to open up to the community but you'll need to add a checklist of the image processors so it's clear who is working on what and what's done as well as ideally some instructions on what it means for each one to be "done" e.g. making sure to run slow tests for models.

nileshkokane01 commented 10 months ago

@NielsRogge ,

If I understand it correctly, we need to match the interpolation:

For example for convnext: convnext should be changed to Bicubic as per timm/convnext .

If that's correct , I can take this up for all the models. Let me know.

NielsRogge commented 10 months ago

Yes that is correct, see also the original implementation. Thanks for spotting that. Hence feel free to open a PR to update this, along with the image processor created in the conversion script. Ideally we assert the pixel values created by it against the original implementation, like done here for DINOv2.

nileshkokane01 commented 10 months ago

Sure! thanks for the pointers, will work on it.

nileshkokane01 commented 10 months ago

DieT and DPT default interpolation types matches with the original implementation types to BICUBIC . That's what I see it. Let me know if I overlooked.

nileshkokane01 commented 10 months ago

@NielsRogge ,

would you have a look ?

amyeroberts commented 9 months ago

@NielsRogge Can you please complete the checklist here?

hritikranjan1 commented 1 month ago

@NielsRogge I would like to take this issue up.

shubham-61969 commented 1 month ago

@NielsRogge , @nileshkokane01 Can I work on this issue and help in completing the checklist ?

farrosalferro commented 3 weeks ago

@NielsRogge , is it possible for me to contribute ? Thanks.

huggingface / transformers