Benchmarking by jeilealr · Pull Request #410 · GeoStat-Framework/GSTools

jeilealr · 2026-05-15T15:04:35Z

Write Benchmarks for Current DS-MPS Implementation

Summary

This PR adds a reproducible benchmarking setup for GSTools using
Airspeed Velocity plus a small complementary
cProfile helper.

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:

variogram estimation
global kriging
spatial random field generation
conditioned spatial random field generation

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

Added the default ASV configuration in asv.conf.json.
Added benchmark workflows in benchmarks/benchmark_backends.py.
Added helper scripts for:
- Rust-vs-Cython speedup summaries
- cProfile workflow profiling
- Cython OpenMP verification
- macOS OpenMP-enabled gstools-cython installation inside ASV
Added an optional macOS OpenMP ASV configuration in
asv.macos-openmp.conf.json.
Added a detailed benchmark guide in benchmarks/README.md.
Added the benchmark optional dependency extra.
Added a GitHub Actions workflow for benchmark checks.

Most of the detailed usage instructions, benchmark case explanations, and
interpretation notes are intentionally kept in benchmarks/README.md to avoid
duplicating 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.

cProfile is used for a different purpose: it profiles one selected workflow in
the 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

Test and document the OpenMP benchmark setup on Linux.
Test and document the OpenMP benchmark setup on Windows.
Test and document the OpenMP benchmark setup on an HPC environment.
Decide whether Linux, Windows, and HPC need separate ASV config files or can
share one generalized OpenMP setup.
Connect these benchmarks more directly to the DS-MPS implementation once the
current migration work is ready to benchmark.

… SHAs

…s from ASV in a python script

# Conflicts: # pyproject.toml

…n rust in python 3.14

…tion jobs

- 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>

…ame-repo PRs

LSchueler

Thanks for your great work! I think once you fixed the requested changes, this is ready to be merged.

LSchueler · 2026-08-05T10:16:50Z

+{
+  "version": 1,
+  "project": "GSTools",
+  "project_url": "https://github.com/jeilealr/GSTools",


Check all the URLs, they are pointing to your fork, not the original repo

All jeilealr references are gone. Important note on this topic. There are still two things we need for the benchmarking to work. Unfortunately, I cannot do this myself due to permissions. The two missing steps are: 1) GeoStat-Framework/GSTools-Benchmarks must exist with a main branch and Pages enabled (creating an empty README.md file in main branch and pushing the changes, and then to enable Pages in repo: Settings → Pages → source "Deploy from a branch", branch main, folder / (root)). And 2) BENCHMARKS_REPO_TOKEN secret with push rights must be added to GeoStat-Framework/GSTools (on GeoStat-Framework/GSTools: Settings → Secrets and variables → Actions → New repository secret → name BENCHMARKS_REPO_TOKEN → paste the token).

@LSchueler This will be something left to do that I cannot due to permissions

LSchueler · 2026-08-05T10:26:46Z

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?

I think you were refering in this comment to file plot_case_backend_comparison.py. I moved the content of the html into a templates folder to have the python script much more clearer. I

LSchueler · 2026-08-05T10:28:05Z

I think it would be helpful to add some tests to check if ASV introduces breaking changes (API or result format) in the future.

Yes, I agree. I've added tests/test_asv_integration.py, which runs the installed ASV on one fast case and checks the result-JSON layout, asv compare output, asv publish, and that both config files load (runs in source_check, ~7 s).

LSchueler · 2026-08-05T10:30:48Z

+    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,
+            )


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

Good point. Now _run_variogram, run_krige, and run_field for the random-field class; each time*/peakmem* pair now delegates to its helper. ASV discovers the same six benchmarks with identical parameters, so existing results stay comparable and we reduced 46 lines :)

LSchueler · 2026-08-05T10:32:43Z

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?

LSchueler · 2026-08-05T10:36:57Z


 [tool.hatch.version.raw-options]
 local_scheme = "no-local-version"
