search

ViCCo-Group / thingsvision

Python package for extracting representations from state-of-the-art computer vision models
https://vicco-group.github.io/thingsvision/
MIT License
157 stars 21 forks source link

fix scaling of RGBs for tensorflow #180

Open candemircan opened 1 week ago

candemircan commented 1 week ago

Hi,

I've noticed that when using tensorflow models, the RGB values don't get scaled between 0 and 1. I believe ToTensor in torchvision.transforms handles this, but there is no such operation in the tf transforms that are defined. I created a minimal example of this issue here where extracting features directly from the Harmonized models vs. using thingsvision yields different results.

For ImageDataset, I've added the scaling for the tf backend to fix this. Let me know if there is something I'm missing.

Best wishes, Can

candemircan commented 5 days ago

just an fyi, i was digging deeper into tf docs and it turns out that some pretrained models from tf.keras.applications expect inputs to be rescaled between -1 and 1 (e.g. MobileNet) whereas others don't (e.g. VGG16). They have different preprocessing functions that handle this (tf.keras.applications.mobilenet.preprocess_input). I don't know whether thingsvision uses these preprocessing functions. You should probably test that the rescaling between -1 and 1 does not mess up other models, whose preprocessing functions don't do this operation.

When it comes to custom models, I don't know whether there are any other tf models that expect non-scaled RGB values. I don't normally use tf, so maybe someone who does can weigh in on this. If different custom models are likely to handle this differently, it might be a good idea to raise a warning when defining extractors from custom models, stating what the default preprocessing does.

Let me know if you'd like me to help out with any of this, but again since I don't use tf, it might be more sensible coming from someone more experienced.

cheers, Can

martinhebart commented 5 days ago

Hi @candemircan If you were willing to look into this, we would be extremely grateful, since @hahahannes who has implemented the TF pipeline no longer works with us and you seem to have a better grasp at this functionality (even though you don't use it yourself). Would that be possible? I could see if we can get someone more experienced with TF to review the changes you propose.

LukasMut commented 5 days ago

Agree with @martinhebart: If you could go ahead and dig deeper into this, that would be great @candemircan! šŸ™šŸ¼

@Alxmrphi @jonasd4 @andropar could one of you three potentially help @candemircan, or at least review further changes that he is making? (Unfortunately, I am currently on vacation without access to a laptop and this seems urgent. Also, I don't have much experience with tf/keras either.)

In addition, it would be very helpful if @fel-thomas can make sure that this time everything is correctly implemented - since he knows best what's going on with those models. Would you, Thomas, be able to review this PR and help us make sure the models work the way they are supposed to?

Thanks everyone! Ping me as soon as the PR is finalized and ready to be merged into main. I will then update thingsvision accordingly.

Alxmrphi commented 5 days ago

I'm tied up with giving presentations over the next two days but after that I have some time free and would be glad to look into this a bit more. Enjoy your vacation, Lukas!

LukasMut commented 5 days ago

I'm tied up with giving presentations over the next two days but after that I have some time free and would be glad to look into this a bit more. Enjoy your vacation, Lukas!

Thank you, Alex! šŸ™šŸ¼ No rush. Let's see whether @jonasd4 @andropar @fel-thomas have the bandwidth to help out and review. I suspect this to be a very small PR anyway because it seems to require changes only to the preprocessing function(s).

candemircan commented 5 days ago

hi @martinhebart, sure I'd be happy to give it a go, especially if there are people who can review it.

just a heads up, I can only attend this starting next week. Hope that's all good :)

martinhebart commented 5 days ago

Yes, we will definitely find someone to review it! Iā€™m really grateful for your support here! Thanks!