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

Improve conversions and support ImageCore #4

Merged
merged 12 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
fail-fast: false
matrix:
version:
- '1.6'
- '1.7'
- '1'
- 'nightly'
os:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
*.jl.cov
*.jl.mem
/Manifest.toml
/docs/Manifest.toml
/docs/build/
9 changes: 4 additions & 5 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,23 @@ version = "0.1.0"
ColorTypes = "3da002f7-5984-5a60-b8a6-cbb66c0b333f"
ColorVectorSpace = "c3611d14-8923-5661-9e6a-0046d554d3a4"
Colors = "5ae59095-9a9b-59fe-a467-6f913c188581"
Compat = "34da2185-b29b-5c13-b0c7-acf172513d20"
FixedPointNumbers = "53c48c17-4a7d-5ca2-90c5-79b7896eea93"
Reexport = "189a3867-3050-52da-a836-e630ba90ab69"
Requires = "ae029012-a4dd-5104-9daa-d747884805df"

[compat]
ColorTypes = "0.11"
ColorTypes = "0.11.1"
ColorVectorSpace = "0.9"
Colors = "0.12"
Compat = "3.36"
FixedPointNumbers = "0.8"
Reexport = "1"
Requires = "1"
julia = "1.6"
julia = "1.7"

[extras]
ImageCore = "a09fc81d-aa75-5fe9-8630-4744c3626534"
StructArrays = "09ab397b-f2b6-538f-b94a-2f83cf4a842a"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["StructArrays", "Test"]
test = ["ImageCore", "StructArrays", "Test"]
32 changes: 32 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,30 @@ RGB{N0f16}(0.75,0.87549,0.09117)

The latter is how this color would be rendered in a viewer; embedded in a function, the conversion is extremely well optimized (~2.2ns on the author's machine).

## Overflow protection

Depending on the colors you pick for conversion to RGB (e.g., `channels`), it is possible to exceed the 0-to-1 bounds of RGB.
With the choice above,

```julia
julia> c = ctemplate(0.99, 0.99)
(0.99001N0f16₁, 0.99001N0f16₂)

julia> convert(RGB, c)
ERROR: ArgumentError: component type N0f16 is a 16-bit type representing 65536 values from 0.0 to 1.0,
but the values (0.9900053f0, 1.7664759f0, 0.36105898f0) do not lie within this range.
See the READMEs for FixedPointNumbers and ColorTypes for more information.
Stacktrace:
[...]
```

If you want to guard against such errors, one good choice would be

```julia
julia> convert(RGB{Float32}, c)
RGB{Float32}(0.9900053, 1.7664759, 0.36105898)
```

## Advanced usage

`ctemplate` stores the RGB *values* for each fluorophore as a type-parameter. This allows efficient conversion to RGB
Expand All @@ -49,3 +73,11 @@ f(i1, i2) = ColorMixture{N0f8}((fluorophore_rgb"EGFP", fluorophore_rgb"tdTomato"
Note the absence of `[]` brackets around the fluorophore names. For such constructors, `N0f8` is the only option if you're
looking up the RGB values with `fluorophore_rgb`; however, if you hard-code the RGB values there is no restriction
on the element type.

## Why are the RGB colors encoded in the *type*? Why not a value field?

In many places, JuliaImages assumes that you can convert from one color space to another purely from knowing the type you want to convert to. This would not be possible if the RGB colors were encoded as a second field of the color.

## Why does this package require a minimum of Julia 1.7?

To achieve good performance, the RGB values must be aggressively constant-propagated, a feature available only on Julia 1.7 and higher.
johnnychen94 marked this conversation as resolved.
Show resolved Hide resolved
97 changes: 0 additions & 97 deletions docs/Manifest.toml

This file was deleted.

3 changes: 3 additions & 0 deletions docs/Project.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
[deps]
Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
FluorophoreColors = "d4071afc-4203-49ee-90bc-13ebeb18d604"

[compat]
Documenter = "0.27"
Copy link
Member

Choose a reason for hiding this comment

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

If we don't need fancy features, then it's perhaps a good idea to just drop the compat. I'm okay either way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mostly it's to avoid breakage if Documenter 0.28 or higher breaks some things we use, but that hasn't happened in quite some time so...:shrug:

I'll change it.

3 changes: 1 addition & 2 deletions src/FluorophoreColors.jl
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
module FluorophoreColors

using Compat # for Compat.@constprop

using Reexport
@reexport using FixedPointNumbers
@reexport using ColorTypes
Expand All @@ -17,6 +15,7 @@ include("utils.jl")

function __init__()
@require StructArrays = "09ab397b-f2b6-538f-b94a-2f83cf4a842a" include("structarrays.jl")
@require ImageCore = "a09fc81d-aa75-5fe9-8630-4744c3626534" include("imagecore.jl")
end

end
3 changes: 3 additions & 0 deletions src/imagecore.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# clamp01 is designed to work on RGB colors (a major usage is for display), so convert first
ImageCore.clamp01(c::ColorMixture{T}) where T = convert(RGB{T}, ImageCore.clamp01(convert(RGB{floattype(T)}, c)))
ImageCore.clamp01nan(c::ColorMixture{T}) where T = convert(RGB{T}, ImageCore.clamp01nan(convert(RGB{floattype(T)}, c)))
26 changes: 18 additions & 8 deletions src/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ julia> c = ColorMixture{N0f16}(channelcolors, #= GFP intensity =# 0.2, #= tdToma
(0.2N0f16₁, 0.85N0f16₂)

julia> convert(RGB, c)
RGB{N0f16}(0.85,0.9151,0.07294)
RGB{N0f16}(0.85, 0.9151, 0.07294)
```

If you must construct colors inferrably inside a function body, use
Expand All @@ -50,15 +50,15 @@ and you must preserve the `N0f8` element type of fluorophore_rgb"NAME".
struct ColorMixture{T,N,Cs} <: Color{T,N}
channels::NTuple{N,T}

Compat.@constprop :aggressive function ColorMixture{T,N,Cs}(channels::NTuple{N}) where {T,N,Cs}
Base.@constprop :aggressive function ColorMixture{T,N,Cs}(channels::NTuple{N}) where {T,N,Cs}
Cs isa NTuple{N,RGB{T}} || throw(TypeError(:ColorMixture, "incompatible color types", NTuple{N,RGB{T}}, typeof(Cs)))
return new{T,N,Cs}(channels)
end
end
ColorMixture{T,N,Cs}(channels::Vararg{Real,N}) where {T,N,Cs} = ColorMixture{T,N,Cs}(channels)
Compat.@constprop :aggressive ColorMixture{T}(Cs::NTuple{N,RGB{T}}, channels::NTuple{N,Real}) where {T,N} = ColorMixture{T,N,Cs}(channels)
ColorMixture{T}(Cs::NTuple{N,AbstractRGB}, channels::NTuple{N,Real}) where {T,N} = ColorMixture{T,N,RGB{T}.(Cs)}(channels)
ColorMixture{T}(Cs::NTuple{N,AbstractRGB}, channels::Vararg{Real,N}) where {T,N} = ColorMixture{T}(Cs, channels)
Base.@constprop :aggressive ColorMixture{T}(Cs::NTuple{N,RGB{T}}, channels::NTuple{N,Real}) where {T,N} = ColorMixture{T,N,Cs}(channels)
Base.@constprop :aggressive ColorMixture{T}(Cs::NTuple{N,AbstractRGB}, channels::NTuple{N,Real}) where {T,N} = ColorMixture{T,N,RGB{T}.(Cs)}(channels)
Base.@constprop :aggressive ColorMixture{T}(Cs::NTuple{N,AbstractRGB}, channels::Vararg{Real,N}) where {T,N} = ColorMixture{T}(Cs, channels)

@inline _promote_typeof(::Type{C1}, ::Type{C2}) where {C1,C2} = promote_type(C1, C2)
@inline _promote_typeof(::Type{C1}, ::Type{C2}, obj, objs...) where {C1,C2} =
Expand All @@ -69,8 +69,8 @@ ColorMixture{T}(Cs::NTuple{N,AbstractRGB}, channels::Vararg{Real,N}) where {T,N}
@inline promote_typeof(obj1, obj2, objs...) = _promote_type(typeof(obj1), typeof(obj2), objs...)

computeT(Cs::NTuple{N,AbstractRGB}, channels::NTuple{N,Real}) where {N} = eltype(promote_typeof(map(*, Cs, channels)...))
ColorMixture(Cs::NTuple{N,AbstractRGB}, channels::NTuple{N,Real}) where {N} = ColorMixture{computeT(Cs, channels)}(Cs, channels)
ColorMixture(Cs::NTuple{N,AbstractRGB}, channels::Vararg{Real,N}) where {N} = ColorMixture{computeT(Cs, channels)}(Cs, channels)
Base.@constprop :aggressive ColorMixture(Cs::NTuple{N,AbstractRGB}, channels::NTuple{N,Real}) where {N} = ColorMixture{computeT(Cs, channels)}(Cs, channels)
Copy link
Member

Choose a reason for hiding this comment

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

Is this the optimization that reduces the generalization overhead you found in #2 (comment)?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, in this case the performance hit was about 100x. But that penalty was clearly dependent on specifics of the implementation since as stated in the previous comment it seems to have gone away.

The issue basically is that the RGB values are part of the type, and if we change the values in some way (e.g., RGB{N0f8} -> RGB{Float32}) then unless the values are aggressively const-propped then inference may not be able to calculate them. But now all operations return an element of the same type as the original, by default, so that seems to fix things.

Copy link
Member Author

@timholy timholy May 10, 2022

Choose a reason for hiding this comment

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

In other words, sometimes inference needs to be able to anticipate this change:

julia> C1 = typeof(ColorMixture(channels))
ColorMixture{N0f8, 2, (RGB{N0f8}(0.0, 0.925, 0.365), RGB{N0f8}(1.0, 0.859, 0.0))}

julia> C2 = typeof(ColorMixture(float.(channels)))
ColorMixture{Float32, 2, (RGB{Float32}(0.0, 0.9254902, 0.3647059), RGB{Float32}(1.0, 0.85882354, 0.0))}

and inference is not capable of that on 1.6. I added this point to the README.

Copy link
Member Author

Choose a reason for hiding this comment

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

And the bigger story is that I'm now supporting 1.6. I think the caution is still important, though.

Base.@constprop :aggressive ColorMixture(Cs::NTuple{N,AbstractRGB}, channels::Vararg{Real,N}) where {N} = ColorMixture{computeT(Cs, channels)}(Cs, channels)

"""
cobj = ColorMixture((rgb₁, rgb₂)) # create an all-zeros ColorMixture
Expand Down Expand Up @@ -103,7 +103,17 @@ function Base.show(io::IO, c::ColorMixture)
print(io, ')')
end

# These definitions use floats to avoid overflow
function Base.convert(::Type{RGB{T}}, c::ColorMixture{T,N,Cs}) where {T,N,Cs}
sum(map(*, c.channels, Cs))
convert(RGB{T}, sum(map(*, c.channels, Cs); init=zero(RGB{floattype(T)})))
end
function Base.convert(::Type{RGB{T}}, c::ColorMixture{R,N,Cs}) where {T,R,N,Cs}
convert(RGB{T}, sum(map((w, rgb) -> convert(RGB{floattype(T)}, w*rgb), c.channels, Cs)))
end
Base.convert(::Type{RGB}, c::ColorMixture{T}) where T = convert(RGB{T}, c)
Base.convert(::Type{RGB24}, c::ColorMixture) = convert(RGB24, convert(RGB, c))

ColorTypes._comp(::Val{N}, c::ColorMixture) where N = c.channels[N]
Base.@constprop :aggressive ColorTypes.mapc(f, c::ColorMixture{T,N,Cs}) where {T,N,Cs} = ColorMixture(Cs, map(f, c.channels))
Base.@constprop :aggressive ColorTypes.mapreducec(f, op, v0, c::ColorMixture{T,N,Cs}) where {T,N,Cs} = mapreduce(f, op, v0, c.channels)
Base.@constprop :aggressive ColorTypes.reducec(op, v0, c::ColorMixture{T,N,Cs}) where {T,N,Cs} = reduce(op, c.channels; init=v0)
38 changes: 38 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ using Test

# interacts via @require
using StructArrays
using ImageCore

@testset "FluorophoreColors.jl" begin
@test isempty(detect_ambiguities(FluorophoreColors))
Expand Down Expand Up @@ -30,6 +31,20 @@ using StructArrays
@test c.channels[1] == c.channels[2] == 0.5
@test convert(RGB, c) ≈ 0.5*channels[1] + 0.5*channels[2]

# Overflow behavior
ctemplate = ColorMixture{N0f8}((RGB(1, 0, 0), RGB(0.5, 0.5, 0)))
c = ctemplate(0.8, 0.8)
if Base.VERSION >= v"1.8.0-DEV.363"
@test_throws "the values (1.2f0, 0.4f0, 0.0f0) do not lie within this range" convert(RGB{N0f8}, c)
@test_throws "the values (1.2f0, 0.4f0, 0.0f0) do not lie within this range" convert(RGB24, c)
@test_throws "the values (1.2f0, 0.4f0, 0.0f0) do not lie within this range" convert(RGB, c)
else
@test_throws ArgumentError convert(RGB{N0f8}, c)
@test_throws ArgumentError convert(RGB24, c)
@test_throws ArgumentError convert(RGB, c)
end
@test convert(RGB{Float32}, c) === RGB{Float32}(1.2, 0.4, 0)

# Macro syntax & inferrability
f_infer(i1, i2) = ColorMixture{N0f8}((fluorophore_rgb"EGFP", fluorophore_rgb"tdTomato"), (i1, i2))
f_noinfer(i1, i2) = ColorMixture((fluorophore_rgb["EGFP"], fluorophore_rgb["tdTomato"]), (i1, i2))
Expand All @@ -47,6 +62,15 @@ using StructArrays
@test @inferred(ctmpl(1, 0)) === ColorMixture{N0f8}(channels, (1, 0))
end

@testset "mapc etc" begin
channels = (fluorophore_rgb["EGFP"], fluorophore_rgb["tdTomato"])
ctemplate = ColorMixture{N0f8}(channels)
c = ctemplate(0.4, 0.2)
@test @inferred(mapc(x->2x, c)) === ColorMixture{Float32}(channels, (0.8, 0.4))
@test @inferred(mapreducec(x->2x, +, c)) === 1.2f0
@test @inferred(reducec(+, c)) === reduce(+, (0.4N0f8, 0.2N0f8))
end

@testset "StructArrays" begin
channels = (fluorophore_rgb["EGFP"], fluorophore_rgb["tdTomato"])
ctemplate = ColorMixture(channels)
Expand All @@ -72,6 +96,20 @@ using StructArrays
@test soa[1] == ctemplate(ntuple(i->(i-1)/32, 16))
end

@testset "ImageCore" begin
# Integration with the rest of the JuliaImages ecosystem
ctemplate = ColorMixture{N0f8}((RGB(1, 0, 0), RGB(0.5, 0.5, 0)))
c = ctemplate(0.8, 0.8)
@test convert(RGB{Float32}, c) === RGB{Float32}(1.2, 0.4, 0)
@test clamp01(c) === RGB{N0f8}(1, 0.4, 0)
@test clamp01nan(c) === RGB{N0f8}(1, 0.4, 0)
ctemplate = ColorMixture((RGB{Float32}(1, 0, NaN), RGB{Float32}(0.5, 0.5, NaN)))
c = ctemplate(0.8, 0.8)
@test isequal(convert(RGB, c), RGB{Float32}(1.2,0.4,NaN))
@test isequal(clamp01(c), RGB{Float32}(1.0,0.4,NaN))
@test clamp01nan(c) === RGB{Float32}(1, 0.4, 0)
end

@testset "IO" begin
channels = (fluorophore_rgb["EGFP"], fluorophore_rgb["tdTomato"])
c = ColorMixture(channels, (1, 0))
Expand Down