keras-team / tf-keras

The TensorFlow-specific implementation of the Keras API, which was the default Keras from 2019 to 2023.
Apache License 2.0
63 stars 28 forks source link

Scipy affine transform #89

Open bhack opened 2 years ago

bhack commented 2 years ago

System information.

TensorFlow version (you are using): master Are you willing to contribute it (Yes/No) : I need more detail Describe the feature and the current behavior/state. I think that we need to cover core image processing transformation with TF native ops.

Currently a core transformation in preprocessing still rely on numpy/scipy impl. https://github.com/keras-team/keras/blob/master/keras/preprocessing/image.py#L2622

Describe the feature clearly here. Be sure to convey here why the requested feature is needed. Any brief description about the use-case would help.

Will this change the current api? How?

Who will benefit from this feature?

Contributing

bhack commented 2 years ago

/cc @qlzh727 @LukeWood as this could have a quite core impact on CV ops.

bhack commented 2 years ago

I suppose that we could port this to the internal transform right?

I have also a related XLA ticket (for jit_compile) at https://github.com/tensorflow/tensorflow/issues/55194

haifeng-jin commented 2 years ago

@mattdangerw Is this also related to our refactoring of keras preprocessing?

mattdangerw commented 2 years ago

It's not just apply_affine_transform, most of the code in the keras/preprocessing directory is doing numpy transformations of images/text/etc. Rather then rewriting these non-layer utilities to a tensor friendly form, we plan on redirecting users to preprocessing layers that can achieve a similar effect with native ops. We generally want layers not utilities for new code when possible.

Zooming out, we are mostly covering the keras.preprocessing.image offering with core keras preprocessing layers and upcoming layers in keras-cv.

But apply_affince_transform we do need to figure out. We could direct users to tfa.image.transform, to tf.raw_ops.ImageProjectiveTransformV3 directly, or write a replacement preprocessing layer for affine transformations in keras-cv. I suspect this is common enough that an AffineTransformation layer in keras-cv would be the most friendly option long term. What do people think?

bhack commented 2 years ago

What I meant is:

https://github.com/keras-team/keras/blob/bb3c71ea2d21b204a779ebd9746a058eab3f80b1/keras/layers/preprocessing/image_preprocessing.py#L975-L981

mattdangerw commented 2 years ago

Not sure I follow?

At least in my understanding, I don't believe we want to port any of the keras.preprocessing symbols to tensorized op implementations. Essentially because we don't want to encourage those APIs. Basically everything in keras.preprocessing would be better done as a Keras preprocessing layer.

We will be starting to shipping guidance in 2.9 (see https://www.tensorflow.org/api_docs/python/tf/keras/preprocessing?version=nightly) to start pointing users away from these symbols are towards the tf.data + preprocessing layers way of structuring things. This will be an ongoing effort. So our task with things like keras.preprocessing.image.apply_affine_transform should be to figure out how to sign post away from those symbols.

Also note that we will not be breaking these symbols in anyway! They fall under our compatibility guarantees, and will keep working for the foreseeable future. It is just that we don't want to target the keras.preprocessing utilities, broadly speaking, with work to make them more efficient. Critical bug fixes only at this point.

bhack commented 2 years ago

It Is ok for me if this is your internal goal. So you can use this ticket to track the introductiom of a deprecation warning (eventually with an EOL) as in TF.

mattdangerw commented 2 years ago

Thanks! The guidance away from preprocessing utilities (in keras.preprocessing) and towards preprocessing layers (in keras.layers) will be going live with 2.9 (shortly I think). I don't believe we will have a EOL for these symbols, as they should continue to function for the duration of tensorflow 2.x.

Most official tf guides have moved off of keras.preprocessing but this will still be an ongoing effort, particular with keras.io examples.

bhack commented 2 years ago

Generally a deprecation warning help a lot for a "natural migration" other then the guidance as not all users read extensively docs.