Add basic functions for colorspace transforms. #3

SomTambe · 2021-06-07T07:54:33Z

This PR makes the base for any colorspace transform.

add colorify and channelify functions.

Usage:

julia> f = Chain(x->HSV.(x),channelify,flatten,Dense(768,16),Dense(16,10),x->σ.(x))

julia> f(rand(RGB,16,16,1)) # single image of dims 16 x 16

colorify can be used if one ever requires a colorspace transform between two layers.

julia> f = Chain(...,
                         x->colorify(RGB,x),
                         x->Gray.(x),
                         channelify,
                         ...
                         ) # this would allow for a Grayscale colorspace transform in between the network

I am not sure if ever this will come to use. It's better to provide an option rather than not providing anything at all.
I would love to hear your comments @DhairyaLGandhi @johnnychen94

Also, channelview is not differentiable. I will be writing an adjoint for that.

johnnychen94

This is a good start.

One thing I notice about this PR is that permutedims creates a memory contiguous array while colorview doesn't. This implies that channeify creates a contiguous dense array, while colorify does not. This memory non-contiguity can be a source of performance regression.

Perhaps a 1-argument version of colorify would be useful, i.e., colorify(RGB) that creates x->colorify(RGB, x).

Should this function specially handle ndims(m) == 2? I guess no? If so it's probably worth documenting something like "the last dimension is faithfully recognized as a batch dimension so you need to unsqueeze the input manually if the input is only one sample"

src/colors/conversions.jl

SomTambe · 2021-06-07T09:14:32Z

Should this function specially handle ndims(m) == 2? I guess no? If so it's probably worth documenting something like "the last dimension is faithfully recognized as a batch dimension so you need to unsqueeze the input manually if the input is only one sample"

Yes you are right. I do require them to have the last dimension as a batch. I should document it as you say.

src/colors/conversions.jl

SomTambe · 2021-06-08T11:48:01Z

@johnnychen94 Should I merge this PR? I'll add the adjoints in another PR, will that be fine?

johnnychen94 · 2021-06-08T11:49:51Z

Every non-trivial PR should have its associated test cases.

SomTambe · 2021-06-08T11:51:48Z

Every non-trivial PR should have its associated test cases.

I completely forgot about the tests. No worries I will write them.

SomTambe · 2021-06-08T13:26:02Z

@johnnychen94 I have written the tests. Could you review them please?

johnnychen94

I feel this unit test is unnecessarily complicated. You don't need to introduce all other layers as long as the basic properties of channelify/colorify get tested.

Why not just test

X = rand(RGB{N0f8}, 4, 4, 5)
Y = channelify(X)

@test Y[:, :, 1, 1] == channelview(X[:, :, 1])
@test size(Y) == (4, 4, 3, 5)
@test eltype(Y) == N0f8
# make sure it has contiguous memory layout
@test stride(Y, 1) == 1
# maybe there're other properties

and also test 2d, 3d and 4d cases. And test various colorspace, e.g., RGB, HSV, Gray, RGBA, Gray24.

SomTambe · 2021-06-13T15:57:56Z

I have added the adjoints for the necessary functions. I'll be adding the tests by tonight too.
cc. @DhairyaLGandhi @johnnychen94

src/colors/conversions.jl

DhairyaLGandhi · 2021-06-13T17:15:36Z

tests/colors/conversions.jl

+    @testset "Testing colorify" begin
+        @test size(g_3(rand(20,20,7,3))) == (10,3)
+        @test size(g_1(rand(20,20,7,3))) == (10,3)
+    end


Also test for differentiability

johnnychen94 · 2021-06-13T17:53:51Z

src/colors/conversions.jl

+          :DIN99,:ADIN99,:DIN99A,
+          :LMS,:ALMS,:LMSA,
+          :YIQ,:AYIQ,:YIQA)
+    @eval @adjoint function $f(x...)


This adjoint is for RGB(1, 1, 1), we would also need adjoint for c=Lab(100, 0, 0); RGB(c)

Suggested change

@eval @adjoint function $f(x...)

@eval @adjoint function $f(x::Real...)

Why should it be for Real only?

My first look is that the pullback is not correct for the Colorant inputs.

The most important thing currently is to add unit tests. More than 70% of the efforts should be paid to unit tests to make sure things are written correctly.

With unit tests appropriately written, we can then revisit this one and see if it is correct for colorant inputs.

src/colors/conversions.jl

johnnychen94 · 2021-06-13T17:57:22Z

src/colors/conversions.jl

+end
+
+# adjoint for colorview
+@adjoint function colorview(T, x) where {T}


There's also a multi inputs version: colorview(T, gray1, gray2...)

Does colorview handle batches?

Yes it does

Good question. Yes it does; channelview and colorview is just a wrapper of Base.ReinterpretedArray:

julia> X = rand(RGB, 12, 24, 64); julia> channelview(X) |> size (3, 12, 24, 64) julia> colorview(eltype(X), channelview(X)) == X true

johnnychen94 · 2021-06-13T17:59:34Z

src/colors/conversions.jl

+    e = eltype(x)
+    y = channelview(x)
+    function pullback(Δ)
+        return (colorview(e,Δ),)


does collect(colorview(e, Δ)) solve the issue in FluxML/Zygote.jl#993?

src/colors/conversions.jl

johnnychen94

A good start on improving coverage. Nicely written 👍

some numerical tests are still required, e.g., 1) differentiability 2) reversibility (that channelify(colorify(RGB, data)) == data and colorify(RGB, channelify(rgb_data)) == rgb_data

tests/colors/conversions.jl

SomTambe · 2021-06-17T16:42:21Z

If the adjoint of (CT<:Colorant)(x::T) where T is still an issue. The relevant codes should be removed for this PR and added back in future PR.

I have commented out the tests which would evaluate gradients for these. This would be added in the next PR.

Added gradients for colorview and channelview.
Completed CI integration.

I am still debugging CI integration. Will be done soon.
I feel the colorview derivative tests are a bit messy.

SomTambe · 2021-06-17T18:13:10Z

@johnnychen94 I have done all the required tasks we needed. I have also created #5 to track all our progress remaining over here.

johnnychen94

Last round of comments with only style suggestions.

@DhairyaLGandhi any more comments?

.github/workflows/ci.yml

src/colors/conversions.jl

test/colors/conversions.jl

SomTambe · 2021-06-18T06:06:57Z

Committed all the changes you asked for.

johnnychen94 · 2021-06-18T08:00:27Z

.github/workflows/ci.yml

@@ -42,8 +42,15 @@ jobs:
      - run: |
          julia --project=docs -e '
            using Pkg
+            Pkg.add("Documenter")


This one can also be added into docs/Project.toml

johnnychen94 · 2021-06-18T08:01:42Z

.github/workflows/ci.yml

+      - run: |
+          julia --project=docs -e '
+            using Pkg
+            using Documenter: doctest
+            using DiffImages
+            doctest(DiffImages)'


Sorry that I didn't make it clear. I was thinking of adding doctest(DiffImages) into test/runtests.jl. See OffsetArrays as an example.

DhairyaLGandhi

Largely LGTM , added a couple thoughts on the tests and let's get this merged!

src/colors/conversions.jl

test/colors/conversions.jl

SomTambe · 2021-06-22T14:36:35Z

I am merging the PR as per conversation with @DhairyaLGandhi
cc @johnnychen94

DhairyaLGandhi · 2021-06-22T14:42:15Z

src/colors/conversions.jl

+(25, 25, 7)
+```
+"""
+function colorify(color::Type{CT}, m::AbstractArray) where CT <: Colorant


I feel like we may want to restrict this function to AbstractArray{T,4} since the case of (28,28,3) with Grayscale should be a simple reinterpret

The current function gives the freedom of any number of dims >= 2. We could restrict it to 4 but it could be we may require something for higher dimensions in the future.

My main concern is the case I highlighted earlier. If that is handled correctly, then I'm fine

I understand that classic deep learning mainly deals with 4D tensor but I'm not very positive about this type constraint; especially when it's not due to implementation restriction.

I am the first person to not restrict types for sure, just wanted to ensure we're doing the correct thing for different inputs we can expect to get.

Add basic functions for colorspace transforms.

901b875

johnnychen94 reviewed Jun 7, 2021

View reviewed changes

src/colors/conversions.jl Outdated Show resolved Hide resolved

src/colors/conversions.jl Outdated Show resolved Hide resolved

SomTambe added 6 commits June 7, 2021 14:52

Modify docs.

093f8b7

Make vector a tuple

3c39144

Removed CUDA from ependency

684ea01

Add single not for batch dim

f714e2a

Add ChainRules dependency

9f92224

Added V to LICENSE

d11a5cb

johnnychen94 reviewed Jun 8, 2021

View reviewed changes

src/colors/conversions.jl Outdated Show resolved Hide resolved

Add tests.

f074375

Remove newline from EOF

15d1d31

johnnychen94 reviewed Jun 8, 2021

View reviewed changes

SomTambe added 4 commits June 9, 2021 21:52

Add channelify adjoint

fbcb568

sample commit using gpg

9511d4b

Signed commit

26aa7a0

Add custom adjoints for colorview, channelview, (::Colorant{T,N})

d348861

DhairyaLGandhi reviewed Jun 13, 2021

View reviewed changes

Make necessary changes

8cd6768

johnnychen94 reviewed Jun 13, 2021

View reviewed changes

SomTambe added 4 commits June 14, 2021 15:24

\Delta changed to (\Delta...,)

76e77ea

Complete adjoints for (CT<:Colorant{T})(::Real)

0fe299e

Removed debugging line

0141543

Add tests for channelify

1a3ec8f

johnnychen94 reviewed Jun 16, 2021

View reviewed changes

tests/colors/conversions.jl Outdated Show resolved Hide resolved

SomTambe added 7 commits June 17, 2021 18:11

Added Documenter package

5fdec09

Merge branch 'main' of github.com:SomTambe/DiffImages.jl into dev

c019c8b

Make changes

02e9085

Change name of folder from tests to test

75797b0

Add Test dependency

598b427

Add channelview and colorview differentiability tests

c11cb13

Add note for colorview(T, gray1, gray2...) adjoint and tests

62a66a0

SomTambe added 6 commits June 17, 2021 22:14

Change doctests a bit

95e10a9

Changed the docstring arguments

a71d974

add requirements

2b1e4aa

Add Images to doctests

ace5a7f

Add Test dependency

89b15d5

Fix CI issue

7f4cc3f

SomTambe mentioned this pull request Jun 17, 2021

Add adjoints for (<: Colorants) #5

Open

3 tasks

johnnychen94 approved these changes Jun 17, 2021

View reviewed changes

SomTambe added 2 commits June 18, 2021 09:44

Add Project.toml file for docs

fae45dd

remove comments

041507c

Commit necessary changes

1bbce28

johnnychen94 approved these changes Jun 18, 2021

View reviewed changes

DhairyaLGandhi reviewed Jun 18, 2021

View reviewed changes

src/colors/conversions.jl Show resolved Hide resolved

src/colors/conversions.jl Show resolved Hide resolved

test/colors/conversions.jl Show resolved Hide resolved

SomTambe added 2 commits June 21, 2021 22:18

Change unsqueeze to reshape

45e1787

Add tests for channelify and colorify

f6ddc1a

SomTambe requested a review from DhairyaLGandhi June 21, 2021 17:22

DhairyaLGandhi approved these changes Jun 22, 2021

View reviewed changes

SomTambe merged commit f9694c5 into main Jun 22, 2021

DhairyaLGandhi reviewed Jun 22, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic functions for colorspace transforms. #3

Add basic functions for colorspace transforms. #3

SomTambe commented Jun 7, 2021

johnnychen94 left a comment •

edited

Loading

SomTambe commented Jun 7, 2021

SomTambe commented Jun 8, 2021

johnnychen94 commented Jun 8, 2021

SomTambe commented Jun 8, 2021

SomTambe commented Jun 8, 2021

johnnychen94 left a comment

SomTambe commented Jun 13, 2021

DhairyaLGandhi Jun 13, 2021

SomTambe Jun 13, 2021

johnnychen94 Jun 13, 2021

SomTambe Jun 14, 2021

johnnychen94 Jun 14, 2021

johnnychen94 Jun 13, 2021

DhairyaLGandhi Jun 16, 2021

SomTambe Jun 16, 2021

johnnychen94 Jun 16, 2021

johnnychen94 Jun 13, 2021

johnnychen94 left a comment •

edited

Loading

SomTambe commented Jun 17, 2021 •

edited

Loading

SomTambe commented Jun 17, 2021

johnnychen94 left a comment

SomTambe commented Jun 18, 2021

johnnychen94 Jun 18, 2021

johnnychen94 Jun 18, 2021

DhairyaLGandhi left a comment

SomTambe commented Jun 22, 2021

DhairyaLGandhi Jun 22, 2021

SomTambe Jun 22, 2021

DhairyaLGandhi Jun 22, 2021

johnnychen94 Jun 22, 2021

DhairyaLGandhi Jun 22, 2021

	@eval @adjoint function $f(x...)
	@eval @adjoint function $f(x::Real...)

Add basic functions for colorspace transforms. #3

Add basic functions for colorspace transforms. #3

Conversation

SomTambe commented Jun 7, 2021

johnnychen94 left a comment • edited Loading

Choose a reason for hiding this comment

SomTambe commented Jun 7, 2021

SomTambe commented Jun 8, 2021

johnnychen94 commented Jun 8, 2021

SomTambe commented Jun 8, 2021

SomTambe commented Jun 8, 2021

johnnychen94 left a comment

Choose a reason for hiding this comment

SomTambe commented Jun 13, 2021

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnychen94 left a comment • edited Loading

Choose a reason for hiding this comment

SomTambe commented Jun 17, 2021 • edited Loading

SomTambe commented Jun 17, 2021

johnnychen94 left a comment

Choose a reason for hiding this comment

SomTambe commented Jun 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DhairyaLGandhi left a comment

Choose a reason for hiding this comment

SomTambe commented Jun 22, 2021

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

johnnychen94 left a comment •

edited

Loading

johnnychen94 left a comment •

edited

Loading

SomTambe commented Jun 17, 2021 •

edited

Loading