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

[AutoAugment] Add Rotate operation. #402

Closed
sebastian-sz opened this issue Apr 30, 2022 · 12 comments
Closed

[AutoAugment] Add Rotate operation. #402

sebastian-sz opened this issue Apr 30, 2022 · 12 comments
Labels
needs-impact-verification Unclear whether or not the feature should be included.

Comments

@sebastian-sz
Copy link
Contributor

Short Description
Rotate is one of the operations used by AutoAugment. Currently it's not present in keras_cv, so AutoAugment cannot be implemented.

What it does
From Tensorlfow Addons docs: Rotate image(s) counterclockwise by the passed angle(s) in radians.

Papers
AutoAugment: Learning Augmentation Strategies from Data
arxiv link

Existing Implementations
Tensorflow addons
tensorflow/tpu

Other Information

  • This is different from keras RandomRotation as we need to be able to control the probability at which the op is applied.
  • Following standard Keras layer naming this could probably be called Rotation?
@sebastian-sz
Copy link
Contributor Author

It looks like linked implementation uses ImageProjectiveTransformV3 which will fail with XLA: tensorflow/tensorflow#55194

@bhack
Copy link
Contributor

bhack commented Apr 30, 2022

Yes, honestly it already fails on master to optimize the performance without XLA (as many other ops that we currently use):

#291 (comment)

See also my rationale at:
#361 (comment)

@bhack
Copy link
Contributor

bhack commented Apr 30, 2022

P.s there is a partial coverage effort (translation only) with tensorflow/tensorflow#55335 but for MLIR (as XLA is still a different code path today in TF)

@LukeWood
Copy link
Contributor

LukeWood commented May 2, 2022

Hi @sebastian-sz! Is this different from the RandomRotation KPL? We can use utils.transform however needed if we must.

@sebastian-sz
Copy link
Contributor Author

@LukeWood In terms of using the layer as a part of AutoAugment I do believe this is different. For instance, with default KLP we cannot directly control the probability as well as "angles" parameter at which the Rotation's are applied, which is necessary in the AutoAugment pipeline.

@quantumalaviya
Copy link
Contributor

I have added some utils from keras that are required for the rotation and the translation as part of #407, if that's any help. I too had to code the rotate operation for AugMix. This layer may be useful.

@LukeWood
Copy link
Contributor

LukeWood commented May 3, 2022

I have added some utils from keras that are required for the rotation and the translation as part of #407, if that's any help. I too had to code the rotate operation for AugMix. This layer may be useful.

Is there any use case we can think of that requires this to be a layer; in particular: will you ever want to preprocess your inputs by ALWAYS rotating by X deg? If not, maybe it is best as a preprocessing util

@quantumalaviya
Copy link
Contributor

True, it makes no sense to rotate images at a fixed angle.

This util may be useful*

@sebastian-sz
Copy link
Contributor Author

will you ever want to preprocess your inputs by ALWAYS rotating by X deg?

@LukeWood yes - this is actually the case in AutoAugment, where you want to rotate the image by X degrees based on subpolicy's magnitude:
rotate_autoaugment

@quantumalaviya Thanks for linking #407 I didn't know about it and it looks like exactly the thing needed.

I agree that this function doesn't need to be a KLP and is good enough as a utility function.
From my perspective this issue can be closed.

@LukeWood
Copy link
Contributor

LukeWood commented May 3, 2022

Cool, let’s leave it open until we can merge @quantumalaviya ’s PR and actually include the utility to rotate an image a fixed amount

@LukeWood
Copy link
Contributor

Hey @sebastian-sz for now lets close this until we decide to fully pursue AutoAugment! If someone wants to pursue it, we can discuss but for now we are just recommending the use of RandAugment as it's search space is MUCH easier to traverse.

Cheers!

@sebastian-sz
Copy link
Contributor Author

@LukeWood absolutely! Upon further reading I even think that current Rotate ops are sufficient for this.

freedomtan pushed a commit to freedomtan/keras-cv that referenced this issue Jul 20, 2023
* Fix typo in functional model input flattening

* Add unit test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-impact-verification Unclear whether or not the feature should be included.
Projects
None yet
Development

No branches or pull requests

5 participants