fix SeCo transforms
The current SeCo transforms do the following:
- resize
- center crop
- normalize using the quantiles
- multiply by 255
- normalize using imagenet stats
However in the script they actually do the following:
- resize
- center crop
- normalize using the quantiles
- multiply by 255
- clip to [0. 255]
- convert to uint8
- divide by 255
- normalize using imagenet stats
Whether you agree or not with the changes, our current seco transforms are incorrect. I'm just taking what they have from their script.
I don't actually know how to fix the mypy errors other than ignoring them.
- I can't type hint a lambda but kornia doesn't have a clip augmentation.
-
K.AugmentationSequentialcomplains about theK.contrib.Lambdainput arg
Whether you agree or not with the changes, our current seco transforms are incorrect. I'm just taking what they have from their script.
In my mind, the transforms supplied with the weights aren't designed to reproduce SeCo pre-training, they're designed to use the SeCo weights for downstream tasks. If these changes don't improve performance on downstream tasks and only make transforms slower, or even harm performance on downstream tasks, then we shouldn't add them. That's why an ablation study is needed to ensure that these changes are beneficial.
In that case then the transforms are completely wrong, because SeCo doesn't use these for downstream tasks at all. They only divide eurosat by 255, they use different BigEarthNet mean and std. They also don't resize to 224x224. I would argue that you would need to show an ablation study of why you shouldn't use the same transforms as those used during training since that's less intuitive. It's trained to take 8-bit inputs so I would expect performance would degrade if you feed it 12-bit inputs.
Let's hold off on this PR until after the deadline so we have time to do a proper ablation study on no transforms, current transforms, and proposed transforms.
I'm good with this just wanted to use it for comparisons to SeCo
Added this to the 0.4.2 milestone so we don't forget about it. At the bare minimum, we should remove the multiply by 255 stuff. The rest is more controversial and will need ablation studies that we don't have time to work on until after the deadline.
Is this still a work in progress?
@adamjstewart I can finish this today. It fell off my radar.
No rush. If we can squeeze it into 0.5.2, great. If not, we can continue to punt it. Just trying to finish up old PRs.