Add asv benchmarks for edge_detection (#3672)#3673
Open
brendancol wants to merge 2 commits into
Open
Conversation
brendancol
commented
Jul 18, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: Add asv benchmarks for edge_detection (#3672)
Blockers (must fix before merge)
- None.
Suggestions (should fix, not blocking)
- None.
Nits (optional improvements)
-
benchmarks/benchmarks/edge_detection.py:14-19: the cupy timings don't synchronize the device after the call, so for async numba kernel launchestime_*can measure launch overhead rather than kernel time. Every other benchmark in the suite (common.Benchmarking,Convolve2d) does the same, so fixing it only here would make the numbers incomparable. Fine to leave; a suite-wide sync pass would be its own change.
What looks good
setupgoes throughget_xr_dataarray, so a missing cupy raisesNotImplementedErrorand asv records a skip instead of an error.- The dask result is forced with
.compute()(checked viahasattronresult.data), so the benchmark times the kernel, not graph construction. numpy and cupy results pass through untouched. - Sizes ([300, 3000]) follow the "one small, one realistic" convention and keep added CI runtime low; the PR body reports all 30 combos executed locally, including cupy on a GPU host.
- The state CSV row is one physical line and matches the sweep schema.
Checklist
- Algorithm matches reference/paper (n/a, benchmark-only)
- All implemented backends produce consistent results (n/a, no source change)
- NaN handling is correct (n/a)
- Edge cases are covered by tests (n/a, benchmark-only)
- Dask chunk boundaries handled correctly (default
get_xr_dataarray2x2 chunking, real chunked execution) - No premature materialization (
.compute()only where the timing requires it) - Benchmark exists or is not needed (this PR is the benchmark)
- README feature matrix updated (n/a, no new functions)
- Docstrings present and accurate (n/a, benchmark classes follow suite style)
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.
Closes #3672.
benchmarks/benchmarks/edge_detection.pywith anEdgeDetectionclass covering all five public functions (sobel_x,sobel_y,prewitt_x,prewitt_y,laplacian), parameterized over nx in [300, 3000] and numpy/cupy/dask via the suite'sget_xr_dataarray.Backend coverage: numpy, cupy, dask+numpy. dask+cupy is not parameterizable with the current
get_xr_dataarray(common.py has no such type), same as the rest of the suite.Test plan: