make optimize_ticks faster

[jkrumbiegel]

This still fails a couple of tests, although I'm not sure why 1000 or so tests don't fail and a random looking subset does. Anyway, the changes here make the function about 12 times faster for me, from about 4.2ms to 395µs. Maybe someone else has an idea how to fix the tests.

[asinghvi17]

This is an MWE:

function test_ticks(n, i)
           y0 = 10^n
           x0 = y0-1
           x, y = (x0,y0) .* 10.0^i
           ticks = optimize_ticks(x, y)[1]
       return ticks
end
test_ticks(1, -9)
#┌ Warning: No strict ticks found
#└ @ PlotUtils ~/.julia/dev/PlotUtils/src/ticks.jl:258
[0.0, 0.0, 0.0, 0.0, 0.0, 0.0]

and it seems that the corrections to the algorithm fail whenever i ≤ -5:

[test_ticks(1, i) for i in -9:9]
┌ Warning: No strict ticks found
└ @ PlotUtils ~/.julia/dev/PlotUtils/src/ticks.jl:258
┌ Warning: No strict ticks found
└ @ PlotUtils ~/.julia/dev/PlotUtils/src/ticks.jl:258
┌ Warning: No strict ticks found
└ @ PlotUtils ~/.julia/dev/PlotUtils/src/ticks.jl:258
┌ Warning: No strict ticks found
└ @ PlotUtils ~/.julia/dev/PlotUtils/src/ticks.jl:258
┌ Warning: No strict ticks found
└ @ PlotUtils ~/.julia/dev/PlotUtils/src/ticks.jl:258
19-element Array{Array{Float64,1},1}:
 [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]
 [0.0, 0.0, 0.0]
 [0.0, 0.0, 0.0]
 [0.0, 0.0, 0.0, 0.0, 0.0, 0.0]
 [0.0, 0.0]
 [0.001, 0.001, 0.001]
 [0.01, 0.01, 0.01]
 [0.09, 0.1, 0.1]
 [0.9, 0.95, 1.0]
 [9.0, 9.5, 10.0]
 [90.0, 95.0, 100.0]
 [900.0, 950.0, 1000.0]
 [9000.0, 9500.0, 10000.0]
 [90000.0, 95000.0, 100000.0]
 [900000.0, 950000.0, 1.0e6]
 [9.0e6, 9.5e6, 1.0e7]
 [9.0e7, 9.5e7, 1.0e8]
 [9.0e8, 9.5e8, 1.0e9]
 [9.0e9, 9.5e9, 1.0e10]

[asinghvi17]

This also gives less tight ticks, probably because of the rounding approximation.

Current master:

This PR:

[daschw]

By the way, I was thinking if we should remove the last item (3.0, 0.2) of Q in the default arguments optimize_ticks. I'm unsure if a step length of a multiple of 3 like in the second plot above ever is the best option.

[jkrumbiegel]

so I did some wizardry with the filtering operation and reduced the number of rounding calls to only those that seemed necessary. With all improvements, benchmarking shows this result for the current version:

399.803 μs (2029 allocations: 97.80 KiB)

versus this for the original version on master

4.624 ms (159628 allocations: 3.17 MiB)

[mkborregaard]

Awesome 💖 Ready to merge?
Oh, and would you mind doing this for the rest of the code in Plots and PlotUtils while you're at it?

[jkrumbiegel]

Ready to merge?

I would think so, although I haven't written the tests so I don't know if they leave room for unnoticed errors.

[asinghvi17]

Let me do some regression tests with MakieGallery, might take an hour or so but if those work I see no reason not to merge this!

[asinghvi17]

@jkrumbiegel Makie and Plots tests pass...looks good to merge!

[daschw]

I'm afraid I see a regression with this in the Plots tests.

Compare the yticks in the first and the third subplot of the second row.

[yha]

I think I fixed this regression in #89.
But I'm wondering if it's OK to skip rounding except at the endpoints. IIRC, rounding was added because the tick computation sometimes produced non-round result due to FP error - not necessarily at the endpoints.

[jkrumbiegel]

I think I fixed this regression in #89.
But I'm wondering if it's OK to skip rounding except at the endpoints. IIRC, rounding was added because the tick computation sometimes produced non-round result due to FP error - not necessarily at the endpoints.

I skipped rounding except at the edges at first because only the edges are looked at in the following code in all cases. The full vector is only used if the values are better candidates than the current ones. I struggled a bit with making the tests work, and when they worked I thought everything was fine. But there is still the option to add rounding back in for the non-edge numbers at the end, that would still be better than doing all numbers always (and the rounding appeared to cause quite a bit of slowdown)