Update to ImageCore 0.9

[timholy]

This drops Images entirely, but doesn't pass tests. There are three issues, all in the watershed test:

• the use of magnitude_phase, which AFAICT is mostly to generate a box. I've prepared a substitute.
• perhaps as a consequence, the result of a test involving compact watersheds changed by a count of two pixels: now the first region is literally the upper-left triangle. Is this a bad change, @tlnagy?
• TODO: findlocalminima is still needed from Images. I'm tempted to move nearly everything out of Images and make it a pure meta-package. If people like that, I can think of two places, ImageBase and ImageFiltering. Thoughts?

closes #71

[johnnychen94]

My first impression of findlocalextrema is that it can be implemented quite easily with mapwindow so living in ImageFiltering.jl sounds a natural choice to me.

# a slightly different implementation
function my_findlocalmaxima(X)
    mask = mapwindow(CartesianIndices(X), (3, 3)) do Rp
        p = X[Rp[OffsetArrays.center(Rp)...]]
        all(q->p >= q, view(X, Rp))
    end
    out = CartesianIndices(X)[mask]
end

[timholy]

Move in progress. I do think we want to keep these since they have some advantages:

julia> img = rand(100,100);

julia> @btime findlocalmaxima($img);
  378.800 μs (10013 allocations: 377.05 KiB)

julia> @btime my_findlocalmaxima($img);
  3.532 ms (76365 allocations: 2.39 MiB)

(EDIT: and fixing an inferrability problem revealed by this demo reduces the time by more than another factor of 2.)

Aside from speed, findlocalmaxima works on images that are too big to store in memory all at once, but that would be a lot harder to achieve with the mapwindow solution.

What to do about imROF? Is that an ImageFiltering.jl operation or an ImageNoise.jl operation?

[johnnychen94]

What to do about imROF? Is that an ImageFiltering.jl operation or an ImageNoise.jl operation?

I'll make a new implementation in ImageNoise. JuliaImages/Images.jl#755 years later I think I'm ready to make a fastest implementation :)

[timholy]

Here's a question: the package only requires ImageFiltering 0.6 or higher, but the tests require ImageFiltering 0.7. How should we handle the [compat]?

[johnnychen94]

Pkg resolver will automatically use the latest version, right? Or are you worried about the tests on ImageFiltering v0.6? I think this is an issue of all packages; IIUC Julia doesn't have a test tool to run thorough compatibility tests.

[timholy]

Right, as long as there's no blocker then it will use the latest and pass the tests, which is why I left the [compat] at the package requirement rather than the test requirement.

Should I add a comment at the top of the watershed test file? Or are we good with interpreting the problem and should just keep as-is?

[johnnychen94]

Should I add a comment at the top of the watershed test file? Or are we good with interpreting the problem and should just keep as-is?

Instead of leaving a comment, we should mark them as conditional test groups using if branch. That if we ever have the chance to bring a comprehensive compatibility test, we don't see a failure there.

But this isn't something important right now IMHO

[timholy]

Sounds good. Ready to merge and tag a new release?

[johnnychen94]

It's your call 😄

[timholy]

😆 I will go through the examples in the docs and make sure they all still work...

[timholy]

Yep! Small differences for felzenswalb and perhaps others (mostly likely because of the division by 3 in _abs2), so we may have to adjust a few constants, but other than that this looks good. I may wait to update the docs until we have a compatible version of Images, since the examples still load Images.

[timholy]

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.

[johnnychen94]

Should the next release be a 2.0 release? In which case we want to wait until we make more breaking API changes.

This depends on how quickly we want to bump the entire JuliaImages to ImageCore v0.9. If you can wait for more time until other PRs are done then it's totally okay to me. Another solution is to still use the mapreducec(v->float(v)^2, +, 0, c) definition, which unblocks ImageSegmentation release pipeline.

[johnnychen94]

Maybe stick to the old usage for now?

Suggested change

	_abs2(c::MathTypes) = c ⋅ c
	_abs2(c::MathTypes) = mapreducec(v->float(v)^2, +, 0, c)

[timholy]

That sounds like a good idea.

[timholy]

That seems to fix things, by and large. The few differences:

• the horse demo with felzenszwalb(img, 100) merges grass and sky, resulting in a dull gray used for both. That's true here (with your suggested change) and on the released version of the package. Both also give 43 segments, making me think that this probably changed some time ago but the image in the docs hasn't been updated. felzenszwalb(img, 10) produces the result shown in the image.
• There's now an off-by-one in the number of segments in https://juliaimages.org/stable/pkgs/segmentation/#Fast-Scanning (2539 instead of 2538, and 64 instead of 65). There is no visible change in the quality of the result, so to me that seems acceptable.
• New Julia releases change the order of random numbers for a given seed, and hence the coloration of images for the demos using get_random_color.