Add requirements.txt, missing patch to ASV benchmarks#331
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the benchmarks setup to force NumPy’s FFT to run through Intel MKL (via mkl_fft) and tightens ASV regression thresholds.
Changes:
- Add an import-time patch step that applies
mkl_fft.patch_numpy_fft()during benchmark initialization. - Add a benchmarks
requirements.txtwith baseline dependencies. - Reduce ASV
regressions_thresholdsfrom0.3to0.2.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| benchmarks/requirements.txt | Adds benchmarks dependencies list for environment creation. |
| benchmarks/benchmarks/_patch_setup.py | Implements mkl_fft patching with hard-fail checks and diagnostics. |
| benchmarks/benchmarks/init.py | Calls the patch function during benchmark package import. |
| benchmarks/asv.conf.json | Tightens regression threshold to catch smaller performance changes. |
| _PATCH_MAP = [ | ||
| ("mkl_fft", "patch_numpy_fft"), | ||
| ] |
There was a problem hiding this comment.
making this a dictionary would make more sense to me, especially with the name "PATCH_MAP"
| patch_fn = getattr(mod, patch_fn_name, None) | ||
| if patch_fn is None: | ||
| raise RuntimeError( | ||
| f"[mkl-patch] {mod_name} has no {patch_fn_name}(). " |
There was a problem hiding this comment.
| f"[mkl-patch] {mod_name} has no {patch_fn_name}(). " | |
| f"{mod_name} has no {patch_fn_name}(). " |
There was a problem hiding this comment.
all [mkl-patch] should be removed I feel, it's a bit confusing, makes it seem like a package (we have mkl-service after all).
| attr = "unknown" | ||
| print(f"[mkl-patch] {mod_name}: numpy.fft dispatch -> {attr}") | ||
|
|
||
| print("[mkl-patch] ALL OK -- mkl_fft active") |
There was a problem hiding this comment.
I think we can drop this line -- the patching already has a verbose mode. If anything, we could just call into the verbose mode, I feel.
| attr = _attr_checks[mod_name]() | ||
| except Exception: | ||
| attr = "unknown" | ||
| print(f"[mkl-patch] {mod_name}: numpy.fft dispatch -> {attr}") |
There was a problem hiding this comment.
same here, a call with verbose patching could substitute
_patch_setup.pythat callspatch_numpy_fft()at worker import time with hard-fail validation (ensures benchmarks never silently run with stock NumPy)requirements.txtwithpsutilandscipybenchmark dependencies