keras-team / keras-cv

Industry-strength Computer Vision workflows with Keras
Other
1k stars 332 forks source link

Finalize APIs for RandomCropAndResize and RandomlyZoomedCrop #826

Open LukeWood opened 1 year ago

martin-gorner commented 1 year ago

Can we have consistent names ? Please choose between Random and Randomly for naming random layers. By the way, it looks like the choice has already been made: RandomCrop RandomFlip RandomTranslation RandomRotation RandomZoom RandomHeight RandomWidth RandomContrast RandomBrightness

martin-gorner commented 1 year ago

For RandomResizedCrop, if the only goal is to offer the same functionality as the existing (and really bad) RandomResizedCrop from TorchVision, then we should keep the same name RandomResizedCrop. In Albumentations, they also have it and they kept the name: https://albumentations.ai/docs/api_reference/augmentations/crops/transforms/#albumentations.augmentations.crops.transforms.RandomResizedCrop "Torchvision's variant of crop a random part of the input and rescale it to some size"

LukeWood commented 1 year ago

Can we have consistent names ? Please choose between Random and Randomly for naming random layers. By the way, it looks like the choice has already been made: RandomCrop RandomFlip RandomTranslation RandomRotation RandomZoom RandomHeight RandomWidth RandomContrast RandomBrightness

Thanks Martin good point - what do you recommend we name the zoom variant? RandomCropAndZoom? RandomZoomAndCrop?

martin-gorner commented 1 year ago

RandomZoomCrop ?

On Fri, 23 Sept 2022 at 19:10, Luke Wood @.***> wrote:

Can we have consistent names ? Please choose between Random and Randomly for naming random layers. By the way, it looks like the choice has already been made: RandomCrop https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_crop RandomFlip https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_flip RandomTranslation https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_translation RandomRotation https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_rotation RandomZoom https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_zoom RandomHeight https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_height RandomWidth https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_width RandomContrast https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_contrast RandomBrightness https://keras.io/api/layers/preprocessing_layers/image_augmentation/random_brightness

Thanks Martin good point - what do you recommend we name the zoom variant? RandomCropAndZoom? RandomZoomAndCrop?

— Reply to this email directly, view it on GitHub https://github.com/keras-team/keras-cv/issues/826#issuecomment-1256456082, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHKKZ7G666W2MA4K3VKO2TV7XQATANCNFSM6AAAAAAQTSFOH4 . You are receiving this because you were assigned.Message ID: @.***>

--

Martin Görner | Product Manager, TensorFlow & Keras | @.*** | +1 425 273 0605

martin-gorner commented 1 year ago

Thinking a bit more about mirroring:

We already have: RandomCrop: does not mirror. Disrespects the zoom factor if target size is larger than the image. RandomZoom: does mirror, or fill with zeros etc is the target size is larger than the original image.

Test Colab here

RandomZoomCrop, when all effects are dialed down, should gradually become RandomCrop (RandomZoom does not crop to a target size so cannot be an "at the limit" bahavior of RandomZoomCrop.

Therefore, to be consistent between RandomCrop and RandomZoomCrop with no zooming, RandomZoomCrop should not mirror.

It is however a valid point that some people might want the mirroring. In that case, they can easily get it by combining:

RandomZoom(height_factor=(0.0, 1.0), fill_mode="reflect") # zoom out between +0% and +100%
RandomZoomCrop(width=128, height=512, zoom_factor=(...))

Thoughts ?

PS: