Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add plot capability #194

Merged
merged 2 commits into from
Jul 5, 2021
Merged

Add plot capability #194

merged 2 commits into from
Jul 5, 2021

Conversation

gustaphe
Copy link
Contributor

Adds a recipe to visualize the timings of a Trial or a BenchmarkGroup of Trials using boxplots. Only pulls in RecipesBase.jl so should be fairly low impact (but I don't know how best to check that).

using BenchmarkTools, Plots, StatsPlots
...
result = run(benchmarkgroup)
plot(result, outliers=false)

benchmarks

@vchuravy
Copy link
Member

Thank you for your PR! I am hesitant to take on any extra dependencies for BenchmarkTools, right now we only depend on JSON.jl

But I agree with you that better visualization would be really nice. I have hacked up to many ad-hoc plots before :) Maybe a companion package?

Lastly, I am not sure if a boxplot is the best visualization strategy since benchmark results are often not uni-modal. So if you get a multi-modal distribution that wouldn't show up in a boxplot.

@gustaphe
Copy link
Contributor Author

I don't think I know of any universally accepted multi-modal visualization, but you can

plot(result; st=:violin)

and it'll do the right thing. Maybe that should be the default, I haven't run so advanced benchmarks that I've needed more than a quick glance.

Would anyone go through the trouble of finding and loading BenchmarkToolPlots? It feels like the kind of functionality that you check if it just works, and when you find it doesn't you hack something together. I could be wrong.

@vchuravy
Copy link
Member

Ah violin plot might work quite nicely.

We could emphasize the visualization packages in the readme and have the examples use it!
(It could also live in this repo, but as a sub-package). Keeping the dependency list so small is an intentional feature for BenchmarkTools since it is generally at the bottom of the dependency chain.

@gustaphe
Copy link
Contributor Author

Sure, I can draft up a pr like that one of these days.

@gustaphe
Copy link
Contributor Author

This is difficult to test, I haven't done any subdir-package stuff before, but there's a process:

https://github.com/JuliaRegistries/Registrator.jl#registering-a-package-in-a-subdirectory

@gustaphe
Copy link
Contributor Author

Now one can choose the order of the trials:

using BenchmarkTools, BenchmarkPlots, StatsPlots
group = BenchmarkGroup()
group[:apple] = @benchmarkable sort!(a) setup=(a=rand(1000))
group[:banana] = @benchmarkable sort!(a) setup=(a=rand(1100))
group[:pineapple] = @benchmarkable sort!(a) setup=(a=rand(900))
trial = run(group)
plot(
    plot(trial),
    plot(trial, [:pineapple, :apple, :banana]),
    )

benchmarkplots

@vchuravy vchuravy self-requested a review June 7, 2021 09:27
@gustaphe gustaphe changed the title Add boxplot capability Add plot capability Jun 7, 2021
@vchuravy
Copy link
Member

vchuravy commented Jul 3, 2021

Thanks! This looks great :) Would you mind rebasing?

@gustaphe
Copy link
Contributor Author

gustaphe commented Jul 3, 2021

How's this? I guess no bump in version, because the base package only has docs changes.

Co-authored-by: Valentin Churavy <vchuravy@users.noreply.github.com>
@vchuravy
Copy link
Member

vchuravy commented Jul 5, 2021

We should figure out a way to setup tests.

@codecov
Copy link

codecov bot commented Jul 5, 2021

Codecov Report

Merging #194 (e3be16a) into master (4a00cb3) will decrease coverage by 10.57%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #194       +/-   ##
===========================================
- Coverage   85.63%   75.05%   -10.58%     
===========================================
  Files           6        6               
  Lines         738      842      +104     
===========================================
  Hits          632      632               
- Misses        106      210      +104     
Impacted Files Coverage Δ
src/trials.jl 47.43% <0.00%> (-23.72%) ⬇️

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 4a00cb3...e3be16a. Read the comment docs.

@vchuravy vchuravy merged commit 51a918d into JuliaCI:master Jul 5, 2021
@mcabbott
Copy link

mcabbott commented Dec 5, 2021

Should this sub-package depend on StatsPlots?

julia> b = @benchmark copy(rand(128));

julia> using BenchmarkPlots

julia> plot(b)
┌ Warning: seriestype violin has been moved to StatsPlots.  To use: `Pkg.add("StatsPlots"); using StatsPlots`
└ @ Plots ~/.julia/packages/Plots/AJMX6/src/args.jl:1575
ERROR: The backend must not support the series type Val{:violin}, and there isn't a series recipe defined.

(jl_Nt1QFi) pkg> st
Status `/private/var/folders/yq/4p2zwd614y59gszh7y9ypyhh0000gn/T/jl_Nt1QFi/Project.toml`
  [ab8c0f59] BenchmarkPlots v0.1.0
  [91a5bcdd] Plots v1.25.0

@vchuravy
Copy link
Member

vchuravy commented Dec 5, 2021

Probably...

@gustaphe
Copy link
Contributor Author

gustaphe commented Dec 6, 2021

In theory, you can plot(b; st=:hist), or some other series type not from statsplots. But yes, in practice that would probably be simpler.

The next question is, does using StatsPlots from BenchmarkPlots make the seriestypes available in the outer scope?

@mcabbott
Copy link

mcabbott commented Dec 6, 2021

I presume that would work, as Plots uses Revise which would notice, but I'm not sure without trying.

I guess the other option would be to make the default be a histogram, since that works.

However, I think the histogram has mis-labeled axes? Also, I need to set limits by hand to see anything, in the common case that the maximum is huge. And when I do so, the violin plot seems to have been down-sampled to not show very much?

Screenshot 2021-12-05 at 22 17 08

(The REPL histogram cuts off at the 99th percentile, which doesn't seem like a bad default. Typically excludes all the samples in which GC runs.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants