add cutout image op #1338

fsx950223 · 2020-03-19T02:53:58Z

Related #1333

abhichou4 · 2020-03-19T06:56:13Z

tensorflow_addons/image/cutout_ops.py

+        if tf.equal(tf.rank(image), 3):
+            mask = tf.expand_dims(mask, -1)
+            mask = tf.tile(mask, [1, 1, tf.shape(image)[-1]])
+        elif tf.equal(tf.rank(image), 4):
+            mask = tf.expand_dims(mask, 0)
+            mask = tf.expand_dims(mask, -1)
+            mask = tf.tile(mask, [tf.shape(image)[0], 1, 1, tf.shape(image)[-1]])


Can from_4D_image and to_4D_image be used to handle this?

Changed. But I wonder about the performance of these ops.

abhichou4 · 2020-03-19T06:57:14Z

tensorflow_addons/image/cutout_ops_test.py

+        for channel in [0, 1, 3, 4]:
+            with self.subTest(channel=channel):
+                test_image = tf.image.decode_image(
+                    test_image_file, channels=channel, dtype=tf.uint8


Did you mean dtype=dtype here and in some other places

I have tested dtype in test_different_dtypes and it will increase test time if I use it on all test cases.

gabrieldemarmiesse · 2020-03-19T10:39:55Z

Thank you for this pull request! I believe we're going to change the API of the reference implementation to make it more user friendly. Here is what I propose:

def cutout(
    images: TensorLike,
    mask_size: TensorLike,
    offset: TensorLike= (0, 0),
    constant_values: Number = 0,
    data_format="channels_last"
) -> tf.Tensor:
"""

Args:
    image: A 4D tensor, (N, H, W, C). 
    mask_size: a tuple (or tensor) with two values. Height and width of the mask (correspond to 2x pad_size). A single scalar means a square mask (like in keras conv and pad layers). Can be a tensor of shape (N, 2) to have different mask sizes in the same batch.
    offset: the offset relative to the center of the image. Default is (0, 0) which means the mask will be in the middle of the image. Can be a tensor of shape (N, 2) to have different offsets in the same batch.
    constant_values: the values used to fill the mask.
    data_format: The data format
"""

The rational behind using only 4D tensors is that when the users passes 3D tensors, there is a non-negligable probability that user mistake NHW and HWC.

It's totally fine to not implement everything. For example, you can do

if data_format == "channels_first":
    raise NotImplementedError("Channels first is not yet available for cutout. Contributions welcome!")

if tf.rank(mask_size) != 0:
     raise NotImplementedError("Having non-square masks is not supported yet, contributions welcome.")

random_cutout should follow a similar pattern.

I believe that it should not change too much your code, it's already really close to the end result.

For the tests, please avoid adding images to the git repo, also we moving away from tf.test.TestCase and @run_all_in_graph_and_eager_mode. The philosophy now is to use plain pytest tests with the decorator @pytest.mark.usefixtures("maybe_run_functions_eagerly") . This enables eager mode in tests which helps a lot in readability. See #1328 .

gabrieldemarmiesse · 2020-03-22T17:03:16Z

@fsx950223 feel free to ping me when you're done with the changes.

fsx950223 · 2020-03-23T11:20:06Z

cc @gabrieldemarmiesse

gabrieldemarmiesse

Thank you for the pull request. We're nearly there, a few changes here and there need to be made.

gabrieldemarmiesse · 2020-03-23T13:49:15Z

tensorflow_addons/image/cutout_ops.py

+      images: A tensor of shape
+        (num_images, num_rows, num_columns, num_channels)
+        (NHWC), (num_images, num_channels, num_rows, num_columns)(NCHW), (num_rows, num_columns, num_channels) (HWC), or
+        (num_rows, num_columns) (HW).


We'll focus here only on 4D tensors for easier implementation and to avoid users shooting themselves in the foot.

gabrieldemarmiesse · 2020-03-23T13:50:14Z

tensorflow_addons/image/cutout_ops.py

+      mask_size: Specifies how big the zero mask that will be generated is that
+        is applied to the images. The mask will be of size
+        (2*pad_size[0] x 2*pad_size[1]).
+      constant_values: What pixel value to fill in the images in the area that has
+        the cutout mask applied to it.


We should specify what shapes are authorized, as it's not clear for someone who didn't read the code.

gabrieldemarmiesse · 2020-03-23T13:53:05Z

tensorflow_addons/image/cutout_ops.py

+
+def _norm_params(images, mask_size, data_format):
+    mask_size = tf.convert_to_tensor(mask_size)
+    if tf.equal(tf.rank(mask_size), 0):


For readability, I believe you can use:

Suggested change

if tf.equal(tf.rank(mask_size), 0):

if tf.rank(mask_size) == 0:

gabrieldemarmiesse · 2020-03-23T13:56:51Z

tensorflow_addons/image/cutout_ops.py

+                    mask_4d, [tf.shape(images)[0], tf.shape(images)[1], 1, 1]
+                )
+        images = tf.where(
+            tf.equal(mask, 0),


Same here, you can replace all the tf.equal by ==

gabrieldemarmiesse · 2020-03-23T14:02:09Z

tensorflow_addons/image/cutout_ops.py

+        shape=[], minval=0, maxval=image_height, dtype=tf.int32, seed=seed
+    )
+    cutout_center_width = tf.random.uniform(
+        shape=[], minval=0, maxval=image_width, dtype=tf.int32, seed=seed


If I understand correctly, if a batch of images is passed and random cutout is applied, the mask will be at the same place for all the images in the batch, right? Is this something we want? Maybe users would expect (as an augmentation strategy) to have the mask being at different places for all the images in the batch.
If the original implementation uses the same mask for all the images in the batch, let's go with this implementation, but the function shouldn't be public. I believe a public random_cutout shoud respect the principle of least astonishment.
What do you think?

Exactly! In a batch the cutout operation has to be random for each image.

fsx950223 · 2020-03-23T14:15:49Z

Thank you for the pull request. We're nearly there, a few changes here and there need to be made.

I will change them later.

fsx950223 · 2020-03-24T04:11:46Z

cc @gabrieldemarmiesse

gabrieldemarmiesse

Thank you again for your work, the implementation looks great! Some comments on the docs and the tests. I'm sorry to always ask more, but I feel this feature is going to be used by many for data augmentation, we need to make it perfect :)

gabrieldemarmiesse · 2020-03-25T17:42:10Z

tensorflow_addons/image/cutout_ops.py

+      mask_size: Specifies how big the zero mask that will be generated is that
+        is applied to the images. The mask will be of size
+        (2 * mask_height x 2 * mask_width).


mask_size should be the shape of the mask. Here the shape of the true mask is (2 * mask_height x 2 * mask_width) which might confuse users. Could you change that to (mask_height x mask_width)? You can throw an error if the width and height cannot be divided by 2.

gabrieldemarmiesse · 2020-03-25T17:42:32Z

tensorflow_addons/image/cutout_ops.py

+      mask_size: Specifies how big the zero mask that will be generated is that
+        is applied to the images. The mask will be of size
+        (2 * mask_height x 2 * mask_width).


Same as above.

gabrieldemarmiesse · 2020-03-25T17:43:17Z

tensorflow_addons/image/cutout_ops.py

+      mask_size: Specifies how big the zero mask that will be generated is that
+        is applied to the images. The mask will be of size
+        (2 * mask_height x 2 * mask_width).
+      offset: A tuple of (height, width)


Could you specify all the shapes are are possible? A tuple is possible but a 2D tensor too (batch_size, 2)

gabrieldemarmiesse · 2020-03-25T17:43:47Z

tensorflow_addons/image/cutout_ops.py

+    """Apply cutout (https://arxiv.org/abs/1708.04552) to images.
+
+    This operation applies a (2 * mask_height x 2 * mask_width) mask of zeros to
+    a random location within `img`. The pixel values filled in will be of the


Suggested change

a random location within `img`. The pixel values filled in will be of the

a location within `img` specified by the offset. The pixel values filled in will be of the

gabrieldemarmiesse · 2020-03-25T17:47:05Z

tensorflow_addons/image/cutout_ops_test.py

+@pytest.mark.usefixtures("maybe_run_functions_eagerly")
+def test_with_tf_function():
+    test_image = tf.ones([1, 40, 40, 1], dtype=tf.uint8)
+    result_image = tf.function(random_cutout)(test_image, 2)


Great way to test the graph mode :)
Here we need to make sure that the for loop with the TensorArray works well. To do that, we need to make sure the loop is possible even if the size of the batch is not known when drawing the graph. Could you use input_signature and set the shape of images to [None, 40, 40, 1] ?

gabrieldemarmiesse · 2020-03-25T17:49:40Z

tensorflow_addons/image/cutout_ops_test.py

+    np.testing.assert_allclose(tf.shape(result_image), tf.shape(expect_image))
+
+
+if __name__ == "__main__":


Could you add one more test to ensure that with random cutout, the masks are at different places in the images of the batch? I'll let you decide what's the easier way of doing that.

gabrieldemarmiesse · 2020-03-25T17:51:00Z

tensorflow_addons/image/cutout_ops_test.py

+@parameterized.named_parameters(
+    ("float16", np.float16), ("float32", np.float32), ("uint8", np.uint8)
+)


Suggested change

@parameterized.named_parameters(

("float16", np.float16), ("float32", np.float32), ("uint8", np.uint8)

)

@pytest.mark.parameterize("dtype", [np.float16, np.float32, np.uint8])

gabrieldemarmiesse · 2020-03-25T17:52:16Z

tensorflow_addons/image/cutout_ops_test.py

+    np.testing.assert_allclose(result_image, expect_image)
+
+


Suggested change

np.testing.assert_allclose(result_image, expect_image)

np.testing.assert_allclose(result_image, expect_image)

assert result_image.dtype == dtype

fsx950223 · 2020-03-26T09:47:21Z

cc @gabrieldemarmiesse .

gabrieldemarmiesse · 2020-03-26T10:26:14Z

tensorflow_addons/image/cutout_ops.py

+    with tf.control_dependencies(
+        [
+            tf.assert_equal(
+                tf.reduce_any(mask_size % 2 != 0),
+                False,
+                "mask_size should be divisible by 2",
+            )
+        ]
+    ):


control_dependencies is quite tf 1.x style, I recommend to activate the check only in eager mode. It will also be easier to debug for users.

Your piece of code raised a very important question that I discussed in #1417 . We don't need to fix #1417 for merging your pull request of course.

gabrieldemarmiesse

Thanks for this great pull request!

fsx950223 · 2020-03-26T12:08:26Z

Thanks for this great pull request!

Thanks for your review

* add cutout op * export module * remove test_utils * use tf.rank * remove decorator * add tf function test * fix cutout channels test * add norm param * change batch random strategy * fix flake8 * add more checks * add missing comment * add seed * remove control dependencies

fsx950223 requested review from facaiy and WindQAQ as code owners March 19, 2020 02:53

boring-cyborg bot added the image label Mar 19, 2020

googlebot added the cla: yes label Mar 19, 2020

abhichou4 reviewed Mar 19, 2020

View reviewed changes

abhichou4 mentioned this pull request Mar 19, 2020

Solarize_ops for Tensorflow Addons #1340

Closed

fsx950223 force-pushed the cutout branch from d87ca7c to cd24b19 Compare March 19, 2020 11:56

boring-cyborg bot added the github label Mar 19, 2020

fsx950223 added 2 commits March 19, 2020 12:01

add cutout op

e870ad7

export module

5203f3e

fsx950223 force-pushed the cutout branch from e21b4e7 to 5203f3e Compare March 19, 2020 12:02

fsx950223 and others added 5 commits March 19, 2020 12:05

remove test_utils

43c8e2d

use tf.rank

edaa30c

remove decorator

8be97a0

add tf function test

f057aff

fix cutout channels test

7723f22

gabrieldemarmiesse self-assigned this Mar 22, 2020

add norm param

40e31d3

gabrieldemarmiesse reviewed Mar 23, 2020

View reviewed changes

fsx950223 added 2 commits March 24, 2020 03:21

change batch random strategy

049b65a

fix flake8

0e9b39d

gabrieldemarmiesse requested changes Mar 25, 2020

View reviewed changes

add more checks

7a4ba5b

add missing comment

bebe07f

gabrieldemarmiesse requested changes Mar 26, 2020

View reviewed changes

fsx950223 added 2 commits March 26, 2020 10:26

add seed

595d63d

remove control dependencies

df94c2d

gabrieldemarmiesse mentioned this pull request Mar 26, 2020

Discussion: When to do checks of user inputs? #1417

Closed

gabrieldemarmiesse approved these changes Mar 26, 2020

View reviewed changes

gabrieldemarmiesse added the awaiting testing (then merge) label Mar 26, 2020

gabrieldemarmiesse merged commit 44ea32a into tensorflow:master Mar 26, 2020

fsx950223 mentioned this pull request May 16, 2020

fix #1824 #1840

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add cutout image op #1338

add cutout image op #1338

fsx950223 commented Mar 19, 2020

abhichou4 Mar 19, 2020

fsx950223 Mar 19, 2020

abhichou4 Mar 19, 2020 •

edited

Loading

fsx950223 Mar 19, 2020

gabrieldemarmiesse commented Mar 19, 2020

gabrieldemarmiesse commented Mar 22, 2020

fsx950223 commented Mar 23, 2020

gabrieldemarmiesse left a comment

gabrieldemarmiesse Mar 23, 2020

gabrieldemarmiesse Mar 23, 2020

gabrieldemarmiesse Mar 23, 2020

gabrieldemarmiesse Mar 23, 2020

gabrieldemarmiesse Mar 23, 2020

AakashKumarNain Mar 24, 2020

fsx950223 commented Mar 23, 2020

fsx950223 commented Mar 24, 2020

gabrieldemarmiesse left a comment

gabrieldemarmiesse Mar 25, 2020

gabrieldemarmiesse Mar 25, 2020

gabrieldemarmiesse Mar 25, 2020

gabrieldemarmiesse Mar 25, 2020

gabrieldemarmiesse Mar 25, 2020 •

edited

Loading

gabrieldemarmiesse Mar 25, 2020

gabrieldemarmiesse Mar 25, 2020

gabrieldemarmiesse Mar 25, 2020

fsx950223 commented Mar 26, 2020

gabrieldemarmiesse Mar 26, 2020

gabrieldemarmiesse Mar 26, 2020

gabrieldemarmiesse left a comment

fsx950223 commented Mar 26, 2020

	if tf.equal(tf.rank(mask_size), 0):
	if tf.rank(mask_size) == 0:

	a random location within `img`. The pixel values filled in will be of the
	a location within `img` specified by the offset. The pixel values filled in will be of the

		np.testing.assert_allclose(tf.shape(result_image), tf.shape(expect_image))


		if __name__ == "__main__":

add cutout image op #1338

add cutout image op #1338

Conversation

fsx950223 commented Mar 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhichou4 Mar 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrieldemarmiesse commented Mar 19, 2020

gabrieldemarmiesse commented Mar 22, 2020

fsx950223 commented Mar 23, 2020

gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fsx950223 commented Mar 23, 2020

fsx950223 commented Mar 24, 2020

gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrieldemarmiesse Mar 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fsx950223 commented Mar 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrieldemarmiesse left a comment

Choose a reason for hiding this comment

fsx950223 commented Mar 26, 2020

abhichou4 Mar 19, 2020 •

edited

Loading

gabrieldemarmiesse Mar 25, 2020 •

edited

Loading