-
Notifications
You must be signed in to change notification settings - Fork 283
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 bm_runner 'trialrun' subcommand. #5957
Conversation
Since |
Or maybe we SHOULD automate the installation, at the same time as Mule? It could be an identical approach? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this. Delivers exactly what is needed, especially useful as a guide to someone inexperienced with ASV.
Sorry I just force-pushed because the pre-commit bot had added corrections. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5957 +/- ##
=======================================
Coverage 89.78% 89.78%
=======================================
Files 93 93
Lines 23007 23007
Branches 5017 5017
=======================================
Hits 20657 20657
Misses 1620 1620
Partials 730 730 ☔ View full report in Codecov by Sentry. |
Ok, latest commit I think addresses the review comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of new points
Thanks @trexfeathers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All seems to work, but the --help
is slightly misleading so I have a suggestion
Thanks for your patience, @trexfeathers ! |
* main: (759 commits) Bump scitools/workflows from 2024.05.1 to 2024.06.0 (SciTools#5986) [pre-commit.ci] pre-commit autoupdate (SciTools#5980) Updated environment lockfiles (SciTools#5983) Bump scitools/workflows from 2024.05.0 to 2024.05.1 (SciTools#5984) Make `slices_over` tests go faster (SciTools#5973) Updated environment lockfiles (SciTools#5979) Update lock files with associated fixes (SciTools#5953) List 25 slowest tests (SciTools#5969) used a note to highlight some text (SciTools#5971) Lazy `iris.cube.Cube.rolling_window` (SciTools#5795) Add memory benchmarks (SciTools#5960) Whatsnew for several benchmark developments. (SciTools#5961) Remove "on-demand" from some benchmarks (SciTools#5959) Add bm_runner 'trialrun' subcommand. (SciTools#5957) Automatically install iris-test-data for benchmark data generation (SciTools#5958) Added benchmarks for collapse and aggregate (SciTools#5954) Use tracemalloc for memory measurements. (SciTools#5948) Provide a Nox `benchmarks` session as the recommended entry point (SciTools#5951) [pre-commit.ci] pre-commit autoupdate (SciTools#5952) Remove unit benchmarks (SciTools#5949) ...
Closes #5944
Notes: I tried to make this as foolproof and self-contained as possible.
so it sets up DATA_GEN_PYTHON and ON_DEMAND_BENCHMARKS.
However you must still provide iris-test-data, via OVERRIDE_TEST_DATA_REPOSITORY (at least for most benchmarks).
If you don't, the resulting error is not that obvious (!) -- see below
You may also need to set BENCHMARK_DATA if you don't want to overfill your homespace or /tmp/persistent (here at MetOffice).
So, this is potentially still tricky to use, and even dangerous if you total your disk quota!
You may also provide a suitable environment, to prevent it creating one.
Omitting this does work, but can be slow as although we can build a suitable test env, we still don't have a reliable way of automatically caching + re-using them.
If the "Speed up environment prep" task currently on the board comes good, it might improve that case ?
Detail of error report due to missing test-data