Simplify control logic for :logbins (#257)

There wasn't an actual bug, but the logic was so tortured that when I
looked back at it, I thought there was. This should be easier on the eyes.

Author: timholy

src/trials.jl

@@ -446,9 +446,9 @@ function Base.show(io::IO, ::MIME"text/plain", t::Trial)
     bins = bindata(histtimes, histwidth - 1, histmin, histmax)
     append!(bins, [1, floor((1-histquantile) * length(times))])
     # if median size of (bins with >10% average data/bin) is less than 5% of max bin size, log the bin sizes
-    if (logbins === nothing || logbins === true) && median(filter(b -> b > 0.1 * length(times) / histwidth, bins)) / maximum(bins) < 0.05
+    if logbins === true || (logbins === nothing && median(filter(b -> b > 0.1 * length(times) / histwidth, bins)) / maximum(bins) < 0.05)
         bins, logbins = log.(1 .+ bins), true
-    elseif logbins === nothing
+    else
         logbins = false
     end
     hist = asciihist(bins, histheight)

test/ExecutionTests.jl

@@ -135,15 +135,33 @@ tune!(b)
 # test kwargs separated by `,`
 @benchmark(output=sin(x), setup=(x=1.0; output=0.0), teardown=(@test output == sin(x)))
 
-io = IOBuffer()
-ioctx = IOContext(io, :histmin=>0.5, :histmax=>8, :logbins=>false)
-b = @benchmark x^3   setup=(x = rand()); show(ioctx, MIME("text/plain"), b)
-b = @benchmark x^3.0 setup=(x = rand()); show(ioctx, MIME("text/plain"), b)
-str = String(take!(io))
-idx = findfirst(r"0.5 ns +Histogram: frequency by time +8 ns", str)
-@test isa(idx, UnitRange)
-idx = findnext( r"0.5 ns +Histogram: frequency by time +8 ns", str, idx[end]+1)
-@test isa(idx, UnitRange)
+for (tf, rex1, rex2) in ((false, r"0.5 ns +Histogram: frequency by time +8 ns",        r"Histogram: frequency"),
+                         (true,  r"0.5 ns +Histogram: log\(frequency\) by time +8 ns", r"Histogram: log\(frequency\)"))
+    io = IOBuffer()
+    ioctx = IOContext(io, :histmin=>0.5, :histmax=>8, :logbins=>tf)
+    @show tf
+    b = @benchmark x^3   setup=(x = rand()); show(ioctx, MIME("text/plain"), b)
+    b = @benchmark x^3.0 setup=(x = rand()); show(ioctx, MIME("text/plain"), b)
+    str = String(take!(io))
+    idx = findfirst(rex1, str)
+    @test isa(idx, UnitRange)
+    idx = findnext( rex1, str, idx[end]+1)
+    @test isa(idx, UnitRange)
+    ioctx = IOContext(io, :logbins=>tf)
+    # A flat distribution won't trigger log by default
+    b = BenchmarkTools.Trial(BenchmarkTools.DEFAULT_PARAMETERS, 0.001 * (1:100) * 1e9, zeros(100), 0, 0)
+    show(ioctx, MIME("text/plain"), b)
+    str = String(take!(io))
+    idx = findfirst(rex2, str)
+    @test isa(idx, UnitRange)
+    # A peaked distribution will trigger log by default
+    t = [fill(1, 21); 2]
+    b = BenchmarkTools.Trial(BenchmarkTools.DEFAULT_PARAMETERS, t/sum(t)*1e9*BenchmarkTools.DEFAULT_PARAMETERS.seconds, zeros(100), 0, 0)
+    show(ioctx, MIME("text/plain"), b)
+    str = String(take!(io))
+    idx = findfirst(rex2, str)
+    @test isa(idx, UnitRange)
+end
 
 #############
 # @bprofile #