Implement abs2

[timholy]

abs2 was removed in #131 out of concerns about mathematical consistency. This brings it back in a way that's now consistent, but I think with different numeric values than previously. Even though ColorVectorSpace 0.9 has not yet bubbled up to the top of the JuliaImages stack, it may be time to consider restoring it.

Fixes #157.

[kimikage]

I cannot decide whether this definition of abs2 is a good one or not.
However, I think you need to remove the error hint for abs2 before merging this PR.

[johnnychen94]

The unit test is disabled after 60 days of inactivity. I just updated the CI configuration in #173, and I'm going to close&reopen to trigger the test.

[codecov]

Codecov Report

Merging #172 (46cb733) into master (ab3dab2) will decrease coverage by 0.06%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master     #172      +/-   ##
==========================================
- Coverage   85.99%   85.93%   -0.07%     
==========================================
  Files           2        2              
  Lines         257      263       +6     
==========================================
+ Hits          221      226       +5     
- Misses         36       37       +1     

Impacted Files	Coverage Δ
src/ColorVectorSpace.jl	90.40% <83.33%> (-0.18%)	⬇️

---

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 ab3dab2...46cb733. Read the comment docs.

[johnnychen94]

JuliaImages/ImageSegmentation.jl#72 (comment)
Hmm, it occurs to me that the division by 3 is effectively breaking for older scripts. The transition to ImageCore 0.9 (and specifically, the new consistency in abs2) has more implications than I expected! Should the next release be a 2.0 release? In which case we want to wait until we make more breaking API changes.

To avoid confusion, this PR should be released under another breaking release v0.10 or v1.0; we need a long window to allow people update their codes.

[Octogonapus]

Thanks for the implementation, @timholy. I want to express my support for this implementation; it works for our purposes as well as the prior implementation did.

[timholy]

Great! It should be far better in the following sense:

julia> using ColorTypes, ColorVectorSpace

julia> g = Gray(0.8)
Gray{Float64}(0.8)

julia> rgb = convert(RGB, g)
RGB{Float64}(0.8,0.8,0.8)

julia> abs2(g) ≈ abs2(rgb)
true

There was a 3x difference between these in the 0.8 series.

[timholy]

To avoid confusion, this PR should be released under another breaking release v0.10 or v1.0; we need a long window to allow people update their codes.

I've thought about this a bit; I'm not at all opposed to making this part of v0.10 or v1.0; however, there's a sense where much of the change will have to come after we release this. The reason? In JuliaImages/ImageSegmentation.jl#72 (comment), how do we go about deprecating this? I think we have to have a warning on every invocation that uses the fallback default_diff_fn. We can have the warning explain they can circumvent the warning by passing in their own metric, and at that time telling them to adjust for the new values. That metric will probably be built from abs2, while the depwarn can use the existing https://github.com/JuliaImages/ImageSegmentation.jl/blob/cf47b5a802443e574557983debce3dd152da1282/src/core.jl#L12. So, we can even begin to fix this until the new abs2 is merged.

[johnnychen94]

In JuliaImages/ImageSegmentation.jl#72 (comment), how do we go about deprecating this?

Why do we need to deprecate it? For compatibility with old versions and other frameworks, we still want to have the inconsistent _abs2(c) = mapreducec(abs2, +, 0, c) implementation. The implementation is itself valid so unless people want to upgrade/change the implementations, they don't need to worry about the deprecations. I reckon it is sufficient to add a note on the CVS readme on the possible divided-by-3 confusion.

[timholy]

You were concerned that people would need a long time to adjust in #172 (comment). My point is that they cannot even begin adjusting until this gets merged.

Because I know I wasn't very clear, let me try again:

• We preserve the old default default_diff_fn, together with its 3x difference between Gray & RGB, but for anyone calling it with an RGB we issue a warning (always-visible, not just with --depwarn=yes) and tell them to circumvent it by passing in their own diff_fn based on abs2 with the 3x correction (i.e., the "new" abs2, meaning, this is the point at which they change their numeric thresholds). This phase is nonbreaking but noisy.
• After a suitable deprecation period, we change default_diff_fn to use the new abs2. No depwarn, of course. This phase is breaking.

[johnnychen94]

More eagerly advising users to change the codes sounds good to me. I've seen many hard landings to CVS v0.9 (e.g., the most recent one is JuliaPlots/VisualRegressionTests.jl#34), if we had ever had these messages it would be easier to do the transition.

[timholy]

OK, here's a pretty different approach that I think solves all problems:

tim@diva:~/.julia/dev/ColorVectorSpace$ julia -q --project --depwarn=yes
julia> using ColorTypes, ColorVectorSpace

julia> g = Gray(0.8)
Gray{Float64}(0.8)

julia> abs2(g)
0.6400000000000001

julia> abs2(RGB(g))
┌ Warning: The return value of `abs2` will change to ensure that `abs2(g::Gray) ≈ abs2(RGB(g::Gray))`.
│ For `RGB` colors, this results in dividing the previous output by 3.
│ 
│ To avoid this warning, use `ColorVectorSpace.Future.abs2` instead of `abs2`; currently,
│ `abs2` returns the old value (for compatibility), and `ColorVectorSpace.Future.abs2` returns the new value.
│ When making this change, you may also need to adjust constants like color-difference thresholds
│ to compensate for the change in the returned value.
│ 
│ If you are getting this from `var`, use `varmult` instead.
│   caller = top-level scope at REPL[4]:1
└ @ Core REPL[4]:1
1.9200000000000004

julia> ColorVectorSpace.Future.abs2(RGB(g))
0.6400000000000001

help?> ColorVectorSpace.Future.abs2
  ColorVectorSpace.Future.abs2(c)

  Return a scalar "squared magnitude" for color types. For RGB and gray, this is just the mean-square channelwise intensity.

  Compatibility note: this gives a different result from Base.abs2(c), but eventually Base.abs2 will switch to the definition used here. Using ColorVectorSpace.Future.abs2 thus future-proofs your code.
  For more information about the transition, see ColorVectorSpace's README.

The proposed transition schedule is in the README; for convenience, it's

• Sept 20, 2021: release ColorVectorSpace 0.9.7 with both abs2 and Future.abs2. abs2 will have a "quiet" deprecation warning (visible with --depwarn=yes or when running Pkg.test)
• Jan 1, 2022: make the deprecation warning "noisy" (cannot be turned off). This is designed to catch user-level scripts that may need to update thresholds or other constants.
• *Apr 1, 2022: transition abs2 to the new definition and make Future.abs2 give a "noisy" depwarn to revert to regular abs2.
• *July 1, 2022: remove Future.abs2.

The two marked with * denote breaking releases.

Personally, this feels like the right way to do it. I am kicking myself that I didn't think of this from the beginning. Now that we've had https://github.com/JuliaGraphics/ColorVectorSpace.jl#abs-and-abs2 out there for a while, we've "decoupled" users' usage of abs2 and lost control of the ability to guide people to the consistent way of doing things. But since the next release of Images isn't out yet, it's not too late for what I'm guessing might be the majority of our users.

[timholy]

Barring concerns, I'm merging this today and tagging a new release. However, I'm happy to make changes (e.g., if people think we need an even longer deprecation period, I'm fine with that, although it will delay the 1.0 release).