Work around #496 for Luv, Lab

[timholy]

This fixes the specific demo in #496. These constants were added in 8f6550d, but there is nothing wrong with that PR---those should be perfectly reasonable operations to perform during package definition.

The origin of the problem is a julia bug whose identity remains mysterious. Some diagnostics were done in JuliaLang/julia#35972, but this particular case appears to be distinct from the mechanism identified there and has not yet been isolated. This is a simple workaround.

[codecov]

Codecov Report

Merging #511 (d3c3065) into master (a515eab) will decrease coverage by 0.24%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #511      +/-   ##
==========================================
- Coverage   94.97%   94.72%   -0.25%     
==========================================
  Files           9        9              
  Lines        1193     1213      +20     
==========================================
+ Hits         1133     1149      +16     
- Misses         60       64       +4     

Impacted Files	Coverage Δ
src/algorithms.jl	75.00% <ø> (+0.24%)	⬆️
src/parse.jl	100.00% <0.00%> (ø)
src/display.jl	100.00% <0.00%> (ø)
src/precompile.jl	0.00% <0.00%> (ø)
src/utilities.jl	98.84% <0.00%> (+<0.01%)	⬆️
src/colormaps.jl	94.78% <0.00%> (+0.39%)	⬆️

---

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 a515eab...d3c3065. Read the comment docs.

[kimikage]

I am interested in how you found this workaround. Also, I would like to know the specific cases where this workaround is effective.
I might be able to suggest another workaround.

[timholy]

Ah, very simple actually. precompile(f, argtyps) simply forces f to be compiled for f(::argtyps...), nothing more. So does executing f(args::argtyps...). Consequently, after commenting out the explicit precompile directives in Colors, ColorTypes, and FixedPointNumbers failed to change anything, I went looking for other examples of statements that executed code while the package was being defined (i.e., before it gets cached to disk). Method definitions, of course, don't actually run any code unless they are being defined by metaprogramming.

And so anything that ran code related to colorspace conversion from toplevel was a candidate. Just searching for consts flagged them immediately. The workaround was not to use any code that performed colorspace conversion until after the package had been cached to disk.

[timholy]

Following up: precompile has nothing to do with caching to the *.ji. It just compiles code. This is almost the same as running code, except that precompile only works until it gets to a runtime dispatch barrier and then quits, whereas running code executes the runtime dispatch and then continues to compile as needed for the next steps of execution. Hence running code is often a more thorough version of precompile. This is part of why in my more recent precompilation work I've tended to just supply tiny workloads (e.g., https://github.com/JuliaImages/ImageCore.jl/blob/ba4abdd6c9d30ffd9360a4b0f9e22636bfb3cf2f/src/precompile.jl#L51-L108).

The caching is an independent step that caches all methods + their specializations defined in the package. Anything that gets compiled is a specialization, so precompile == running code for the purposes of caching.

But actually, the paragraph above this one makes me think: given all the inference failures in JuliaGraphics/ColorTypes.jl#266, would some of our weird issues be fixed by switching from precompile(Tuple{typeof(convert),Type{C{T}},XYZ{T}}) to something like convert(C{T}, zero(XYZ{T})) in https://github.com/JuliaGraphics/Colors.jl/blob/master/src/precompile.jl? It won't cache things in ColorTypes, unfortunately (not without JuliaLang/julia#42016). Worth testing, but I'm already running behind on some other deadlines and have to move on for the next couple of days.

[kimikage]

Thank you for the detailed explanation. It is very logical. However, I feel that the effect of this PR is somewhat less than the logical expectation.

Of course, since the rewriting of the core code of conversions is mostly complete, there is no major disadvantage to making the consts literals.

[timholy]

I'm in no hurry to merge this, if you want to investigate more. Just wanted to make sure you knew about the causes & potential workarounds.

[kimikage]

If we use BigFloat to calculate the constants, the execution may not harm the code for Float32/Float64. It's ugly, though.

[kimikage]

switching from precompile(Tuple{typeof(convert),Type{C{T}},XYZ{T}}) to something like convert(C{T}, zero(XYZ{T}))

It seems that a simple switching does not work well. 🤔 It might be worth looking into the differences with precompile a bit more.

[timholy]

It might be worth looking into the differences with precompile a bit more

The reason for the difference is known (precompile only works up until runtime-dispatch "barriers," whereas execution continues through them), but the consequences of the difference may be revealing. Given that we now know that convert has so many internal inference failures (JuliaGraphics/ColorTypes.jl#266, even though the final result is inferrable), this is interesting but not surprising. But it might indeed help further diagnose Julia's own issues here.

[kimikage]

For example, if we don't call cnvt(Luv{Float64}, ::XYZ{Float64}), the problem seems to be avoided. However, while that is superficially obvious, it is still essentially mysterious.

function _get_hue_in(::Type{C}, r, g, b) where C <: Union{Luv, Lab}
    xyz = linear_rgb_to_xyz(RGB{Float64}(r, g, b))
    wp = WP_DEFAULT
    if C === Luv
        c = xyz2luv(Luv{Float64}, xyz, xyz.y, xyz_to_uv(wp))
    else
        c = xyz2lab(Lab{Float64}, XYZ(xyz.x / wp.x, xyz.y, xyz.z / wp.z))
    end
    hue(c)
end

const LUV_HUE_R = _get_hue_in(Luv, 1, 0, 0)
...

Unlike Base.convert, ColorTypes.cconvert, etc., cnvt and xyz2luv are internal methods of Colors.
Also, broadcast (. operations) seems to have an indirect effect, but I haven't isolated the factors yet.

[kimikage]

I see some room for improvement in the implementation of Luv conversions, but the compilation-related mystery still deepens.

I will merge #509 after merging this.

[timholy]

compilation-related mystery still deepens

I'm not sure it's much of a mystery anymore, it's just a difficult fix.