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

Mitigate inference problems in color conversions #508

Merged
merged 1 commit into from
Aug 15, 2021

Conversation

kimikage
Copy link
Collaborator

@kimikage kimikage commented Aug 2, 2021

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
Copy link

codecov bot commented Aug 2, 2021

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%) ⬆️
src/conversions.jl 99.39% <100.00%> (+<0.01%) ⬆️
src/colormaps.jl 94.39% <0.00%> (-3.74%) ⬇️

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

This comment has been minimized.

@kimikage kimikage force-pushed the cnvt_inference branch 3 times, most recently from 9956d2b to ba5dcc4 Compare August 11, 2021 12:15
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`.
@kimikage kimikage marked this pull request as ready for review August 13, 2021 12:45
@kimikage kimikage changed the title [RFC/WIP] Mitigate inference problems in color conversions Mitigate inference problems in color conversions Aug 13, 2021
@kimikage
Copy link
Collaborator Author

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

@kimikage kimikage merged commit a515eab into JuliaGraphics:master Aug 15, 2021
@kimikage kimikage deleted the cnvt_inference branch August 15, 2021 00:38
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.

1 participant