+tag_regex = "^v?(?P<version>[0-9]+\\.[0-9]+.*)$"


Why have you added this?

Must have been there from some time and didn't remove it, I just checked and this is not needed so I removed it.

LSchueler · 2026-08-05T10:44:01Z

+        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"


Secure this against content injection.

Done. The comparison text can no longer close its code fence, SHAs are validated as hex, and the report link is only kept if it is an artifact URL of this repository. The workflow_run comment job now takes only the PR number and comparison text from the untrusted artifact, verifies the number against the run's head SHA, and resolves commits and artifact URL from the API. Tests added in tests/test_render_benchmark_comment.py.

Added in fc2698b as a workaround for a fork CI install failure that can no longer be reproduced. The default setuptools_scm tag regex handles the project's vX.Y.Z tags. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jeilealr · 2026-09-15T08:15:54Z

Hi @LSchueler

Thank you for having a look at the repo, I addressed all the comments and check that everything is good to go. The only last step I left it in a comment above (first push into benchmark repo and creating BENCHMARKS_REPO_TOKEN)

LSchueler

Thanks! I think this PR is nearly finished. We don't need the comparison anymore, as we have dropped Cython-support. But for future Rust-developments, this will become very handy.

One more thing besides the other requests:
Mann-Witney U test needs the samples, which are discarded, right? Do we really need that test? If yes, do not discard the samples.

LSchueler · 2026-10-05T13:55:02Z

+    markers = [change_marker(line) for line in comparison.splitlines()]
+    regressed = markers.count("+")
+    improved = markers.count("-")
+    note = "Mann-Whitney U · 5% threshold"
+    if regressed:
+        badge = f"⚠️ {regressed} benchmark(s) regressed · {note}"
+    elif improved:
+        badge = f"✅ {improved} benchmark(s) improved, none regressed · {note}"
+    else:
+        badge = f"✅ No significant changes detected · {note}"


This ignores the failure markers, which can result in "No significant changes detected"

You are totally right about this, now it should be recognise and flag properly

LSchueler · 2026-10-05T13:55:20Z

+  "number": 1,
+  "repeat": 20


I think these are ignored by asv, they should be benchmark attributes, not configurations.

Yes, you were right. I re-ran it and it was ignored. I created a small class in the benchmark_two_point_statistic called _TwoPointBenchmark that will get this number and repeat correctly and then it is pass to each class. I think we can keep it like that unless you want more flexibility when editing these numbers

LSchueler · 2026-10-05T13:55:28Z

+  "number": 1,
+  "repeat": 20


I think these are ignored by asv, they should be benchmark attributes, not configurations

same comment above

LSchueler · 2026-10-05T13:56:14Z

+        case_values = [
+            item
+            for item in combo_values
+            if item not in BACKENDS and not item.startswith(THREAD_PREFIX)
+        ]
+        threads = next(
+            (item for item in combo_values if item.startswith(THREAD_PREFIX)),
+            parsed_threads,
+        )
+        rows.append(
+            {
+                "backend": backend,
+                "benchmark": parsed_benchmark,
+                "case": "/".join(case_values) if case_values else parsed_case,
+                "threads": threads,


The thread # are part of the case strings, not part of the threads strings

LSchueler · 2026-10-05T13:58:03Z

+  const groups = new Map();
+  for (const row of data) {
+    const key = `${row.backend}|${row.case}|${row.threads}`;
+    if (!groups.has(key)) groups.set(key, []);
+    groups.get(key).push(row);
+  }
+
+  let svg = `<svg viewBox="0 0 ${width} ${height}" width="${width}" height="${height}" role="img">`;
+  svg += `<text x="${margin.left}" y="26" class="title">${escapeHtml(chartSubtitle(data))}</text>`;
+  svg += `<text x="${margin.left}" y="45" class="axis-label">${unit.label} (${unit.unit}), lower is better</text>`;
+  for (let i = 0; i <= 5; i++) {
+    const value = (maxValue / 5) * i;
+    const yy = y(value);
+    svg += `<line x1="${margin.left}" x2="${width - margin.right}" y1="${yy}" y2="${yy}" class="grid" />`;
+    svg += `<text x="${margin.left - 10}" y="${yy + 4}" text-anchor="end" class="tick">${formatNumber(value)}</text>`;
+  }
+  xKeys.forEach(xKey => {
+    const xx = x(xKey);
+    const row = rows.find(item => rowXKey(item) === xKey);
+    svg += `<text x="${xx}" y="${height - 56}" text-anchor="middle" class="tick" transform="rotate(-25 ${xx} ${height - 56})">${escapeHtml(xKey)}</text>`;
+    if (row && row.date_label !== "-") {
+      svg += `<text x="${xx}" y="${height - 30}" text-anchor="middle" class="tick">${escapeHtml(row.date_label)}</text>`;
+    }
+  });
+
+  Array.from(groups.entries()).forEach(([key, group], index) => {
+    const color = linePalette[index % linePalette.length];
+    const byXKey = new Map(group.map(row => [rowXKey(row), row]));


You have the info on the environment where the benchmarks run, but they are ignored in the graphs. This means that results from completely different machines are treated the same, leading to wrong graphs. Either add environment selection to graphs or reject from different environments

I added a Machine filter. The data already carried per-row machine/OS/CPU info, but yes it graph ignored it, with this machine selector as single choice the results from different machines will never merge :). That was a good catch thanks

LSchueler · 2026-10-05T13:58:23Z

+          PR_HEAD_SHA=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" \
+            --jq '.head.sha')
+          if [[ "$PR_HEAD_SHA" != "$HEAD_SHA" ]]; then
+            echo "::warning::PR #$PR_NUMBER head does not match the" \
+              "benchmarked commit $HEAD_SHA; skipping comment."
+            exit 0
+          fi
+          BASE_SHA=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER" \
+            --jq '.base.sha')


This looks like it could run into problems, if the main branch gets updated

AGain. good catch. solved :)

…tney U

jeilealr · 2026-10-07T17:31:08Z

One last question that is no clear to me. should I then drop now all the benchmarking Cython related scripts or should we do that for the next release? (I think for the next one will be better, but I wonder)

…ficant

LSchueler

Thanks for the edits!
I found a few last(?) things that have to change.

Drop LAST_CHAT.md, AGENTS.md, direct_sampling_mps_guide.md

LSchueler

Thanks again, LGTM!

jeilealr and others added 4 commits April 26, 2026 18:00

Add ASV backend benchmarks

8879b80

First Bench marking: current status, both backends, ASV and cProfile

bfba184

fix parallelisation in benchmarking, adding macos parallelisation

f887065

Merge branch 'GeoStat-Framework:main' into benchmarking

5bc4251

jeilealr mentioned this pull request May 15, 2026

Write benchmarks for current DS-MPS impementation #402

Open

jeilealr added 11 commits May 27, 2026 15:16

Add cross-platform benchmark availability CI

806e386

Fix benchmark CI config resolution and linter scope

2531146

Add Vale Markdown terminology checks

99e5328

Second iteration to fix GitHub actions + pin GitHub actions to commit…

bddc2df

… SHAs

Fix ASV benchmark CI matrix and lint config

fca014e

third iteration GitHub actions

00d1909

removing not needed dependencies

db86d44

4th iteration benchmarking GitHub Actions

bd58de3

simplified version GitHub actions for a pracmatic approach

9bdb2c1

fifth iteration GitHub Actions for benchmarking

9c4f46b

Simplify benchmark CI smoke checks

c894709

jeilealr mentioned this pull request May 29, 2026

Rust Backend failing with Python 3.14 GeoStat-Framework/GSTools-Core#26

Open

jeilealr and others added 3 commits June 1, 2026 15:13

final implementation of benchmarking, adding isolated plotting result…

2d4f157

…s from ASV in a python script

benchmark selector mismatch fix in .github/workflows/asv-benchmarks.yml

c39f88e

Merge branch 'main' into benchmarking

dffb73d

MuellerSeb added this to the v1.8 milestone Jun 5, 2026

jeilealr added 9 commits June 7, 2026 22:01

Merge remote-tracking branch 'upstream/main' into benchmarking

e62e6ba

# Conflicts: # pyproject.toml

rebasing from main, benchmark install dependencies & addign fix to ru…

b588b50

…n rust in python 3.14

first approach to deploy html to static github pages

e0861b9

Fix benchmark report on Python 3.8

8e7d521

Keep benchmark helpers compatible with Python 3.8

2bc1bcb

Install pip before ASV benchmark driver

0e9bc53

Record OpenMP runner hardware metadata

f3a3632

cleaning code

30e0fe4

fix GB display in html github actions

db43a09

jeilealr and others added 9 commits June 9, 2026 14:48

add tag_regex to toml to prevent version failures, renaming github ac…

fc2698b

…tion jobs

improve html benchmarking adding tagging

befd49b

adding info in contributions | implementing asv-openmp-comment.yml

af3b350

improving html visualisation

78703ba

restore the direct comment step in asv-openmp-pr-comparison.yml for s…

d684bba

…ame-repo PRs

add change log, improve html and github actions

302b717

debuging and code cleaning

d57ada2

fix error in benchmark checks

cfd8b06

LSchueler requested changes Aug 5, 2026

View reviewed changes

jeilealr and others added 4 commits September 14, 2026 15:32

solving some questions related to the PR

c34aea2

remove tag_regex from hatch-vcs options

770737a

Added in fc2698b as a workaround for a fork CI install failure that can no longer be reproduced. The default setuptools_scm tag regex handles the project's vX.Y.Z tags. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

final changes addressing the PR commments

0ac21d1

cleaning doc stings/comments

8cccca3

LSchueler requested changes Oct 5, 2026

View reviewed changes

jeilealr added 2 commits October 7, 2026 16:31

Surface benchmark failures in PR comment; record samples for Mann-Whi…

d4e3417

…tney U

solving ASV benchmarking bugs

125b77f

jeilealr added 6 commits October 11, 2026 13:08

merge changes in main, droppping cython

b1c6d76

removing Cython from benchmarking

c59086e

cleaning benchmarking and adding back the html GSTools results

b24ffb0

fix false-green comparison now report unabailable instead of no signi…

ba75e07

…ficant

ruff format check

1824b2b

cahnging repeat from 20 to 5 to bring down the run time of the PR jobs

230f09a

LSchueler requested changes Oct 12, 2026

View reviewed changes

Comment thread .github/workflows/benchmark-pr-comment.yml

Comment thread benchmarks/benchmark_two_point_statistics.py Outdated

Comment thread benchmarks/benchmark_two_point_statistics.py

jeilealr added 2 commits October 12, 2026 15:50

tweaks for PR preparation, removing files and making adjustments in code

0308194

removing benchmarking contributions as well as in the chagelog

7453294

LSchueler approved these changes Oct 13, 2026

View reviewed changes

LSchueler merged commit a31e5fc into GeoStat-Framework:main Oct 13, 2026
43 checks passed

Conversation

jeilealr commented May 15, 2026

Write Benchmarks for Current DS-MPS Implementation

Summary

What Changed

Why ASV and cProfile?

Initial Observations

Follow-Up Tasks

Uh oh!

LSchueler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeilealr commented Sep 15, 2026

Uh oh!

LSchueler left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeilealr commented Oct 7, 2026

Uh oh!

LSchueler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LSchueler left a comment

Choose a reason for hiding this comment

LSchueler left a comment •

edited

Loading