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

Vectorize Auto Contrast. #1382

Closed
sebastian-sz opened this issue Feb 9, 2023 · 25 comments · Fixed by #1394
Closed

Vectorize Auto Contrast. #1382

sebastian-sz opened this issue Feb 9, 2023 · 25 comments · Fixed by #1394

Comments

@sebastian-sz
Copy link
Contributor

sebastian-sz commented Feb 9, 2023

Short Description
Testing the water with this issue as #1373 is merged.

How do you feel about migrating more layers to use vectorized approach? Below is a simple performance comparison of vectorized vs current.

EDIT: misclick and submitted the issue too early...

Eager performance is better.
Graph performance is the same.
Eager performance
Graph performance

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

Can you do the same with RandomContrast? Thanks

@sebastian-sz
Copy link
Contributor Author

@bhack Similar behaviour:

  1. Eager: vectorized is quite faster
  2. Graph (tf.function): similar, but this time original is a bit faster.
  3. Graph (XLA): similar, but vectorized is a bit faster.
    Random Contrast - eager
    Random Contrast - tf function
    Random Contrast - XLA

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

Are these on CPU or on GPU?

@sebastian-sz
Copy link
Contributor Author

CPU

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

Is this the result of RandomContrast with a single value or a list/tuple for the factor param?

@sebastian-sz
Copy link
Contributor Author

Single value: RandomContrast(0.5) - I don't think this should affect performance?

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

I suppose not:

if isinstance(factor, (tuple, list)):
min = 1 - factor[0]
max = 1 + factor[1]
else:
min = 1 - factor
max = 1 + factor
self.factor_input = factor
self.factor = preprocessing.parse_factor((min, max), min_value=-1, max_value=2)
self.seed = seed

So It could be interesting to test, please also try to use realistic enough input (WxH) size for production use cases.

@bhack

This comment was marked as resolved.

@sebastian-sz
Copy link
Contributor Author

@bhack I don't think there is a bug:
self.factor = preprocessing.parse_factor((min, max), min_value=-1, max_value=2) is parsed into UniformFactorSampler, which returns a float, sampled from a uniform random distribution upon call.

@sebastian-sz
Copy link
Contributor Author

please also try to use realistic enough input (WxH) size for production use cases.

Here is a basic script that I'm using:

# layer = RandomContrast((0,1))
layer = VectorizedRandomContrast((0,1))

@tf.function
def graph_mode(inputs):
    return layer(inputs)

@tf.function(jit_compile=True)
def xla_mode(inputs):
    return layer(inputs)

for batch_size in range(8, 132, 8):
    input_shape = (batch_size, 224, 224, 3)
    batch = tf.random.uniform(input_shape)

    for _ in range(10):
        layer(batch)

    results = []
    for _ in range(100):
        batch = tf.random.uniform(input_shape)
        start = time.perf_counter()
        layer(batch)
        stop = time.perf_counter()
        results.append(stop-start)

    print(batch_size, float(tf.reduce_mean(results).numpy()))

@sebastian-sz
Copy link
Contributor Author

@bhack Sorry, my mistake in the previous comment. I'm using tuple for factor, not 0.5.

@bhack

This comment was marked as outdated.

@bhack

This comment was marked as outdated.

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

Ok sorry my bad, you are right it was a call not a variable.

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

@sebastian-sz I still don't undertand with your code example:

import tensorflow as tf
from tensorflow import keras
from keras_cv.layers.preprocessing import RandomContrast

layer = RandomContrast((0,1))
input_shape = (20, 224, 224, 3)
batch = tf.random.uniform(input_shape)
layer(batch)

And if you add a debug print tf.print(contrast_factor) I still see only a single value.

def augment_image(self, image, transformation, **kwargs):
contrast_factor = transformation

@sebastian-sz
Copy link
Contributor Author

sebastian-sz commented Feb 9, 2023

@bhack Sorry for the confusion. The 0,1 argument was meant to be value_range but I passed it as a factor by accident. This is only a benchmark so I don't think the value shift matters.

In this case the 0,1 is treated as min and max values, which after slight modifications:

if isinstance(factor, (tuple, list)):
min = 1 - factor[0]
max = 1 + factor[1]

Are passed to uniform factor sampler:

def __init__(self, lower, upper, seed=None):
self.lower = lower
self.upper = upper
self.seed = seed
def __call__(self, shape=(), dtype="float32"):
return tf.random.uniform(
shape, seed=self.seed, minval=self.lower, maxval=self.upper, dtype=dtype
)

This sampler seems to be only a wrapper around tf.random.uniform to set it's min and max values.
Hence calling self.factor() is equivalent to calling tf.random.uniform with the default scalar shape ().

This is the value that can be printed via tf.print(contrast_factor) and it's the value passed to tf.image.adjust_contrast.

Currently, in case of vectorized implementation, if we use this self.factor() + tf.image.adjust_contrast (if I'm not mistaken) every image in the batch receives the same contrast_factor.

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

So where is the within the batch randomization in RandomContrast?

@sebastian-sz
Copy link
Contributor Author

@bhack Wait, it's not called per sample?

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

I have used that gist.
I supposed the we have 1 factor for each sample in the batch but it print only a single scalar

@sebastian-sz
Copy link
Contributor Author

From the code it looks like it is sampling new scalar per image:

def _batch_augment(self, inputs):
return self._map_fn(self._augment, inputs)

_augment calls get_random_transformation, but on my end it's also printing only a single scalar, regardless of batch size.

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

@LukeWood ?

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

@sebastian-sz I suppose that map_fn expect a stacked elem. But who is stacking factor?

elems: A tensor or (possibly nested) sequence of tensors, each of which will be unstacked along their first dimension. 

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

_augment calls get_random_transformation, but on my end it's also printing only a single scalar, regardless of batch size.

_augment is only called once for the whole batch and now I am very confused on how the map_fn call was organized.

@bhack
Copy link
Contributor

bhack commented Feb 9, 2023

@sebastian-sz Check the analysis at https://github.com/keras-team/keras-cv/issues#issuecomment-1424701740.

It is why this benchmark seems so unexpected.

@LukeWood
Copy link
Contributor

@sebastian-sz feel free to contribute this! Thanks a lot for the contribution!

In general we should look to vectorize layers unless there’s a clear reason not to.

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 a pull request may close this issue.

3 participants