Benchmarks for individual dynamics functions #577
Conversation
maximilian-gelbrecht
added
performance 🚀
In practice this just means someone has to have BenchmarkTools in their environment and it's not automatically installed no? Because I wouldn't find that thaaat problematic given how mature and widely used BenchmarkTools is |
benchmark/README.md
Outdated
| @@ -1,6 +1,6 @@ | |||
| # Benchmarks | |||
|
|
|||
| created for SpeedyWeather.jl v0.9.0 on Tue, 02 Apr 2024 14:05:51. | |||
| created for SpeedyWeather.jl v0.10.0 on Wed, 25 Sep 2024 18:36:59. | |||
There was a problem hiding this comment.
Should we already change the version to 0.11 here?
There was a problem hiding this comment.
Yeah, makes sense. It's a major change, and some of it is also user facing (the output)
There was a problem hiding this comment.
Happy for you to already bump the version number in this pull request?
benchmark/benchmark_suite.jl
Outdated
| t = median(trial) | ||
|
|
||
| suite.time_median[i_run][i_func] = t.time |
There was a problem hiding this comment.
Is median what's typically printed with @btime? I thought that was lowest?
There was a problem hiding this comment.
Yes, you are right @btime prints the minimum. I didn't know that before. @benchmark prints range, median and mean. Any way, not that it matters a lot, but I'd still prefer median.
There was a problem hiding this comment.
I believe in benchmarking the minimum is often used because it is more robust because that's the most uninterrupted execution of this code
There was a problem hiding this comment.
Ok, I'll take the minimum. I also see now that it's BenchmarkTools default for time(t::Trial) as well
I added the individual dynamics functions as another benchmark suite as in #525. I had to change a few things around because they didn’t fit in the existing
BenchmarkSuitescheme.I use
BenchmarkToolswhich isn’t in the main environment, so I added a new local environment. This isn’t working ideally because theProject.tomldoesn’t know that Speedy was added withdev ... Maybe we should add the Manifest.toml as well here, or you know, just be aware of it when executing the benchmarks.