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

GaussianBlur and ColorJitter ops. #84

Merged
merged 14 commits into from
Jul 12, 2021
Merged

Conversation

barucden
Copy link
Collaborator

This PR introduces first two non-geometric operations, Gaussian blur and contrast & brightness adjustment. It partially solves #16.

At first, I thought I could implement it easily but I encountered some issues. I would greatly appreciate help resolving them (cc @johnnychen94).

  1. The introduced operations change the image eltype (e.g., ColorTypes.RGBA{FixedPointNumbers.N0f8} => ColorTypes.RGBA{Float64}). Is this something we need to worry about? This concern will perhaps apply for all non-geometric operations in future.
  2. What is a good way to test these operations? It is difficult to come up with a test-case whose expected output is not based on the formulas present in the code (e.g., for Rotate90 we know the expected output without evaluating the operation, which is not the case for GaussianBlur).
  3. For a segmentation task, these operations should be applied only on the input image, not the ground truth segmentation. How can we deal with that? Should we introduce a new abstract subtype for this purpose?

I have two additional issues regarding the AdjustContrastBrightness:

  1. The name seems too long. What would be a shorter alternative?
  2. The formula is now out[i] = clamp(α * img[i] + β * M), where M=mean(img). However, sometimes it is better to use M=the maximum value (e.g., for a grayscale image whose pixels are represented as 8-bit unsigned integer, M=255). I could not find a generic way how to get this value for different color types (yet, I am not too experienced). Is there a way?

@johnnychen94
Copy link
Collaborator

The introduced operations change the image eltype (e.g., ColorTypes.RGBA{FixedPointNumbers.N0f8} => ColorTypes.RGBA{Float64}). Is this something we need to worry about?

This is a separate issue. Most algorithms in JuliaImages don't take this assumption seriously so I think we can live this type of promotion without worrying about it too much. Generally, this issue can be fixed in upstream packages (e.g., ColorVectorSpace, FixedPointNumbers).

What is a good way to test these operations?

I would just use something as simple as ref = imfilter(img, gaussian((σ, σ), (k, k))) to be the test reference, but wrap it with assess_psnr(from ImageQualityIndexes) to allow some internal changes. This test looks trivial but it provides some guarantee that internal applyeager is correctly called.

For a segmentation task, these operations should be applied only on the input image, not the ground truth segmentation.

This is also a separate issue, maybe you can open a new issue with a minimal working example to discuss it? augment supports two usages:

augment(input_img, pipeline)
augment((input_img, ground_truth), pipeline) # augmentation applies uniformly to both images

I assume that you can just use the first version.


I have two additional issues regarding the AdjustContrastBrightness:

I forgot to mention this, maybe we can directly use https://github.com/JuliaImages/ImageContrastAdjustment.jl for the eager version, it introduces more contrast adjustment methods, including LinearStretching? Then open an issue in that repo for a lazy view. How do you think of this?

I would like to see more functionality of Augmentor being implemented in other packages, and leave Augmentor a pipeline composer.

@barucden
Copy link
Collaborator Author

I added tests for GaussianBlur but I use the standard pixel-wise comparison to check applyeager. I was trying to use assess_psnr but it seems that it lacks an implementation for RGBA images?

I like your idea about using ImageContrastAdjustment.jl. I am just not clear about the next step. Should I remove the proposed operation AdjustContrastBrightness for now? Or, as it seems that ImageContrastAdjustment.jl does not provide an operation equivalent to AdjustContrastBrightness, should I replace it with a different operation (such as LinearStretching) that comes from ImageContrastAdjustment.jl?

Copy link
Collaborator

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to use assess_psnr but it seems that it lacks an implementation for RGBA images?

Could possibly just use RGB.(testpattern()) as image input.

It's not very clear to me how should we process the alpha channel of RGBA. Are there any references/conventions for this?

However, sometimes it is better to use M=the maximum value (e.g., for a grayscale image whose pixels are represented as 8-bit unsigned integer, M=255). I could not find a generic way how to get this value for different color types (yet, I am not too experienced). Is there a way?

gamutmin/gamutmax from ColorTypes, and is also available in ImageCore (via Reexport).

I like your idea about using ImageContrastAdjustment.jl. I am just not clear about the next step. Should I remove the proposed operation AdjustContrastBrightness for now? Or, as it seems that ImageContrastAdjustment.jl does not provide an operation equivalent to AdjustContrastBrightness, should I replace it with a different operation (such as LinearStretching) that comes from ImageContrastAdjustment.jl?

Ah never mind. I think this is for different purposes. Although clamp(α * img[i] + β * M) is mathematically a linear stretching operation, it seems a more convenient way for augmentation purposes; while the ImageContrastAdjustment LinearStretching output is more human-readable (e.g., stretch to [0, 1] range).


The implementation looks good to me, let me know when it is ready.

src/operations/brightness.jl Outdated Show resolved Hide resolved
src/operations/blur.jl Outdated Show resolved Hide resolved
@johnnychen94
Copy link
Collaborator

One thing I forgot to mention, can you also add two cards to docs/operations? You can use other cards as templates and insert the filename to the corresponding config.json.

src/operations/color.jl Outdated Show resolved Hide resolved
@barucden
Copy link
Collaborator Author

new changes:

  • changed the name from AdjustContrastBrightness to ColorJitter (torchvision uses this name for a similar method)
  • added the feature based on gamutmax
  • added cards to docs/

Regarding the PSNR computed for RGBA images, I am not sure. I tried searching, but did not find any resources. Maybe the alpha channel could be treated just like any other channel?

I think that we can settle with converting to RGB. What is a reasonable PSNR threshold for which two images are considered equal?

@johnnychen94
Copy link
Collaborator

johnnychen94 commented Jul 12, 2021

Maybe the alpha channel could be treated just like any other channel?

For JuliaImages that ranges consistently from [0,1] this is reasonable. But for other library that uses [0, 255] range, it doesn't make much sense to me. Anyway, we doesn't need to worry about this in this PR.

What is a reasonable PSNR threshold for which two images are considered equal?

larger than 25-30 should be okay. I use 25 in ReferenceTests.

@barucden barucden changed the title [WIP] GaussianBlur and AdjustContrastBrightness ops. GaussianBlur and ColorJitter ops. Jul 12, 2021
@barucden
Copy link
Collaborator Author

Okay, I think it is ready now. Thanks for your advices @johnnychen94! Let me know if I should change something.

Copy link
Collaborator

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some small suggestions and questions. (It's midnight here, I'll come back tomorrow and approve tests if there are new commits; Github action by default disable the test running for non-contributor because months ago there are people sending spam PRs to other repositories to run mining script)

test/operations/tst_color.jl Outdated Show resolved Hide resolved
test/runtests.jl Show resolved Hide resolved
src/operations/color.jl Show resolved Hide resolved
src/operations/color.jl Outdated Show resolved Hide resolved
src/operations/blur.jl Show resolved Hide resolved
src/operations/blur.jl Outdated Show resolved Hide resolved
barucden and others added 4 commits July 12, 2021 20:14
Co-authored-by: Johnny Chen <johnnychen94@hotmail.com>
Co-authored-by: Johnny Chen <johnnychen94@hotmail.com>
@johnnychen94 johnnychen94 merged commit a121ac9 into Evizero:master Jul 12, 2021
@johnnychen94
Copy link
Collaborator

johnnychen94 commented Jul 12, 2021

Really nice work! The detailed descriptions in this PR and other issues are really appreciated.

@barucden barucden deleted the nongeom-ops branch July 13, 2021 08:15
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 this pull request may close these issues.

None yet

2 participants