Benchmarking - #410
Conversation
# Conflicts: # pyproject.toml
…n rust in python 3.14
|
see #402 |
- asv-benchmarks.yml → benchmark-readiness.yml - asv-openmp-pr.yml → asv-openmp-pr-comparison.yml - asv-openmp-publish.yml → asv-benchmark-publish.yml - asv-openmp-comment.yml → benchmark-pr-comment.yml (new, workflow_run) Update all internal path references. benchmark-pr-comment.yml posts the ASV comparison as a PR comment via workflow_run, which runs in the base repo context and has pull-requests write access for both fork-internal and cross-fork PRs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LSchueler
left a comment
LSchueler
left a comment
There was a problem hiding this comment.
Thanks for your great work! I think once you fixed the requested changes, this is ready to be merged.
| { | ||
| "version": 1, | ||
| "project": "GSTools", | ||
| "project_url": "https://github.com/jeilealr/GSTools", |
There was a problem hiding this comment.
Check all the URLs, they are pointing to your fork, not the original repo
There was a problem hiding this comment.
This file is huge! I think splitting it into something like a parser and a renderer would make it more readable. I don't know a lot about html etc., but can you outsource the render_html stuff into some kind of template?
There was a problem hiding this comment.
I think it would be helpful to add some tests to check if ASV introduces breaking changes (API or result format) in the future.
| def time_variogram_estimate(self, data, backend, case, threads): | ||
| """Run one variogram estimation case.""" | ||
| pos, field, bins = data[case] | ||
| kwargs = {} | ||
| if case == "sampled_5000_to_1500": | ||
| kwargs = {"sampling_size": 1500, "sampling_seed": 20220504} | ||
| if case == "sampled_15000_to_4500": | ||
| kwargs = {"sampling_size": 4500, "sampling_seed": 20220505} | ||
| with gstools_backend(_use_core(backend), threads): | ||
| gs.vario_estimate( | ||
| pos, | ||
| field, | ||
| bins, | ||
| mesh_type="unstructured", | ||
| return_counts=True, | ||
| **kwargs, | ||
| ) | ||
|
|
||
| def peakmem_variogram_estimate(self, data, backend, case, threads): | ||
| """Measure peak memory for variogram estimation case.""" | ||
| pos, field, bins = data[case] | ||
| kwargs = {} | ||
| if case == "sampled_5000_to_1500": | ||
| kwargs = {"sampling_size": 1500, "sampling_seed": 20220504} | ||
| if case == "sampled_15000_to_4500": | ||
| kwargs = {"sampling_size": 4500, "sampling_seed": 20220505} | ||
| with gstools_backend(_use_core(backend), threads): | ||
| gs.vario_estimate( | ||
| pos, | ||
| field, | ||
| bins, | ||
| mesh_type="unstructured", | ||
| return_counts=True, | ||
| **kwargs, | ||
| ) |
There was a problem hiding this comment.
I think you should write an internal function like _run_variogram which gets called from both peakmem_variogram_estimate and time_variogram_estimate. The same goes for kriging
There was a problem hiding this comment.
Do you think this is necessary once your work on the benchmarks is ready? In case it is, can you include the checks into the other actions in order to reduce the CI runtimes?
|
|
||
| [tool.hatch.version.raw-options] | ||
| local_scheme = "no-local-version" | ||
| tag_regex = "^v?(?P<version>[0-9]+\\.[0-9]+.*)$" |
| f"**Base:** `{base}` → **Head:** `{head}`{report_link}\n\n" | ||
| f"{badge}\n\n" | ||
| "<details>\n<summary>Full ASV comparison</summary>\n\n" | ||
| f"```\n{comparison}\n```\n\n" |
There was a problem hiding this comment.
Secure this against content injection.
3 participants
Write Benchmarks for Current DS-MPS Implementation
Summary
This PR adds a reproducible benchmarking setup for GSTools using
Airspeed Velocity plus a small complementary
cProfilehelper.The goal is to establish a measurement baseline before optimizing or extending
the DS-MPS implementation. The benchmarks focus on representative GSTools
workflows rather than isolated micro-functions:
The suite can compare the current Cython fallback backend with the Rust backend
from
gstools_core, and it can report both runtime and peak-memory results.What Changed
asv.conf.json.benchmarks/benchmark_backends.py.gstools-cythoninstallation inside ASVasv.macos-openmp.conf.json.benchmarks/README.md.benchmarkoptional dependency extra.Most of the detailed usage instructions, benchmark case explanations, and
interpretation notes are intentionally kept in
benchmarks/README.mdto avoidduplicating the guide in this PR description.
Why ASV and cProfile?
ASV is used for repeatable benchmark results across commits. This makes it
useful for detecting performance regressions and tracking how workflows change
over time. It is also the standard benchmarking tool used by several scientific
Python projects, including NumPy, SciPy, pandas, scikit-image, and Astropy.
cProfileis used for a different purpose: it profiles one selected workflow inthe current checkout and shows where runtime is spent internally. In this PR,
the cProfile helper imports the same benchmark classes used by ASV, so the
profiling cases stay aligned with the benchmark suite.
Initial Observations
The initial benchmark runs show that Rust is generally faster for larger
variogram and kriging workloads. For very small cases, Rust can be similar or
slightly slower because fixed overhead dominates. With one thread, Rust is not
always faster for every SRF case, so the backend comparison is workflow- and
problem-size-dependent.
The cProfile results suggest that the slowest parts of the measured workflows
are mostly inside compiled backend/numerical routines. That means Python-level
cleanup is unlikely to be the main optimization path for these cases. The next
useful optimization work should focus on algorithmic cost reduction and backend
implementation details.
Follow-Up Tasks
share one generalized OpenMP setup.
current migration work is ready to benchmark.