Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduces a Random resized crop layer #203

Closed

Conversation

frostbyte012
Copy link

Feature Implementation : Random Resized Crop Layer (Standard Augmentation Transformation).
References issue: #131

keras_cv/layers/preprocessing/random_resized_crop.py Outdated Show resolved Hide resolved
keras_cv/layers/preprocessing/random_resized_crop.py Outdated Show resolved Hide resolved

# method for resizing the images
def _resize(self, inputs):
outputs = image_utils.smart_resize(inputs, [self.height, self.width])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will perform the same resize on all images

Copy link
Author

@frostbyte012 frostbyte012 Mar 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done : called on import random
and used it to randomly resize height and width of these images, for further implementation of random crop.

@LukeWood
Copy link
Contributor

Thanks for the PR! Ideally the layer would:

  • randomly resize each image individually
  • randomly crop each image individually from those resizes

The resulting images would have the same size, but different scalings

@frostbyte012
Copy link
Author

Welcome!
I figured out the problems.
Trying to fix it and resolve the changes as soon as possible.

W_AXIS = -2


class RandomResizedCrop(base_layer.Layer):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets subclass tf.keras.internal.layers.BaseImageAugmentationLayer instead of base_layer.Layer.

Check out
https://github.com/keras-team/keras-cv/blob/master/keras_cv/layers/preprocessing/random_shear.py

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done : Added a subclass tf.keras.internal.layers.BaseImageAugmentation.Layer in place of base_layer.Layer

@@ -0,0 +1,91 @@
from keras import backend
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets also add a script to examples showing the outputs of this layer. Lets base it on the color degeneration example:

https://github.com/keras-team/keras-cv/blob/master/examples/layers/preprocessing/random_color_degeneration_demo.py

@LukeWood
Copy link
Contributor

Thanks for the updates!

@LukeWood LukeWood changed the title Random resized crop branch Introduces a Random resized crop layer Mar 28, 2022

# method for resizing the images
def _resize(self, inputs):
self.new_height=random.randint(0,self.height)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you'll actually need to use a tf.random and use the get_random_transformation API in the BaseImageAugmentation layer. This random value gets baked into the computation graph

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done : Resolved it and modified it using the tf.random and get_random_transformation API from BaseImageAugmentation layer

@frostbyte012
Copy link
Author

I reviewed the changes that are suggested and I'm starting to work on them as soon as possible.

@LukeWood
Copy link
Contributor

LukeWood commented Apr 1, 2022

I reviewed the changes that are suggested and I'm starting to work on them as soon as possible.

Great! Thanks for your hard work @frostbyte012

@innat
Copy link
Contributor

innat commented Apr 16, 2022

@LukeWood cc. @qlzh727
The RandomBrightness was moved to core Keras instead of Keras-cv, described here. In the same way, isn't it more suitable to have this layer in core Keras? As in core keras, we have resizing, and cropping!

@LukeWood
Copy link
Contributor

Hey @frostbyte012 are you planning to continue work on this feature? If not, I may begin working on this. This feature will (at some point soon) become a priority, as the layer is very important to the ecosystem!

Let me know! No pressure, but want to make sure we can move forward on this soon.

@frostbyte012
Copy link
Author

@LukeWood yes sir, I'm extremely sorry I wasn't well for few days , but I'm good to work on with it now, I really am passionate and excited enough to work on with this layer. I'm going to work on this from tomorrow onwards. I'm extremely sorry once again I should have informed about it to you before.

@LukeWood
Copy link
Contributor

@LukeWood yes sir, I'm extremely sorry I wasn't well for few days , but I'm good to work on with it now, I really am passionate and excited enough to work on with this layer. I'm going to work on this from tomorrow onwards. I'm extremely sorry once again I should have informed about it to you before.

Absolutely no worries - being an OSS contributor should be enjoyable and not stressful. Just wanted to check in. Thanks!

@frostbyte012
Copy link
Author

I'm working on the changes requested asap! Will be giving the updates as fast as possible

# Attribute to take random seed

#Methoid for taking the inputs
def call(self, inputs, training=True):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to override augment_image instead of call when subclassing BaseImageAugmentationLayer

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure sir working on it Asap!

Copy link
Author

@frostbyte012 frostbyte012 Apr 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: Resolved it and applied augment_image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LukeWood sir is the last commit all, correct? If not please let me know about it! Waiting to make the necessary changes. if required.

import numpy as np
import tensorflow as tf
from tensorflow.python.util.tf_export import keras_export
from tensorflow.tools.docs import doc_controls
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frostbyte012

  • keras should be imported from tensorflow, for example: from tensorflow.keras import ....
  • remove not-used packages, for ex. keras_export.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! Thanks @innat

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure @LukeWood and @innat sir! I'm removing the unwanted packages and also adding the correct format for keras import .

self.width = width
# Attribute to take the custom width
self.seed = seed
# Attribute to take random seed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may not be needed to give obvious comments.

@innat
Copy link
Contributor

innat commented Apr 26, 2022

@frostbyte012 Great work.
Is it possible to share some demo output of this layer?

@frostbyte012
Copy link
Author

@innat sure sir! Thanks!
Yes sir I'll most probably add an example of this layer to the examples

Copy link
Contributor

@LukeWood LukeWood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I left a few comments, but there are some fundamental changes that should occur. My guide on using BaseImageAugmentationLayer will cover these.

I'll link you to the guide when it is ready.

import numpy as np
import tensorflow as tf
from tensorflow.python.util.tf_export import keras_export
from tensorflow.tools.docs import doc_controls
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! Thanks @innat

keras_cv/layers/preprocessing/random_resized_crop.py Outdated Show resolved Hide resolved
@frostbyte012
Copy link
Author

@LukeWood and @innat sir I applied the suggested changes and working on the examples soon !
If you have some other changes or suggestions kindly let me know.
Thanks for the reviews!

@frostbyte012
Copy link
Author

frostbyte012 commented Apr 30, 2022

@innat and @LukeWood sir ! I just added a demo template test for random resized crop taking the random shear layer reference from preprocessing layers, kindly check the demo template and help me adjust and make the necessary changes, If require so. Thanks!

@innat
Copy link
Contributor

innat commented May 1, 2022

@frostbyte012 I was asking if you could display some sample output like this.

from keras.utils import image_utils

@tf.keras.utils.register_keras_serializable(package="keras_cv")
class RandomResizedCrop(tf.keras.internal.layers.BaseImageAugmentationLayer):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be __internal__.

@LukeWood
Copy link
Contributor

LukeWood commented May 5, 2022

Hey @frostbyte012, make sure to run unit tests before requesting another review! Really glad you are excited to contribute, but reviewer time is quite limited and as such it is best to be sure to test all code before requesting review.
E.x.: tf.keras.internal will fail to run as it should be .internal, and augment_image must augment a single image instead of a batch, etc.

Thanks @frostbyte012 and I'm glad you're excited to contribute!

@frostbyte012
Copy link
Author

@LukeWood I'm extremely sorry sir! I'm making sure to implement the changes and try on the unit test and make sure to get along with it ASAP! Extremely sorry for the inconvenience caused!

@LukeWood
Copy link
Contributor

Hey @frostbyte012! This item is pretty high on our critical roadmap, and I think we will need to pick it up to carry forward. I'm really glad you're excited to contribute to the repo! Maybe a great starting point would be this issue instead?

#425

Thanks!

@LukeWood LukeWood closed this May 12, 2022
@frostbyte012
Copy link
Author

I'm extremely sorry @LukeWood @innat couldn't keep up my promise and couldn't fulfill my commitment, I'm kinda sad about it, anyways thanks for reconsidering me for working with the other issue, I'll definitely not give up.. failing 100 times but rising the 101th time that should be our motto, I may not be experienced and not talented, but I wish my passion and dedication outperforms that. I was having some issues but now I'm comfortable, I wish to start working soon. Thanks

@LukeWood
Copy link
Contributor

I'm extremely sorry @LukeWood @innat couldn't keep up my promise and couldn't fulfill my commitment, I'm kinda sad about it, anyways thanks for reconsidering me for working with the other issue, I'll definitely not give up.. failing 100 times but rising the 101th time that should be our motto, I may not be experienced and not talented, but I wish my passion and dedication outperforms that. I was having some issues but now I'm comfortable, I wish to start working soon. Thanks

Do not worry at all! This is actually a very tricky issue. I tried to work on it the other day, and there are a LOT of edge cases to consider.

By the way, the guide on BaseImageAugmentationLayer is now live.

https://keras.io/guides/keras_cv/custom_image_augmentations/

I will take on RandomlyResizedCrop as it is really tricky! Please keep at it and I am sure we will get a contribution merged! I appreciate your hard work.

freedomtan pushed a commit to freedomtan/keras-cv that referenced this pull request Jul 20, 2023
Also tweaked documentation of `Cropping1D` for consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants