JuliaGraphics / Colors.jl

Color manipulation utilities for Julia
Other
200 stars 44 forks source link

Mitigate inference problems in color conversions #508

Closed kimikage closed 2 years ago

kimikage commented 2 years ago

This uses the cnvt instead of the top-level convert when source and destination are both known colors. This also blocks the propagation of inference failures by adding type assertions to the result of convert. ~However, these changes do not solve the essential problem (#496). So, this PR is for reference only, and I have no plans to merge this for now.~ By PR #510, we can see the practical effects, e.g.:

julia> lchab = rand(LCHab, 1000, 1000);

julia> convert.(RGB{Float64}, lchab); # compilation

julia> @time convert.(RGB{Float64}, lchab);
  0.174340 seconds (4.00 M allocations: 144.959 MiB, 37.16% gc time) # before
  0.137165 seconds (2.00 M allocations: 83.923 MiB, 40.44% gc time)  # after

I haven't solved the problem yet, though.

codecov[bot] commented 2 years ago

Codecov Report

Merging #508 (11257ec) into master (9c55e4a) will decrease coverage by 0.48%. The diff coverage is 76.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #508      +/-   ##
==========================================
- Coverage   95.45%   94.97%   -0.49%     
==========================================
  Files           9        9              
  Lines        1188     1193       +5     
==========================================
- Hits         1134     1133       -1     
- Misses         54       60       +6     
Impacted Files Coverage Δ
src/precompile.jl 0.00% <0.00%> (ø)
src/utilities.jl 98.84% <75.00%> (+0.38%) :arrow_up:
src/conversions.jl 99.39% <100.00%> (+<0.01%) :arrow_up:
src/colormaps.jl 94.39% <0.00%> (-3.74%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9c55e4a...11257ec. Read the comment docs.

kimikage commented 2 years ago

Some problematic math functions have been removed from the major paths. The problems that still remain are trigonometric functions, especially sincos. Since sincos is not that bad in terms of speed and accuracy, there is not enough motivation to reimplement it.

Edit: See PR #510

kimikage commented 2 years ago

RGB <--> LCHab/LCHuv conversions still have problems. However, I would like to address some of the problems, in PR #509.