Skip to content

update benchmark suite - #658

Merged
adrn merged 5 commits into
mainfrom
bench-eval-paths
Oct 26, 2026
Merged

adrn merged 5 commits into
mainfrom
bench-eval-paths

Conversation

@lgarrison

Copy link
Copy Markdown
Collaborator

Describe your changes

This is a collection of small updates to the benchmark suite in preparation for #477. The idea is to get most of the benchmarking in place that we hope will show performance improvements in the transpose. Some bugfixes, some expanded coverage. Codspeed will hopefully be faster by doing fewer O2GF orbits; it was dominating the time.

The different-array-types benchmarking is anticipating improvements in reducing the number of copies when the data is a Quantity or Orbit versus a plain array. But this could be something we test via a one-time benchmark if we don't want it in the suite.

Checklist

  • Did you add tests?
  • Did you add documentation for your changes? No, internal changes only.
  • Did you reference any relevant issues?
  • Did you add a changelog entry? (see CHANGES.rst)
  • Are the CI tests passing?
  • Is the milestone set?

lgarrison and others added 5 commits October 21, 2026 15:28
At n = 1 and 10, each potential benchmark measures only the fixed cost
of a call (unit handling and validation), and n = 1,000 is still mostly
fixed cost for cheap potentials. Replace n = 10 with n = 10,000 so the
per-point cost of each evaluation path shows. The classes with expensive
potentials keep their explicit size caps.

Generate the positions in a fixture, so that the timed region is the
evaluation alone: at n = 10,000, generating them costs about as much as
evaluating a cheap potential. With a fixed seed per test, every test
also gets the same positions, regardless of which tests ran before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Hessians had no benchmark. One cheap potential is enough to track the
cost of the evaluation machinery around the kernels.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both integration benchmarks took an n_orbits parameter but always
integrated a single orbit, so each case ran four times under different
names. Integrate n_orbits copies of the initial condition, and skip the
cases with more than 10^6 orbit-steps, which are too slow under
CodSpeed's simulation.

Build the initial conditions in fixtures, so that only the integration
is timed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The potential benchmarks pass plain arrays already in the potential's
units, so they skip the conversions applied to other inputs. Benchmark
energy with a Quantity, with an integrated Orbit, and through
Orbit.energy().

The orbit has 10^6 points: at 10^4, the copies made converting it cost
a few microseconds, hidden by the ~0.3 ms of fixed overhead per call,
while at 10^6 they take most of the time: Orbit.energy() takes 69 ms,
against 15 ms for the same positions as an array.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The O2GF benchmarks with 10 orbits took 49 and 33 minutes of the
CodSpeed run, about 300 times their real time (11 s and 5.5 s), because
most of O2GF's time is the Python-level fit of the toy potential. Each
orbit is fit separately, so the cost is linear in the number of orbits,
and timing 10 orbits measures the same code as timing one.

Skip the multi-orbit cases under CodSpeed. They still run in the
walltime scaling benchmarks, which plot the scaling with n_orbits in the
docs and take about a minute.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codspeed

codspeed Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 82.28%

2 warnings

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

⚡ 6 improved benchmarks
❌ 36 (👁 36) regressed benchmarks
✅ 270 untouched benchmarks
🆕 89 new benchmarks
⏩ 390 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ test_gradient[1000-HarmonicOscillatorPotential] 1,047.1 µs 758.8 µs +38%
⚡ test_gradient[1000-NullPotential] 1,110.5 µs 811.1 µs +36.91%
⚡ test_gradient[1000-HenonHeilesPotential] 913.7 µs 703.6 µs +29.87%
⚡ test_energy[1000-NullPotential] 1.4 ms 1.1 ms +25.52%
⚡ test_energy[1000-HarmonicOscillatorPotential] 1.6 ms 1.3 ms +22.18%
⚡ test_energy[1000-HenonHeilesPotential] 1.3 ms 1.1 ms +17.6%
🆕 test_density[10000-BovyMWPotential2014] N/A 21.3 ms N/A
🆕 test_density[10000-HernquistPotential] N/A 10.2 ms N/A
🆕 test_density[10000-JaffePotential] N/A 10.2 ms N/A
🆕 test_density[10000-BurkertPotential] N/A 10.1 ms N/A
🆕 test_density[10000-LogarithmicPotential] N/A 10.3 ms N/A
🆕 test_density[10000-LongMuraliBarPotential] N/A 12.9 ms N/A
🆕 test_density[10000-MN3ExponentialDiskPotential] N/A 10.6 ms N/A
🆕 test_density[10000-LeeSutoTriaxialNFWPotential] N/A 10.3 ms N/A
🆕 test_density[10000-MiyamotoNagaiPotential] N/A 10.2 ms N/A
🆕 test_density[10000-MilkyWayPotential_v2] N/A 17.8 ms N/A
🆕 test_density[10000-MilkyWayPotential_v1] N/A 17.4 ms N/A
🆕 test_density[10000-NFWPotential] N/A 10.2 ms N/A
🆕 test_density[10000-KuzminPotential] N/A 9.9 ms N/A
🆕 test_density[10000-PlummerPotential] N/A 10.2 ms N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing bench-eval-paths (365c12f) with main (1e9f88e)

Open in CodSpeed

Footnotes

  1. 390 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@lgarrison

Copy link
Copy Markdown
Collaborator Author

All the benchmark "regressions" are on the orbit integration, where we fixed the bug with n_orbits being ignored. So I think we can just acknowledge the regression on codspeed (I don't have permissions to do it myself).

Benchmark CI time is down to 15 minutes from 1+ hours, too.

@lgarrison
lgarrison requested a review from adrn October 26, 2026 13:35
@adrn

adrn commented Oct 26, 2026

Copy link
Copy Markdown
Owner

Regressions: acknowledged!

@adrn adrn added this to the v1.13 milestone Oct 26, 2026
@adrn
adrn merged commit f14a886 into main Oct 26, 2026
30 checks passed
@adrn
adrn deleted the bench-eval-paths branch October 26, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants