Closed Mantas-Skackauskas closed 3 years ago
Thank you for your PR! This is an important issue.
Using brightness requires a PIL Image which lives in [0, 255]. At this point, we might receive pretty much anything from floats to scaled ints.
While I agree we need to fix this bug, I don't think this is the right fix.
Could we do smart clipping? If it's a float image: scale, apply brightness and clip between 0 and 1 If it's an int image: apply brightness, clip between 0 and 255
Also we might want to investigate if adding Brightness is nothing more than np.clip(img*brightness, 0, 255)
. If that's the case, then we can do it ourselves I think.
Open to suggestions.
Thank you for your PR! This is an important issue.
Using brightness requires a PIL Image which lives in [0, 255]. At this point, we might receive pretty much anything from floats to scaled ints.
While I agree we need to fix this bug, I don't think this is the right fix.
Could we do smart clipping? If it's a float image: scale, apply brightness and clip between 0 and 1 If it's an int image: apply brightness, clip between 0 and 255
Also we might want to investigate if adding Brightness is nothing more than
np.clip(img*brightness, 0, 255)
. If that's the case, then we can do it ourselves I think.Open to suggestions.
@Dref360 Hello, thank you for the message. I have updated this pull request with a code addition that leaves the code as it is and rescales the values to their original range correctly and supports any input range. I am awaiting your suggestions.
Almost ready to merge I think. Could we add some tests to check the cases where we have floats or data not in [0,255]?
Almost ready to merge I think. Could we add some tests to check the cases where we have floats or data not in [0,255]?
@Dref360 Thanks, I have added 2 tests for positive and negative values (1024 and -1024 respectively). The tests are similar to the existing test test_random_brightness_scale()
right above. Let me know if there is anything else I should add.
Why use PIL for such a simple operation in the first place?
Prevents the apply_brightness_shift() function call to alter the image by min-max scaling it because of the missing scale parameter. Issue link: https://github.com/keras-team/keras-preprocessing/issues/327
Summary (updated Dec 10, 2020)
Problem description
I was using ImageDataGenerator with the parameter 'brightness_range'. This lead to unexpected behaviour where using the brightness range in interval [1,1] altered the image even though nothing should have happened. Looking more into the code, I noticed that the brightness range function generates a value 'brightness' from the uniform distribution with interval specified in the 'brightness_range' parameter, hence, the only value from U(1,1) must be 1. However, when the generated value 'brightness' is passed into the function apply_brightness_shift at line 888 seen below
https://github.com/keras-team/keras-preprocessing/blob/58df11e1145b2088092252c4dba02168c6da2b13/keras_preprocessing/image/image_data_generator.py#L888
the scale parameter is ignored and leads to the function using the default parameter 'scale'=True seen below:
https://github.com/keras-team/keras-preprocessing/blob/58df11e1145b2088092252c4dba02168c6da2b13/keras_preprocessing/image/affine_transformations.py#L215
This leads to a scaled output in range from [0-255] which alters the original image value range image-wise, and therefore performs unintentional min-max scaling to the input. Considering this function should only perform brightness modification and not per image min-max scaling of the values, such unexpected behaviour should not happen.
How I found this issue
I found this issue while using TensorFlow (2.3.1) package with function ImageDataGenerator which still relies on calls to keras API and took some time to locate where the issue is coming from.
How to replicate the problem
This leads to the altered image:
Solution
I propose to set the scale parameter in the line 888 to False which will perform the brightness_range without scaling and will return the expected outcome:
Update (Dec 8, 2020)
After some headache, I found out a number of things:
I suggested this in my updated commit:
For comparison, let's have two functions.
The original apply_brightness_shift(...)
The new apply_brightness_shift(...)
And let's create a function to compare input versus output and the difference between the two:
Let our image be:
Then using the
compare_input_output
we get:OLD:
old_apply_brightness_shift
For scale set to False and brightness set to 1.0, 1.5 and 0.5 of the original brightness:NEW:
apply_brightness_shift
For scale set to False and brightness set to 1.0, 1.5 and 0.5 of the original brightness:Update (Dec 10, 2020)
Added two tests for matrix values outside the expected [0,255] range:
Related Issues
https://github.com/keras-team/keras-preprocessing/issues/327
PR Overview