Repository navigation
Set up benchmarks to run nightly on cron (add lf-bench, slurm + cron wrappers, testing) - #8
Conversation
- Wrap each benchmark's compute in lbench_dask so --lbench records timings, cProfile output, and Dask performance reports. - Move the catalog and Dask-client fixtures into conftest; add an ALERT_ARCHIVE_CATALOG_DIR env override (defaulting to the HPC path) and a 2-worker single_thread_dask_client override (ALERT_ARCHIVE_DASK_WORKERS) that reproduces the original LocalCluster. - Add a [benchmark] optional-dependencies group (lf-bench from git, lsdb, nested-pandas, hats, astropy). - Shim an lf-bench / pytest-benchmark>=5 incompatibility (lf-bench passes an open file handle where pytest-benchmark expects a path) so results JSON is written. Remove once lf-bench is fixed upstream. The benchmark logic (including the naive light-curve aggregation) is unchanged.
…ncies - Move lsdb, nested-pandas, and hats from the [benchmark] extra to the main project dependencies (astropy and lf-bench stay benchmark-only). - Use the original multi-statement style in the benchmark bodies; each test only swaps the client fixture for lbench_dask and wraps the compute call.
e721667 to
def6620
Compare
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Added instructions for testing a single benchmark using pytest.
…we add for lbench; check our venv pins)
6234021 to
8f63510
Compare
olivialynn
changed the title
camposandro
left a comment
camposandro
left a comment
There was a problem hiding this comment.
Thanks, this is a lot of work!!
Left some comments/questions below, most of them are minor:
|
|
||
| @pytest.fixture | ||
| def lbench(benchmark_results_dir, benchmark, request): | ||
| """Override lf-bench's ``lbench`` fixture to fix the run count and allow skipping cProfile. |
There was a problem hiding this comment.
As a follow-up to this PR, would it be worth filing an issue in lsdb-benchmarking to make the lbench fixture more flexible? It'd be nice if we could do some of these overrides with a decorator. I think the %%lbench notebook magic already takes --rounds, so I'm assuming there's already something in place to make it possible.
| # Benchmarking harness (pytest plugin + dashboard). Keep the commit in sync | ||
| # with benchmarks/nightly/requirements.txt. | ||
| "lf-bench @ git+https://github.com/lincc-frameworks/lsdb-benchmarking@605bd6fedfdc592e7ff3249bf06db0715f13587e", | ||
| "astropy", |
There was a problem hiding this comment.
Since astropy is already included in hats and we're not forcing any other pin here, do we need it?
There was a problem hiding this comment.
The powers that be have told me that, since we do import it directly in the benchmarks, we should include it in the deps - but a good idea to avoid pinning it for sure
2 participants
Brief overview:
Adds lf-bench as a dependency to wrap and manage runs of the benchmarks written in astronomy-commons/ppdb_hats#7. Also, cron and slurm wrappers for launching from SDF crontab.
For a more detailed explanation of the nightly benchmarks, I recommend reading the README added to the
benchmarks/directory.Once this PR is merged, I will launch the cron job (#9) and nightly results will be saved to
/sdf/data/rubin/shared/lsdb_commissioning/rubin_alert_archive_benchmarks(I explicitly made this dir read/editable to rubin_users group).Main changes:
lbench_daskfixture to wrap the query (mostly the final compute) in each of the benchmark testsbenchmarks/nightly/run.sbatch, Slurm job to run benchmarksbenchmarks/nightly/submit.sh, to set up the venv and launch the Slurm jobAlso:
lbenchoverride in the conftest, specifically to let us usebenchmark.pedanticto ask for 1 warm-up and 5 timed rounds (then skip a follow-up cprofile run)df.query(f"diaObjectId in {sample_ids.tolist()}")withdf[df["diaObjectId"].isin(sample_ids)]for faster querying (recommended by our friend 🙇 )Note: