Repository navigation
Stop timing NumPy in the ASV benchmarks - #3095
Merged
Merged
Conversation
A dpnp change cannot move a NumPy timing, so the numpy executor points only doubled the run time of the elementwise, linalg and random suites and let machine noise on them be reported as a dpnp regression. Keep `executor` as a single-value axis and leave the `setup`/`time_*` bodies untouched, so result names and ASV benchmark versions stay comparable with the recorded history.
Contributor
|
View rendered docs @ https://intelpython.github.io/dpnp/index.html |
Collaborator
Contributor
Author
|
Note, the 65536-size unary entries still keep making about half the runs UNSTABLE, but will be addressed in the follow-up. |
antonwolfy
marked this pull request as ready for review
October 12, 2026 14:12
antonwolfy
requested review from
ndgrigorian and
vlad-perevezentsev
as code owners
October 12, 2026 14:12
Contributor
Author
|
Note, the 65536-size unary entries still keep making about half the runs UNSTABLE, but will be addressed in the follow-up. |
Contributor
|
Array API standard conformance tests for dpnp=0.21.0dev14=np2py314h8d9cdd5_6 ran successfully. |
With numpy gone from _EXECUTORS the helper is only called with "dpnp". It is not part of ASV's benchmark version hash, so benchmark versions stay unchanged.
vlad-perevezentsev
self-requested a review
October 14, 2026 10:19
vlad-perevezentsev
approved these changes
Oct 14, 2026
vlad-perevezentsev
left a comment
Contributor
There was a problem hiding this comment.
LGTM
Thank you @antonwolfy !
github-actions Bot
added a commit
that referenced
this pull request
Oct 14, 2026
This PR removes the `numpy` executor from the ASV benchmark suites, so
`bench_elementwise.py`, `bench_linalg.py` and `bench_random.py` now time
only dpnp.
The suites track dpnp performance from commit to commit, and nothing
compares the dpnp and NumPy timings with each other. A dpnp change
cannot move a NumPy timing, so those points carry no signal. They did
have two costs:
- They made up half of the timed points in these three suites (470 of
940).
- Machine noise on them was reported as a dpnp regression. A `master`
commit that only bumped pre-commit hooks was flagged solely because of
`MatMul.time_matmul('numpy', 16, 'float64')` at 1.22x. On a recent PR,
two of the three reported regressions were NumPy points, including
`MatMul.time_dot('numpy', 64, 'float32')` at 1.6x, with the same NumPy
and MKL builds on both sides.
The change keeps the recorded results comparable:
- `executor` stays as a single-value axis (`'dpnp'`), so result names
such as `time_dot('dpnp', 64, 'float32')` are unchanged.
- The `setup` and `time_*` methods are not modified, including the
`numpy` branches some `setup` methods no longer use. ASV versions a
benchmark by the source of these methods, so editing them would make
every result incomparable with the recorded baseline. All 29 such
methods are byte-identical to `master`.
Against a baseline recorded before this change, the NumPy points exist
only on the baseline side. `asv compare` treats them as skipped, so
`--only-changed` hides them and they are not counted as regressions.
`benchmarks/README.md` is updated to match and gets a new "NumPy is not
timed" section explaining the above. 0809e2d
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR removes the
numpyexecutor from the ASV benchmark suites, sobench_elementwise.py,bench_linalg.pyandbench_random.pynow time only dpnp.The suites track dpnp performance from commit to commit, and nothing compares the dpnp and NumPy timings with each other. A dpnp change cannot move a NumPy timing, so those points carry no signal. They did have two costs:
mastercommit that only bumped pre-commit hooks was flagged solely because ofMatMul.time_matmul('numpy', 16, 'float64')at 1.22x. On a recent PR, two of the three reported regressions were NumPy points, includingMatMul.time_dot('numpy', 64, 'float32')at 1.6x, with the same NumPy and MKL builds on both sides.The change keeps the recorded results comparable:
executorstays as a single-value axis ('dpnp'), so result names such astime_dot('dpnp', 64, 'float32')are unchanged.setupandtime_*methods are not modified, including thenumpybranches somesetupmethods no longer use. ASV versions a benchmark by the source of these methods, so editing them would make every result incomparable with the recorded baseline. All 29 such methods are byte-identical tomaster.Against a baseline recorded before this change, the NumPy points exist only on the baseline side.
asv comparetreats them as skipped, so--only-changedhides them and they are not counted as regressions.benchmarks/README.mdis updated to match and gets a new "NumPy is not timed" section explaining the above.