From e0277d66a48e4c646dcb03fc38a118eb7213f924 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Thu, 30 Jul 2026 09:08:05 -0400 Subject: [PATCH 01/14] fix(sync): check backend capability before granting it, and dispatch fail-closed optimization='metal' resolved to ('metal', 'mps') for every metric whenever PyObjC Metal was importable, without checking whether the metric had a Metal kernel. Only PLI, wPLI and ACCorr do. For the six einsum metrics the hand-written if/elif chain in compute() then fell through to a bare `return self._compute_numpy(...)`, so the caller got a numpy result while the object reported _backend == 'metal', with no warning. The same happened via priority=['metal', ...], a pattern the docs demonstrate. Two independent defects combined here: the resolver never consulted metric capability, and dispatch failed open so the mismatch stayed silent. - Add BaseMetric.supports(), deriving capability from the presence of the matching _compute_* method rather than a hand-kept list that can drift. - Check capability before availability in _resolve_optimization: warn and fall back to numpy, following the convention of the existing fallbacks. - Skip unsupported backends in the _resolve_auto priority loop, so priority=['metal', 'torch'] on an einsum metric lands on torch. - Replace the nine duplicated if/elif chains with a table-driven compute() on BaseMetric. An unknown backend now raises KeyError instead of silently computing in numpy. Per-metric compute() docstrings are preserved. - Make _compute_numpy abstract, enforcing that every metric ships the reference implementation the accelerated backends are validated against. No Metal shader for the six einsum metrics ever existed; torch/MPS is their intended GPU path per the AUTO_PRIORITY benchmarks. optimization='auto' was already correct and is unchanged. Adds TestBackendCapability, which patches the availability flags instead of gating on hardware so the contract is verified in CI. Refs #299 --- hypyp/sync/accorr.py | 11 +-- hypyp/sync/base.py | 133 +++++++++++++++++++++++++++++++++++- hypyp/sync/ccorr.py | 8 +-- hypyp/sync/coh.py | 8 +-- hypyp/sync/envelope_corr.py | 8 +-- hypyp/sync/imaginary_coh.py | 8 +-- hypyp/sync/pli.py | 10 +-- hypyp/sync/plv.py | 8 +-- hypyp/sync/pow_corr.py | 8 +-- hypyp/sync/wpli.py | 10 +-- tests/test_sync.py | 95 +++++++++++++++++++++++++- 11 files changed, 233 insertions(+), 74 deletions(-) diff --git a/hypyp/sync/accorr.py b/hypyp/sync/accorr.py index dd83c2d..0c58320 100644 --- a/hypyp/sync/accorr.py +++ b/hypyp/sync/accorr.py @@ -109,16 +109,7 @@ def compute( con : np.ndarray ACCorr connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "metal": - return self._compute_metal(complex_signal, n_samp, transpose_axes) - elif self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - else: - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_metal( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 955d3f7..29020e4 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -303,6 +303,28 @@ class BaseMetric(ABC): name: str = "base" + #: Maps a backend name to the method implementing it. This table is the + #: single source of truth for dispatch: ``compute`` looks the backend up + #: here, so an unrecognised backend raises ``KeyError`` instead of silently + #: falling through to numpy, and ``supports`` derives capability from the + #: methods a subclass actually defines rather than from a hand-kept list. + _BACKEND_METHODS = { + "numpy": "_compute_numpy", + "numba": "_compute_numba", + "torch": "_compute_torch", + "metal": "_compute_metal", + "cuda_kernel": "_compute_cuda", + } + + #: Human-readable backend names, used in fallback warnings. + _BACKEND_LABELS = { + "numpy": "numpy", + "numba": "numba", + "torch": "torch", + "metal": "Metal", + "cuda_kernel": "CUDA", + } + def __init__( self, optimization: Optional[str] = None, priority: Optional[list] = None ): @@ -310,6 +332,37 @@ def __init__( self._priority = priority self._backend, self._device = self._resolve_optimization(optimization, priority) + @classmethod + def supports(cls, backend: str) -> bool: + """ + Whether this metric implements ``backend``. + + Capability is derived from the presence of the corresponding + ``_compute_*`` method, so it cannot drift out of sync with the code. + Not every metric has every backend — Metal kernels exist only for the + sign-based metrics and ACCorr, because torch on MPS is faster for the + einsum metrics at every channel count (see ``AUTO_PRIORITY``). + + Parameters + ---------- + backend : str + One of ``'numpy'``, ``'numba'``, ``'torch'``, ``'metal'``, + ``'cuda_kernel'``. An unknown name returns ``False``. + + Returns + ------- + bool + True if the metric can run on ``backend``. + + Examples + -------- + >>> from hypyp.sync import PLI, PLV + >>> PLI.supports('metal'), PLV.supports('metal') + (True, False) + """ + method = cls._BACKEND_METHODS.get(backend) + return method is not None and hasattr(cls, method) + @classmethod def _resolve_optimization( cls, optimization: Optional[str] = None, priority: Optional[list] = None @@ -367,6 +420,27 @@ def _resolve_optimization( if optimization == "auto": return cls._resolve_auto(priority) + if optimization not in ("numba", "torch", "metal", "cuda_kernel"): + raise ValueError( + f"Unknown optimization '{optimization}'. " + f"Options: None, 'auto', 'numba', 'torch', 'metal', 'cuda_kernel'" + ) + + # Capability before availability: a backend the machine can run is + # still useless if this metric has no implementation for it. Without + # this check the backend was accepted and dispatch quietly returned a + # numpy result — the caller believed they were on the GPU. + if not cls.supports(optimization): + label = cls._BACKEND_LABELS[optimization] + warnings.warn( + f"{cls.name!r} has no {label} implementation, falling back to " + f"numpy. Use optimization='auto' to select the best backend " + f"available for this metric.", + UserWarning, + stacklevel=3, + ) + return "numpy", "cpu" + if optimization == "numba": if NUMBA_AVAILABLE: return "numba", "cpu" @@ -411,6 +485,9 @@ def _resolve_optimization( ) return "numpy", "cpu" + # Unreachable: the membership test above already rejected any other + # value. Kept as a guard in case a backend is added to that tuple + # without a matching branch here. raise ValueError( f"Unknown optimization '{optimization}'. " f"Options: None, 'auto', 'numba', 'torch', 'metal', 'cuda_kernel'" @@ -463,6 +540,11 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: priority = AUTO_PRIORITY.get(cls.name, {}).get(platform, []) for backend in priority: + # Skip a backend this metric has no implementation for, so a + # priority list falls through to the next candidate instead of + # selecting a backend that would degrade to numpy at dispatch. + if not cls.supports(backend): + continue if backend == "torch" and TORCH_AVAILABLE: return cls._resolve_torch() if backend == "metal" and METAL_AVAILABLE: @@ -513,12 +595,58 @@ def _resolve_torch() -> tuple: warnings.warn("No GPU found, using torch on CPU", UserWarning, stacklevel=4) return "torch", "cpu" - @abstractmethod def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple ) -> np.ndarray: """ - Compute the connectivity metric. + Compute the connectivity metric on the resolved backend. + + Dispatch is table-driven via ``_BACKEND_METHODS``: the backend chosen at + construction selects the ``_compute_*`` method to run. Subclasses + implement those methods and do not override this one. + + Parameters + ---------- + complex_signal : np.ndarray + Complex analytic signals with shape (n_epochs, n_freq, 2*n_channels, n_times). + n_samp : int + Number of time samples. + transpose_axes : tuple + Axes to transpose for matrix multiplication. + + Returns + ------- + con : np.ndarray + Connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). + + Raises + ------ + KeyError + If ``self._backend`` is not a known backend name. This is + deliberate: an earlier hand-written ``if/elif`` chain per metric + ended in a bare ``return self._compute_numpy(...)``, so an + unhandled backend was indistinguishable from the numpy default and + failed silently. Dispatching through the table fails loudly instead. + + Notes + ----- + Output dtype follows the backend: numpy, numba and CUDA return + ``float64``; the Metal kernels return ``float32``. + """ + method_name = self._BACKEND_METHODS[self._backend] + method = getattr(self, method_name) + return method(complex_signal, n_samp, transpose_axes) + + @abstractmethod + def _compute_numpy( + self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple + ) -> np.ndarray: + """ + Reference implementation, in numpy. Always available. + + Every metric must provide this: it is the correctness oracle the + accelerated backends are validated against, and the fallback target + whenever a requested backend is unavailable or unimplemented. Parameters ---------- @@ -534,4 +662,3 @@ def compute( con : np.ndarray Connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - pass diff --git a/hypyp/sync/ccorr.py b/hypyp/sync/ccorr.py index b859776..33e3843 100644 --- a/hypyp/sync/ccorr.py +++ b/hypyp/sync/ccorr.py @@ -61,13 +61,7 @@ def compute( con : np.ndarray CCorr connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_cuda(self, complex_signal, n_samp, transpose_axes): """CUDA kernel for CCorr.""" diff --git a/hypyp/sync/coh.py b/hypyp/sync/coh.py index 3cd37bf..2bbea53 100644 --- a/hypyp/sync/coh.py +++ b/hypyp/sync/coh.py @@ -67,13 +67,7 @@ def compute( con : np.ndarray Coherence connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_cuda(self, complex_signal, n_samp, transpose_axes): """CUDA kernel for Coherence.""" diff --git a/hypyp/sync/envelope_corr.py b/hypyp/sync/envelope_corr.py index 234c611..1d9cc11 100644 --- a/hypyp/sync/envelope_corr.py +++ b/hypyp/sync/envelope_corr.py @@ -71,13 +71,7 @@ def compute( Envelope Correlation connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_cuda(self, complex_signal, n_samp, transpose_axes): """CUDA kernel for Envelope Correlation.""" diff --git a/hypyp/sync/imaginary_coh.py b/hypyp/sync/imaginary_coh.py index 630d4c5..27a58dc 100644 --- a/hypyp/sync/imaginary_coh.py +++ b/hypyp/sync/imaginary_coh.py @@ -70,13 +70,7 @@ def compute( Imaginary Coherence connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_cuda(self, complex_signal, n_samp, transpose_axes): """CUDA kernel for Imaginary Coherence.""" diff --git a/hypyp/sync/pli.py b/hypyp/sync/pli.py index 669b88c..fcb0925 100644 --- a/hypyp/sync/pli.py +++ b/hypyp/sync/pli.py @@ -71,15 +71,7 @@ def compute( con : np.ndarray PLI connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "metal": - return self._compute_metal(complex_signal, n_samp, transpose_axes) - elif self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_numpy( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/plv.py b/hypyp/sync/plv.py index 7fc6194..b909f91 100644 --- a/hypyp/sync/plv.py +++ b/hypyp/sync/plv.py @@ -67,13 +67,7 @@ def compute( con : np.ndarray PLV connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_cuda(self, complex_signal, n_samp, transpose_axes): """CUDA kernel for PLV.""" diff --git a/hypyp/sync/pow_corr.py b/hypyp/sync/pow_corr.py index 21d403c..356f8fb 100644 --- a/hypyp/sync/pow_corr.py +++ b/hypyp/sync/pow_corr.py @@ -73,13 +73,7 @@ def compute( Power Correlation connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_cuda(self, complex_signal, n_samp, transpose_axes): """CUDA kernel for Power Correlation.""" diff --git a/hypyp/sync/wpli.py b/hypyp/sync/wpli.py index a42ca2d..e591196 100644 --- a/hypyp/sync/wpli.py +++ b/hypyp/sync/wpli.py @@ -70,15 +70,7 @@ def compute( con : np.ndarray wPLI connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ - if self._backend == "metal": - return self._compute_metal(complex_signal, n_samp, transpose_axes) - elif self._backend == "cuda_kernel": - return self._compute_cuda(complex_signal, n_samp, transpose_axes) - elif self._backend == "torch": - return self._compute_torch(complex_signal, n_samp, transpose_axes) - elif self._backend == "numba": - return self._compute_numba(complex_signal, n_samp, transpose_axes) - return self._compute_numpy(complex_signal, n_samp, transpose_axes) + return super().compute(complex_signal, n_samp, transpose_axes) def _compute_metal( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/tests/test_sync.py b/tests/test_sync.py index 2e5ca5a..ac8d098 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -5,13 +5,14 @@ implementation to ensure numerical correctness. """ +import warnings from unittest.mock import patch import numpy as np import pytest from hypyp.analyses import compute_sync -from hypyp.sync import get_metric +from hypyp.sync import METRICS, get_metric from hypyp.sync.accorr import ACCorr from hypyp.sync.base import ( BaseMetric, @@ -1237,3 +1238,95 @@ def test_priority_parameter_propagated_via_get_metric(self): """get_metric passes priority through to the metric class.""" m = get_metric("accorr", optimization="auto", priority=["numba"]) assert m._priority == ["numba"] + + +class TestBackendCapability: + """ + A requested backend must either run, or degrade with a warning. + + Only PLI, wPLI and ACCorr have Metal kernels — torch/MPS is the intended + GPU path for the six einsum metrics (see hypyp/sync/base.py AUTO_PRIORITY + rationale and the support matrix in hypyp/sync/README.md). Requesting + 'metal' for a metric that has no Metal kernel must therefore be reported, + never silently answered with numpy. + + These tests patch the availability flags instead of gating on hardware, so + the capability contract is verified on any machine including CI. + """ + + METAL_CAPABLE = {"pli", "wpli", "accorr"} + + def test_supports_reflects_the_implemented_methods(self): + """supports() must be derived from the code, not a hand-kept list.""" + for mode, cls in METRICS.items(): + assert cls.supports("metal") == hasattr(cls, "_compute_metal") + assert cls.supports("numpy") is True + assert cls.supports("numba") == hasattr(cls, "_compute_numba") + + def test_metal_capability_matches_documented_support_matrix(self): + """Exactly PLI/wPLI/ACCorr expose a Metal kernel.""" + actual = {mode for mode, cls in METRICS.items() if cls.supports("metal")} + assert actual == self.METAL_CAPABLE + + @pytest.mark.parametrize("mode", sorted(METRICS)) + def test_metal_request_never_silently_degrades(self, mode): + """ + optimization='metal' either resolves to metal, or warns and uses numpy. + + Regression test: before the capability check, _resolve_optimization + granted ('metal', 'mps') to every metric, and compute() then fell + through its if/elif chain into _compute_numpy — so six metrics returned + a numpy result while reporting _backend == 'metal', with no warning. + """ + with patch("hypyp.sync.base.METAL_AVAILABLE", True): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + metric = get_metric(mode, optimization="metal") + + if mode in self.METAL_CAPABLE: + assert metric._backend == "metal" + else: + assert metric._backend == "numpy", ( + f"{mode}: asked for metal, resolved to {metric._backend!r} " + f"but has no Metal kernel" + ) + messages = [ + str(w.message) for w in caught if issubclass(w.category, UserWarning) + ] + assert any("Metal" in m for m in messages), ( + f"{mode}: degraded to numpy without warning (messages: {messages})" + ) + + @pytest.mark.parametrize("mode", sorted(set(METRICS) - {"pli", "wpli", "accorr"})) + def test_auto_priority_skips_unsupported_backend(self, mode): + """ + A priority list must skip a backend the metric cannot run. + + priority=['metal', 'torch'] on an einsum metric should land on torch, + not on a metal that resolves to numpy behind the caller's back. + """ + with ( + patch("hypyp.sync.base.METAL_AVAILABLE", True), + patch("hypyp.sync.base.TORCH_AVAILABLE", True), + patch("hypyp.sync.base.MPS_AVAILABLE", True), + ): + metric = get_metric(mode, optimization="auto", priority=["metal", "torch"]) + assert metric._backend == "torch", ( + f"{mode}: priority=['metal','torch'] resolved to {metric._backend!r}" + ) + + def test_unknown_backend_fails_closed(self, complex_signal): + """ + An unrecognised _backend must raise, not quietly compute in numpy. + + This is the fail-closed guarantee: the original if/elif chains ended in + a bare `return self._compute_numpy(...)`, so any unhandled backend value + became indistinguishable from the default. + """ + from hypyp.sync.plv import PLV + + n_samp = complex_signal.shape[3] + metric = PLV() + metric._backend = "not_a_backend" + with pytest.raises(KeyError): + metric.compute(complex_signal, n_samp, (0, 1, 3, 2)) From e3207d1eebfa23b01f151bd8036c8df78f14d6cf Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Thu, 30 Jul 2026 09:27:12 -0400 Subject: [PATCH 02/14] test(sync): drop six vacuous Metal tests, assert the kernel ran in the real ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The six einsum metrics have no Metal kernel — torch/MPS is their intended GPU path — so test_{plv,ccorr,coh,imcoh,envcorr,powcorr}_metal_vs_numpy compared numpy against numpy. They passed unconditionally on any macOS with PyObjC and would have kept passing if a Metal kernel were added later and were wrong. Their real subject, the documented fallback, is now covered for all nine metrics by TestBackendCapability, so no coverage is lost by removing them. test_ccorr_metal_vs_numpy also documented a shader that never existed in any commit: "Uses Kahan summation with fastMath=OFF to preserve IEEE-754 compliance". Neither string appears anywhere in hypyp/sync/kernels/. That docstring is what made the resolver bug look like deleted work. Add `assert metric._backend == 'metal'` to the four tests that do exercise a Metal kernel (PLI, wPLI, ACCorr, plus the 256-channel PLI case). Verified this guard bites: deleting PLI._compute_metal makes the metric resolve to numpy and the assertion fail, instead of the test silently comparing numpy to numpy. Refs #300 --- tests/test_sync.py | 111 ++++++--------------------------------------- 1 file changed, 14 insertions(+), 97 deletions(-) diff --git a/tests/test_sync.py b/tests/test_sync.py index ac8d098..b324159 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -226,20 +226,6 @@ def test_plv_symmetry(self, complex_signal): result[e, f], result[e, f].T, rtol=1e-10, atol=1e-12 ) - @pytest.mark.skipif(not METAL_AVAILABLE, reason="Metal not available") - def test_plv_metal_vs_numpy(self, complex_signal): - """Metal PLV should match numpy PLV within float32 tolerance.""" - from hypyp.sync.plv import PLV - - n_samp = complex_signal.shape[3] - result_np = PLV(optimization=None).compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - result_metal = PLV(optimization="metal").compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - np.testing.assert_allclose(result_metal, result_np, rtol=1e-5, atol=1e-5) - @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") def test_plv_cuda_vs_numpy(self, complex_signal): """CUDA PLV should match numpy PLV exactly (both float64).""" @@ -357,23 +343,6 @@ def test_ccorr_torch_vs_numpy(self, complex_signal): else: np.testing.assert_allclose(result_torch, result_np, rtol=1e-9, atol=1e-10) - @pytest.mark.skipif(not METAL_AVAILABLE, reason="Metal not available") - def test_ccorr_metal_vs_numpy(self, complex_signal): - """Metal CCorr should match numpy CCorr within float32 tolerance. - - Uses Kahan summation with fastMath=OFF to preserve IEEE-754 compliance. - """ - from hypyp.sync.ccorr import CCorr - - n_samp = complex_signal.shape[3] - result_np = CCorr(optimization=None).compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - result_metal = CCorr(optimization="metal").compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - np.testing.assert_allclose(result_metal, result_np, rtol=1e-5, atol=1e-5) - @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") def test_ccorr_cuda_vs_numpy(self, complex_signal): """CUDA CCorr should match numpy CCorr exactly (both float64).""" @@ -457,20 +426,6 @@ def test_coh_symmetry(self, complex_signal): result[e, f], result[e, f].T, rtol=1e-10, atol=1e-12 ) - @pytest.mark.skipif(not METAL_AVAILABLE, reason="Metal not available") - def test_coh_metal_vs_numpy(self, complex_signal): - """Metal Coh should match numpy Coh within float32 tolerance.""" - from hypyp.sync.coh import Coh - - n_samp = complex_signal.shape[3] - result_np = Coh(optimization=None).compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - result_metal = Coh(optimization="metal").compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - np.testing.assert_allclose(result_metal, result_np, rtol=1e-5, atol=1e-5) - @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") def test_coh_cuda_vs_numpy(self, complex_signal): """CUDA Coh should match numpy Coh exactly (both float64).""" @@ -554,20 +509,6 @@ def test_imcoh_symmetry(self, complex_signal): result[e, f], result[e, f].T, rtol=1e-10, atol=1e-12 ) - @pytest.mark.skipif(not METAL_AVAILABLE, reason="Metal not available") - def test_imcoh_metal_vs_numpy(self, complex_signal): - """Metal ImCoh should match numpy ImCoh within float32 tolerance.""" - from hypyp.sync.imaginary_coh import ImCoh - - n_samp = complex_signal.shape[3] - result_np = ImCoh(optimization=None).compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - result_metal = ImCoh(optimization="metal").compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - np.testing.assert_allclose(result_metal, result_np, rtol=1e-5, atol=1e-5) - @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") def test_imcoh_cuda_vs_numpy(self, complex_signal): """CUDA ImCoh should match numpy ImCoh exactly (both float64).""" @@ -756,9 +697,11 @@ def test_pli_metal_vs_numpy(self, complex_signal): result_np = PLI(optimization=None).compute( complex_signal, n_samp, self.TRANSPOSE_AXES ) - result_metal = PLI(optimization="metal").compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) + metric_metal = PLI(optimization="metal") + # Assert the kernel actually ran: a silent fallback to numpy would make + # this comparison numpy-vs-numpy and therefore vacuous. + assert metric_metal._backend == "metal" + result_metal = metric_metal.compute(complex_signal, n_samp, self.TRANSPOSE_AXES) # Float32 precision — sign() near zero can flip np.testing.assert_allclose(result_metal, result_np, rtol=1e-2, atol=1e-2) @@ -772,7 +715,9 @@ def test_pli_metal_large_channels(self): (2, 1, 256, 256) ) n_samp = sig.shape[3] - result = PLI(optimization="metal").compute(sig, n_samp, self.TRANSPOSE_AXES) + metric = PLI(optimization="metal") + assert metric._backend == "metal" + result = metric.compute(sig, n_samp, self.TRANSPOSE_AXES) assert result.shape == (2, 1, 256, 256) assert not np.any(np.isnan(result)) assert np.allclose(np.diagonal(result[0, 0]), 0) # diagonal = 0 @@ -956,20 +901,6 @@ def test_envcorr_torch_vs_numpy(self, complex_signal): else: np.testing.assert_allclose(result_torch, result_np, rtol=1e-9, atol=1e-10) - @pytest.mark.skipif(not METAL_AVAILABLE, reason="Metal not available") - def test_envcorr_metal_vs_numpy(self, complex_signal): - """Metal EnvCorr should match numpy EnvCorr within float32 tolerance.""" - from hypyp.sync.envelope_corr import EnvCorr - - n_samp = complex_signal.shape[3] - result_np = EnvCorr(optimization=None).compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - result_metal = EnvCorr(optimization="metal").compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - np.testing.assert_allclose(result_metal, result_np, rtol=1e-5, atol=1e-5) - @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") def test_envcorr_cuda_vs_numpy(self, complex_signal): """CUDA EnvCorr should match numpy EnvCorr exactly (both float64).""" @@ -1053,20 +984,6 @@ def test_powcorr_torch_vs_numpy(self, complex_signal): else: np.testing.assert_allclose(result_torch, result_np, rtol=1e-9, atol=1e-10) - @pytest.mark.skipif(not METAL_AVAILABLE, reason="Metal not available") - def test_powcorr_metal_vs_numpy(self, complex_signal): - """Metal PowCorr should match numpy PowCorr within float32 tolerance.""" - from hypyp.sync.pow_corr import PowCorr - - n_samp = complex_signal.shape[3] - result_np = PowCorr(optimization=None).compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - result_metal = PowCorr(optimization="metal").compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) - np.testing.assert_allclose(result_metal, result_np, rtol=1e-5, atol=1e-5) - @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") def test_powcorr_cuda_vs_numpy(self, complex_signal): """CUDA PowCorr should match numpy PowCorr exactly (both float64).""" @@ -1090,9 +1007,9 @@ def test_wpli_metal_vs_numpy(self, complex_signal): result_np = WPLI(optimization=None).compute( complex_signal, n_samp, self.TRANSPOSE_AXES ) - result_metal = WPLI(optimization="metal").compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) + metric_metal = WPLI(optimization="metal") + assert metric_metal._backend == "metal" + result_metal = metric_metal.compute(complex_signal, n_samp, self.TRANSPOSE_AXES) np.testing.assert_allclose(result_metal, result_np, rtol=1e-2, atol=1e-2) @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") @@ -1124,9 +1041,9 @@ def test_accorr_metal_vs_numpy(self, complex_signal): result_np = ACCorr(optimization=None, show_progress=False).compute( complex_signal, n_samp, self.TRANSPOSE_AXES ) - result_metal = ACCorr(optimization="metal", show_progress=False).compute( - complex_signal, n_samp, self.TRANSPOSE_AXES - ) + metric_metal = ACCorr(optimization="metal", show_progress=False) + assert metric_metal._backend == "metal" + result_metal = metric_metal.compute(complex_signal, n_samp, self.TRANSPOSE_AXES) np.testing.assert_allclose(result_metal, result_np, rtol=1e-5, atol=1e-5) @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") From 795b6a2f17a06e8a4bb9f904ea1158b5d00d25d2 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sat, 3 Oct 2026 22:32:38 -0400 Subject: [PATCH 03/14] fix(sync): explain skipped backends, name the metric on a bad backend, keep the subclass contract Three adjustments on top of the capability check, from the October audit. - In the priority path, a backend skipped for lack of an implementation was reported as "No GPU backend available", which is false on a machine that has one. The fallback warning now names the backend the metric lacks. - An unknown or unimplemented backend at dispatch raised a bare KeyError (or an AttributeError). compute() now raises a ValueError naming the metric, the offending backend and the backends the metric implements. - _compute_numpy is no longer abstract. BaseMetric is public and its earlier contract was "override compute"; a third-party metric written that way could no longer be instantiated. The default now raises NotImplementedError when neither method is provided. No computed value changes. Changelog entries added for the whole branch. Refs #299, #300 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 10 ++++++ hypyp/sync/base.py | 64 ++++++++++++++++++++++++++++---------- tests/test_sync.py | 77 +++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 133 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ea3067..5839618 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,17 @@ ## [Unreleased] +### Fixed +- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. `optimization='metal'` on these metrics now warns and falls back to NumPy, and a `priority` list skips Metal and moves on to its next backend. Only PLI, wPLI and ACCorr have a Metal kernel; `optimization='auto'` was already correct. No computed value changes (#299) +- `hypyp.sync`: when every backend of a `priority` list is skipped or unavailable, the warning now gives the real reason (for example "'plv' has no Metal implementation") instead of "No GPU backend available" +- `hypyp.sync`: a metric whose backend is unknown or not implemented now raises a `ValueError` naming the metric and the backends it implements, instead of computing in NumPy without notice +- Tests: the six Metal tests that compared NumPy with NumPy are removed, and the four tests that do run a Metal kernel now assert that it ran (#300) + +### Added +- `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` + ### Changed +- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric. A subclass that overrides `compute` only, as before, still works - The whole code base is formatted with `ruff format` (line length 88). The change is purely cosmetic: the syntax tree of every file is unchanged, apart from whitespace inside docstrings, and the Python examples of `hypyp/sync/README.md` are formatted too. The vendored `hypyp/ext` and the tutorial notebooks are left untouched. The formatting commit is listed in `.git-blame-ignore-revs`, so `git blame` skips it (run `git config blame.ignoreRevsFile .git-blame-ignore-revs` once in your clone; GitHub applies it automatically) - The CI now checks formatting (`ruff format --check`) and a small set of lint rules that only catch certain bugs: syntax errors, invalid comparisons and undefined names. Ruff comes from the new `lint` dependency group, which the `dev` group includes - `black` is removed from the `dev` dependency group, since `ruff format` replaces it diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 29020e4..1542e77 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -8,9 +8,9 @@ This module is shared by every concrete metric in ``hypyp.sync``. It exposes: -- ``BaseMetric`` — abstract base. Concrete metrics override - ``BaseMetric.compute`` and rely on the shared backend-resolution and - warning-fallback logic. +- ``BaseMetric`` — base class. Concrete metrics implement the + ``_compute_*`` methods (``_compute_numpy`` at least) and rely on the + shared backend-resolution, warning-fallback and dispatch logic. - ``multiply_conjugate``, ``multiply_conjugate_time``, ``multiply_product`` — vectorised einsum kernels (numpy). - ``multiply_conjugate_torch``, ``multiply_conjugate_time_torch`` — @@ -43,7 +43,7 @@ """ import warnings -from abc import ABC, abstractmethod +from abc import ABC from typing import Optional import numpy as np @@ -305,7 +305,7 @@ class BaseMetric(ABC): #: Maps a backend name to the method implementing it. This table is the #: single source of truth for dispatch: ``compute`` looks the backend up - #: here, so an unrecognised backend raises ``KeyError`` instead of silently + #: here, so an unrecognised backend raises ``ValueError`` instead of silently #: falling through to numpy, and ``supports`` derives capability from the #: methods a subclass actually defines rather than from a hand-kept list. _BACKEND_METHODS = { @@ -539,11 +539,16 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: if priority is None: priority = AUTO_PRIORITY.get(cls.name, {}).get(platform, []) + # Backends of the priority list this metric has no implementation for, + # remembered so the fallback warning can give the real reason. + unimplemented = [] for backend in priority: # Skip a backend this metric has no implementation for, so a # priority list falls through to the next candidate instead of # selecting a backend that would degrade to numpy at dispatch. if not cls.supports(backend): + if backend in cls._BACKEND_LABELS: + unimplemented.append(cls._BACKEND_LABELS[backend]) continue if backend == "torch" and TORCH_AVAILABLE: return cls._resolve_torch() @@ -552,10 +557,21 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: if backend == "cuda_kernel" and CUPY_AVAILABLE: return "cuda_kernel", "cuda" - # No GPU backend from priority list available — fall back + # No GPU backend from priority list available — fall back. When a + # backend was skipped for lack of an implementation, say so: "no GPU + # backend available" would be false on a machine that has one. + if unimplemented: + reason = ( + f"{cls.name!r} has no {' or '.join(unimplemented)} " + f"implementation, and no other backend of the priority list " + f"is available on platform '{platform}'." + ) + else: + reason = ( + f"No GPU backend available for {cls.name!r} on platform '{platform}'." + ) warnings.warn( - f"No GPU backend available for {cls.name!r} on platform " - f"'{platform}'. Falling back to CPU.", + f"{reason} Falling back to CPU.", UserWarning, stacklevel=4, ) @@ -603,7 +619,9 @@ def compute( Dispatch is table-driven via ``_BACKEND_METHODS``: the backend chosen at construction selects the ``_compute_*`` method to run. Subclasses - implement those methods and do not override this one. + normally implement those methods and leave this one alone; a subclass + that overrides ``compute`` itself bypasses the dispatch and is + responsible for honouring ``self._backend``. Parameters ---------- @@ -621,9 +639,10 @@ def compute( Raises ------ - KeyError - If ``self._backend`` is not a known backend name. This is - deliberate: an earlier hand-written ``if/elif`` chain per metric + ValueError + If ``self._backend`` is not a backend this metric implements. The + message names the metric and the backends it does implement. This + is deliberate: an earlier hand-written ``if/elif`` chain per metric ended in a bare ``return self._compute_numpy(...)``, so an unhandled backend was indistinguishable from the numpy default and failed silently. Dispatching through the table fails loudly instead. @@ -633,20 +652,27 @@ def compute( Output dtype follows the backend: numpy, numba and CUDA return ``float64``; the Metal kernels return ``float32``. """ - method_name = self._BACKEND_METHODS[self._backend] - method = getattr(self, method_name) + if not self.supports(self._backend): + implemented = [b for b in self._BACKEND_METHODS if self.supports(b)] + raise ValueError( + f"{self.name!r} cannot run on backend {self._backend!r}. " + f"Backends implemented for this metric: {implemented}." + ) + method = getattr(self, self._BACKEND_METHODS[self._backend]) return method(complex_signal, n_samp, transpose_axes) - @abstractmethod def _compute_numpy( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple ) -> np.ndarray: """ Reference implementation, in numpy. Always available. - Every metric must provide this: it is the correctness oracle the + Every metric should provide this: it is the correctness oracle the accelerated backends are validated against, and the fallback target - whenever a requested backend is unavailable or unimplemented. + whenever a requested backend is unavailable or unimplemented. It is + deliberately not an abstract method, so that a subclass written + against the earlier contract (overriding ``compute`` only) can still + be instantiated; this default raises ``NotImplementedError``. Parameters ---------- @@ -662,3 +688,7 @@ def _compute_numpy( con : np.ndarray Connectivity matrix with shape (n_epoch, n_freq, 2*n_ch, 2*n_ch). """ + raise NotImplementedError( + f"{type(self).__name__} must implement _compute_numpy " + f"(or override compute)." + ) diff --git a/tests/test_sync.py b/tests/test_sync.py index b324159..694e144 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1245,5 +1245,80 @@ def test_unknown_backend_fails_closed(self, complex_signal): n_samp = complex_signal.shape[3] metric = PLV() metric._backend = "not_a_backend" - with pytest.raises(KeyError): + with pytest.raises(ValueError) as excinfo: metric.compute(complex_signal, n_samp, (0, 1, 3, 2)) + # The error must be usable: it names the metric, the offending + # backend and the backends that do exist for this metric. + message = str(excinfo.value) + assert "'plv'" in message + assert "'not_a_backend'" in message + assert "numpy" in message + + def test_unimplemented_backend_fails_closed(self, complex_signal): + """ + A known backend the metric does not implement must raise the same + clear error, not an AttributeError on the missing method. + """ + from hypyp.sync.plv import PLV + + n_samp = complex_signal.shape[3] + metric = PLV() + metric._backend = "metal" + with pytest.raises(ValueError, match="cannot run on backend 'metal'"): + metric.compute(complex_signal, n_samp, (0, 1, 3, 2)) + + def test_priority_fallback_warning_names_the_skipped_backend(self): + """ + When the only backend of a priority list has no implementation, the + fallback warning must give that reason. + + Before, priority=['metal'] on an einsum metric warned "No GPU backend + available" on a machine where a GPU backend was available, without + mentioning that Metal was skipped for lack of a kernel. + """ + with ( + patch("hypyp.sync.base.METAL_AVAILABLE", True), + patch("hypyp.sync.base.TORCH_AVAILABLE", True), + patch("hypyp.sync.base.MPS_AVAILABLE", True), + ): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + metric = get_metric("plv", optimization="auto", priority=["metal"]) + + assert metric._backend in ("numba", "numpy") + messages = [ + str(w.message) for w in caught if issubclass(w.category, UserWarning) + ] + assert any("'plv' has no Metal implementation" in m for m in messages), ( + f"fallback warning does not explain the skip (messages: {messages})" + ) + assert not any("No GPU backend available" in m for m in messages) + + def test_subclass_overriding_only_compute_still_instantiates(self, complex_signal): + """ + The pre-0.6.2 subclassing contract keeps working: a third-party metric + that overrides ``compute`` alone, without ``_compute_numpy``, can be + instantiated and used. + """ + from hypyp.sync.base import BaseMetric + + class LegacyMetric(BaseMetric): + name = "legacy" + + def compute(self, complex_signal, n_samp, transpose_axes): + return np.zeros(1) + + n_samp = complex_signal.shape[3] + result = LegacyMetric().compute(complex_signal, n_samp, (0, 1, 3, 2)) + assert result.shape == (1,) + + def test_missing_numpy_implementation_is_reported(self, complex_signal): + """A metric with neither ``compute`` nor ``_compute_numpy`` says so.""" + from hypyp.sync.base import BaseMetric + + class EmptyMetric(BaseMetric): + name = "empty" + + n_samp = complex_signal.shape[3] + with pytest.raises(NotImplementedError, match="_compute_numpy"): + EmptyMetric().compute(complex_signal, n_samp, (0, 1, 3, 2)) From 71ffb84838fd566abe7356bab9f5a706f5da2f67 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sat, 3 Oct 2026 22:49:50 -0400 Subject: [PATCH 04/14] fix(sync): keep self-dispatching subclasses working, make fallbacks respect capability Follow-up to the independent review of this branch. - A subclass of the earlier contract, which overrides compute and branches on self._backend itself without any _compute_* method, was refused every accelerated backend with a false "no implementation" warning, and a super().compute() call from it raised. Such a class (own compute, no _compute_numpy) is now trusted with every known backend, as before the capability check, and super().compute() returns None as the former abstract method did. - The CPU fallback of the 'auto' path returned numba whenever numba was installed, without asking the metric. A metric implementing numpy alone then failed at compute. The fallback now goes through supports(). - supports() no longer counts a non-callable attribute, or the default _compute_numpy of the base class, as an implementation. - The priority fallback warning said "no other backend of the priority list is available", which is false when the CPU fallback is numba. It now speaks of GPU backends only. - Tests: compute() is checked to route every metric to every backend it implements, with a stub and no hardware; the four Metal tests spy on _compute_metal instead of trusting _backend; the fallback after a Metal request is checked to compute the numpy result. - Changelog and docstrings: the numerical implementations are unchanged, but a corrected priority list can now select another backend and therefore another precision (torch on MPS is float32). The dtype note in compute() was wrong for complex64 input. Refs #299, #300 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 6 +- hypyp/sync/base.py | 63 +++++++++++++--- tests/test_sync.py | 175 +++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 216 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5839618..0882d7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,16 +3,16 @@ ## [Unreleased] ### Fixed -- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. `optimization='metal'` on these metrics now warns and falls back to NumPy, and a `priority` list skips Metal and moves on to its next backend. Only PLI, wPLI and ACCorr have a Metal kernel; `optimization='auto'` was already correct. No computed value changes (#299) +- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. `optimization='metal'` on these metrics now warns and falls back to NumPy, and a `priority` list skips Metal and moves on to its next backend. Only PLI, wPLI and ACCorr have a Metal kernel; the default selection of `optimization='auto'` is unchanged. No numerical implementation changes, but a request that used to run NumPy without saying so can now run the backend that comes next in its `priority` list: for example `priority=['metal', 'torch']` on PLV now runs torch, which computes in single precision on Apple GPUs (#299) - `hypyp.sync`: when every backend of a `priority` list is skipped or unavailable, the warning now gives the real reason (for example "'plv' has no Metal implementation") instead of "No GPU backend available" - `hypyp.sync`: a metric whose backend is unknown or not implemented now raises a `ValueError` naming the metric and the backends it implements, instead of computing in NumPy without notice -- Tests: the six Metal tests that compared NumPy with NumPy are removed, and the four tests that do run a Metal kernel now assert that it ran (#300) +- Tests: the six Metal tests that compared NumPy with NumPy are replaced by tests of the fallback itself (warning, backend and result), the dispatch of every metric to every backend it implements is checked without a GPU, and the four tests that run a Metal kernel now assert that the Metal method was called (#300) ### Added - `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` ### Changed -- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric. A subclass that overrides `compute` only, as before, still works +- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric. A subclass written the earlier way, which overrides `compute` and does not define `_compute_numpy`, keeps working as before and is not subject to the capability check - The whole code base is formatted with `ruff format` (line length 88). The change is purely cosmetic: the syntax tree of every file is unchanged, apart from whitespace inside docstrings, and the Python examples of `hypyp/sync/README.md` are formatted too. The vendored `hypyp/ext` and the tutorial notebooks are left untouched. The formatting commit is listed in `.git-blame-ignore-revs`, so `git blame` skips it (run `git config blame.ignoreRevsFile .git-blame-ignore-revs` once in your clone; GitHub applies it automatically) - The CI now checks formatting (`ruff format --check`) and a small set of lint rules that only catch certain bugs: syntax errors, invalid comparisons and undefined names. Ruff comes from the new `lint` dependency group, which the `dev` group includes - `black` is removed from the `dev` dependency group, since `ruff format` replaces it diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 1542e77..be96129 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -343,6 +343,11 @@ def supports(cls, backend: str) -> bool: sign-based metrics and ACCorr, because torch on MPS is faster for the einsum metrics at every channel count (see ``AUTO_PRIORITY``). + A subclass written against the earlier contract, which overrides + ``compute`` and branches on ``self._backend`` itself without defining + ``_compute_numpy``, cannot be inspected this way. It is trusted with + every known backend, exactly as before the capability check existed. + Parameters ---------- backend : str @@ -361,7 +366,34 @@ def supports(cls, backend: str) -> bool: (True, False) """ method = cls._BACKEND_METHODS.get(backend) - return method is not None and hasattr(cls, method) + if method is None: + return False + if not cls._uses_table_dispatch(): + return cls._dispatches_itself() + return callable(getattr(cls, method, None)) + + @classmethod + def _uses_table_dispatch(cls) -> bool: + """Whether the class follows the ``_compute_*`` contract. + + The marker is its own ``_compute_numpy``: the reference implementation + is the one method every metric of the current contract provides. + """ + return cls._compute_numpy is not BaseMetric._compute_numpy + + @classmethod + def _dispatches_itself(cls) -> bool: + """Whether the class follows the earlier contract: its own ``compute`` + and no ``_compute_numpy``.""" + return not cls._uses_table_dispatch() and cls.compute is not BaseMetric.compute + + @classmethod + def _cpu_fallback(cls) -> tuple: + """CPU backend used when no GPU backend can be selected: numba when it + is installed and the metric implements it, numpy otherwise.""" + if NUMBA_AVAILABLE and cls.supports("numba"): + return "numba", "cpu" + return "numpy", "cpu" @classmethod def _resolve_optimization( @@ -532,9 +564,7 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: UserWarning, stacklevel=4, ) - if NUMBA_AVAILABLE: - return "numba", "cpu" - return "numpy", "cpu" + return cls._cpu_fallback() if priority is None: priority = AUTO_PRIORITY.get(cls.name, {}).get(platform, []) @@ -563,7 +593,7 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: if unimplemented: reason = ( f"{cls.name!r} has no {' or '.join(unimplemented)} " - f"implementation, and no other backend of the priority list " + f"implementation, and no GPU backend of the priority list " f"is available on platform '{platform}'." ) else: @@ -575,9 +605,7 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: UserWarning, stacklevel=4, ) - if NUMBA_AVAILABLE: - return "numba", "cpu" - return "numpy", "cpu" + return cls._cpu_fallback() @staticmethod def _resolve_torch() -> tuple: @@ -639,6 +667,9 @@ def compute( Raises ------ + NotImplementedError + If the metric provides neither ``_compute_numpy`` nor its own + ``compute``. ValueError If ``self._backend`` is not a backend this metric implements. The message names the metric and the backends it does implement. This @@ -649,9 +680,21 @@ def compute( Notes ----- - Output dtype follows the backend: numpy, numba and CUDA return - ``float64``; the Metal kernels return ``float32``. + Output precision follows the backend and the input: the Metal kernels + and torch on MPS compute in ``float32`` whatever the input, while the + other backends follow the precision of ``complex_signal``. + + A subclass of the earlier contract may call ``super().compute(...)`` + from its own ``compute``. The base method was then abstract with an + empty body and returned ``None``; it still does for such a subclass. """ + if not self._uses_table_dispatch(): + if self._dispatches_itself(): + return None + raise NotImplementedError( + f"{type(self).__name__} must implement _compute_numpy " + f"(or override compute)." + ) if not self.supports(self._backend): implemented = [b for b in self._BACKEND_METHODS if self.supports(b)] raise ValueError( diff --git a/tests/test_sync.py b/tests/test_sync.py index 694e144..e95f8ed 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -26,6 +26,19 @@ from tests.accorr_reference import accorr_reference +def spy_on(cls, method_name): + """ + Patch ``cls.`` with a spy that still runs the real method. + + Asserting ``metric._backend == 'metal'`` only proves what the metric + reports. The spy proves that ``compute`` really went through the method of + that backend, which is what a broken dispatch would get wrong. + """ + return patch.object( + cls, method_name, autospec=True, side_effect=getattr(cls, method_name) + ) + + class TestAccorrReference: """Basic properties of the reference implementation.""" @@ -701,7 +714,11 @@ def test_pli_metal_vs_numpy(self, complex_signal): # Assert the kernel actually ran: a silent fallback to numpy would make # this comparison numpy-vs-numpy and therefore vacuous. assert metric_metal._backend == "metal" - result_metal = metric_metal.compute(complex_signal, n_samp, self.TRANSPOSE_AXES) + with spy_on(PLI, "_compute_metal") as spy: + result_metal = metric_metal.compute( + complex_signal, n_samp, self.TRANSPOSE_AXES + ) + assert spy.call_count == 1 # Float32 precision — sign() near zero can flip np.testing.assert_allclose(result_metal, result_np, rtol=1e-2, atol=1e-2) @@ -717,7 +734,9 @@ def test_pli_metal_large_channels(self): n_samp = sig.shape[3] metric = PLI(optimization="metal") assert metric._backend == "metal" - result = metric.compute(sig, n_samp, self.TRANSPOSE_AXES) + with spy_on(PLI, "_compute_metal") as spy: + result = metric.compute(sig, n_samp, self.TRANSPOSE_AXES) + assert spy.call_count == 1 assert result.shape == (2, 1, 256, 256) assert not np.any(np.isnan(result)) assert np.allclose(np.diagonal(result[0, 0]), 0) # diagonal = 0 @@ -1009,7 +1028,11 @@ def test_wpli_metal_vs_numpy(self, complex_signal): ) metric_metal = WPLI(optimization="metal") assert metric_metal._backend == "metal" - result_metal = metric_metal.compute(complex_signal, n_samp, self.TRANSPOSE_AXES) + with spy_on(WPLI, "_compute_metal") as spy: + result_metal = metric_metal.compute( + complex_signal, n_samp, self.TRANSPOSE_AXES + ) + assert spy.call_count == 1 np.testing.assert_allclose(result_metal, result_np, rtol=1e-2, atol=1e-2) @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") @@ -1043,7 +1066,11 @@ def test_accorr_metal_vs_numpy(self, complex_signal): ) metric_metal = ACCorr(optimization="metal", show_progress=False) assert metric_metal._backend == "metal" - result_metal = metric_metal.compute(complex_signal, n_samp, self.TRANSPOSE_AXES) + with spy_on(ACCorr, "_compute_metal") as spy: + result_metal = metric_metal.compute( + complex_signal, n_samp, self.TRANSPOSE_AXES + ) + assert spy.call_count == 1 np.testing.assert_allclose(result_metal, result_np, rtol=1e-5, atol=1e-5) @pytest.mark.skipif(not CUPY_AVAILABLE, reason="CuPy not available") @@ -1186,7 +1213,7 @@ def test_metal_capability_matches_documented_support_matrix(self): assert actual == self.METAL_CAPABLE @pytest.mark.parametrize("mode", sorted(METRICS)) - def test_metal_request_never_silently_degrades(self, mode): + def test_metal_request_never_silently_degrades(self, mode, complex_signal): """ optimization='metal' either resolves to metal, or warns and uses numpy. @@ -1213,6 +1240,42 @@ def test_metal_request_never_silently_degrades(self, mode): assert any("Metal" in m for m in messages), ( f"{mode}: degraded to numpy without warning (messages: {messages})" ) + # The fallback must also compute: same result as a plain numpy + # metric, through the numpy method. + n_samp = complex_signal.shape[3] + axes = (0, 1, 3, 2) + expected = get_metric(mode).compute(complex_signal, n_samp, axes) + with spy_on(type(metric), "_compute_numpy") as spy: + result = metric.compute(complex_signal, n_samp, axes) + assert spy.call_count == 1 + np.testing.assert_array_equal(result, expected) + + @pytest.mark.parametrize( + "mode, backend", + [ + (mode, backend) + for mode in sorted(METRICS) + for backend in METRICS[mode]._BACKEND_METHODS + if METRICS[mode].supports(backend) + ], + ) + def test_compute_routes_to_the_method_of_the_backend(self, mode, backend): + """ + compute() must call the ``_compute_*`` method of the resolved backend, + with its arguments, and return its result, for every backend each + metric implements. No hardware is needed: the method is replaced by a + stub. + """ + cls = METRICS[mode] + method_name = cls._BACKEND_METHODS[backend] + metric = cls() + metric._backend = backend + sentinel = object() + with patch.object(cls, method_name, autospec=True) as stub: + stub.return_value = sentinel + result = metric.compute("signal", 7, (0, 1, 3, 2)) + assert result is sentinel + stub.assert_called_once_with(metric, "signal", 7, (0, 1, 3, 2)) @pytest.mark.parametrize("mode", sorted(set(METRICS) - {"pli", "wpli", "accorr"})) def test_auto_priority_skips_unsupported_backend(self, mode): @@ -1293,24 +1356,106 @@ def test_priority_fallback_warning_names_the_skipped_backend(self): f"fallback warning does not explain the skip (messages: {messages})" ) assert not any("No GPU backend available" in m for m in messages) - - def test_subclass_overriding_only_compute_still_instantiates(self, complex_signal): - """ - The pre-0.6.2 subclassing contract keeps working: a third-party metric - that overrides ``compute`` alone, without ``_compute_numpy``, can be - instantiated and used. - """ + # The message must stay true when the CPU fallback is numba: it speaks + # of GPU backends only. + assert any("no GPU backend of the priority list" in m for m in messages) + + @staticmethod + def _legacy_metric(): + """A third-party metric written against the pre-0.6.2 contract: it + overrides ``compute``, branches on ``self._backend`` itself and has no + ``_compute_*`` method.""" from hypyp.sync.base import BaseMetric class LegacyMetric(BaseMetric): name = "legacy" def compute(self, complex_signal, n_samp, transpose_axes): - return np.zeros(1) + base_result = super().compute(complex_signal, n_samp, transpose_axes) + return self._backend, base_result + + return LegacyMetric + + @pytest.mark.parametrize( + "kwargs, expected", + [ + (dict(optimization=None), "numpy"), + (dict(optimization="numba"), "numba"), + (dict(optimization="torch"), "torch"), + (dict(optimization="metal"), "metal"), + (dict(optimization="auto", priority=["metal", "torch"]), "metal"), + (dict(optimization="auto", priority=["torch"]), "torch"), + ], + ) + def test_legacy_subclass_keeps_its_own_dispatch(self, kwargs, expected): + """ + A subclass of the earlier contract is granted the backend it asks for, + as before the capability check, and without a "no implementation" + warning: the base class cannot see inside its ``compute``. + """ + legacy_cls = self._legacy_metric() + with ( + patch("hypyp.sync.base.METAL_AVAILABLE", True), + patch("hypyp.sync.base.TORCH_AVAILABLE", True), + patch("hypyp.sync.base.MPS_AVAILABLE", True), + patch("hypyp.sync.base.NUMBA_AVAILABLE", True), + ): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + metric = legacy_cls(**kwargs) + assert metric._backend == expected + assert not any("implementation" in str(w.message) for w in caught) + # super().compute() used to be an abstract method with an empty body: + # it returned None and must still do so, not raise. + assert metric.compute(None, 0, None) == (expected, None) + + @pytest.mark.parametrize("gpu", [True, False]) + def test_numpy_only_metric_never_gets_numba(self, gpu, complex_signal): + """ + A metric of the current contract that implements numpy alone must + resolve to numpy under 'auto', even when numba is installed: the CPU + fallback has to respect capability like every other path. + """ + from hypyp.sync.base import BaseMetric + + class NumpyOnly(BaseMetric): + name = "numpy_only" + + def _compute_numpy(self, complex_signal, n_samp, transpose_axes): + return np.ones(1) + with ( + patch("hypyp.sync.base.NUMBA_AVAILABLE", True), + patch("hypyp.sync.base.TORCH_AVAILABLE", gpu), + patch("hypyp.sync.base.MPS_AVAILABLE", gpu), + patch("hypyp.sync.base.CUDA_AVAILABLE", False), + ): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + metric = NumpyOnly(optimization="auto") + assert metric._backend == "numpy" n_samp = complex_signal.shape[3] - result = LegacyMetric().compute(complex_signal, n_samp, (0, 1, 3, 2)) - assert result.shape == (1,) + assert metric.compute(complex_signal, n_samp, (0, 1, 3, 2)).shape == (1,) + + def test_supports_ignores_placeholders(self): + """supports() must not count a non-callable attribute, nor the default + ``_compute_numpy`` of the base class, as an implementation.""" + from hypyp.sync.base import BaseMetric + + class Placeholder(BaseMetric): + name = "placeholder" + _compute_torch = None + + def _compute_numpy(self, complex_signal, n_samp, transpose_axes): + return np.ones(1) + + class EmptyMetric(BaseMetric): + name = "empty" + + assert Placeholder.supports("numpy") is True + assert Placeholder.supports("torch") is False + assert EmptyMetric.supports("numpy") is False + assert BaseMetric.supports("numpy") is False def test_missing_numpy_implementation_is_reported(self, complex_signal): """A metric with neither ``compute`` nor ``_compute_numpy`` says so.""" From 6814084ae4022dfc27f62a3992448bbcba44abe1 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sat, 3 Oct 2026 23:00:16 -0400 Subject: [PATCH 05/14] fix(sync): make the dispatch contract explicit, test capability against a hand-written matrix Second follow-up to the independent review. - Whether a class dispatches through the table was inferred from the presence of its own _compute_numpy, which misread an earlier-contract subclass that happened to have a helper of that name, and a current-contract metric without numpy. It is now explicit: the class attribute _dispatch_via_table, set on the nine built-in metrics. Left unset, it is inferred from whether the class overrides compute, which was the earlier contract. - Tests: the capability matrix is written by hand and supports() is checked against it, so the routing cases no longer depend on the function under test; the Metal tests spy on the kernel functions themselves rather than on the method that calls them; earlier-contract subclasses are tested with and without a _compute_numpy helper. - Docstrings and changelog: the precision note no longer generalises beyond what was measured, the class docstring describes the current contract, and the changelog limits its claims to the built-in metrics and to the warning that was actually corrected. Refs #299, #300 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 6 +-- hypyp/sync/accorr.py | 1 + hypyp/sync/base.py | 83 +++++++++++++++++++------------ hypyp/sync/ccorr.py | 1 + hypyp/sync/coh.py | 1 + hypyp/sync/envelope_corr.py | 1 + hypyp/sync/imaginary_coh.py | 1 + hypyp/sync/pli.py | 1 + hypyp/sync/plv.py | 1 + hypyp/sync/pow_corr.py | 1 + hypyp/sync/wpli.py | 1 + tests/test_sync.py | 99 ++++++++++++++++++++++++++++++++----- 12 files changed, 149 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0882d7b..8f1d8c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,15 +4,15 @@ ### Fixed - `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. `optimization='metal'` on these metrics now warns and falls back to NumPy, and a `priority` list skips Metal and moves on to its next backend. Only PLI, wPLI and ACCorr have a Metal kernel; the default selection of `optimization='auto'` is unchanged. No numerical implementation changes, but a request that used to run NumPy without saying so can now run the backend that comes next in its `priority` list: for example `priority=['metal', 'torch']` on PLV now runs torch, which computes in single precision on Apple GPUs (#299) -- `hypyp.sync`: when every backend of a `priority` list is skipped or unavailable, the warning now gives the real reason (for example "'plv' has no Metal implementation") instead of "No GPU backend available" -- `hypyp.sync`: a metric whose backend is unknown or not implemented now raises a `ValueError` naming the metric and the backends it implements, instead of computing in NumPy without notice +- `hypyp.sync`: when a backend of a `priority` list was skipped because the metric does not implement it and no other GPU backend of the list could be used, the warning now says so (for example "'plv' has no Metal implementation") instead of "No GPU backend available" +- `hypyp.sync`: a built-in metric whose backend is unknown or not implemented now raises a `ValueError` naming the metric and the backends it implements, instead of computing in NumPy without notice. Called through `compute_sync`, that error is still reworded as an unsupported metric (#306) - Tests: the six Metal tests that compared NumPy with NumPy are replaced by tests of the fallback itself (warning, backend and result), the dispatch of every metric to every backend it implements is checked without a GPU, and the four tests that run a Metal kernel now assert that the Metal method was called (#300) ### Added - `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` ### Changed -- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric. A subclass written the earlier way, which overrides `compute` and does not define `_compute_numpy`, keeps working as before and is not subject to the capability check +- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, keeps working as before and is not subject to the capability check - The whole code base is formatted with `ruff format` (line length 88). The change is purely cosmetic: the syntax tree of every file is unchanged, apart from whitespace inside docstrings, and the Python examples of `hypyp/sync/README.md` are formatted too. The vendored `hypyp/ext` and the tutorial notebooks are left untouched. The formatting commit is listed in `.git-blame-ignore-revs`, so `git blame` skips it (run `git config blame.ignoreRevsFile .git-blame-ignore-revs` once in your clone; GitHub applies it automatically) - The CI now checks formatting (`ruff format --check`) and a small set of lint rules that only catch certain bugs: syntax errors, invalid comparisons and undefined names. Ruff comes from the new `lint` dependency group, which the `dev` group includes - `black` is removed from the `dev` dependency group, since `ruff format` replaces it diff --git a/hypyp/sync/accorr.py b/hypyp/sync/accorr.py index 0c58320..b4a9d64 100644 --- a/hypyp/sync/accorr.py +++ b/hypyp/sync/accorr.py @@ -79,6 +79,7 @@ class ACCorr(BaseMetric): """ name = "accorr" + _dispatch_via_table = True def __init__( self, diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index be96129..2ddf2e4 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -281,8 +281,14 @@ class BaseMetric(ABC): """ Abstract base class for connectivity metrics. - All connectivity metrics should inherit from this class and implement - the compute method. + A metric inherits from this class, sets ``_dispatch_via_table = True`` and + implements ``_compute_numpy`` plus any of the optional ``_compute_numba``, + ``_compute_torch``, ``_compute_metal`` and ``_compute_cuda``. Backend + selection, capability checks and dispatch are then handled here. + + A subclass that overrides ``compute`` and does not set the flag follows the + earlier contract: it is granted whatever backend is requested and + available, and is itself responsible for honouring ``self._backend``. Parameters ---------- @@ -303,6 +309,13 @@ class BaseMetric(ABC): name: str = "base" + #: Whether ``compute`` dispatches through ``_BACKEND_METHODS`` and backend + #: selection checks which ``_compute_*`` methods exist. ``None`` means + #: "inferred": yes, unless the class overrides ``compute``. The built-in + #: metrics override ``compute`` only to carry a docstring, so they set + #: the flag explicitly; so should a new metric written the same way. + _dispatch_via_table: Optional[bool] = None + #: Maps a backend name to the method implementing it. This table is the #: single source of truth for dispatch: ``compute`` looks the backend up #: here, so an unrecognised backend raises ``ValueError`` instead of silently @@ -344,9 +357,10 @@ def supports(cls, backend: str) -> bool: einsum metrics at every channel count (see ``AUTO_PRIORITY``). A subclass written against the earlier contract, which overrides - ``compute`` and branches on ``self._backend`` itself without defining - ``_compute_numpy``, cannot be inspected this way. It is trusted with - every known backend, exactly as before the capability check existed. + ``compute`` and branches on ``self._backend`` itself, cannot be + inspected this way. Unless it sets ``_dispatch_via_table``, it is + trusted with every known backend, exactly as before the capability + check existed. Parameters ---------- @@ -369,23 +383,25 @@ def supports(cls, backend: str) -> bool: if method is None: return False if not cls._uses_table_dispatch(): - return cls._dispatches_itself() - return callable(getattr(cls, method, None)) + return True + implementation = getattr(cls, method, None) + # The default _compute_numpy of this class only raises: it is not an + # implementation. + return callable(implementation) and ( + implementation is not BaseMetric._compute_numpy + ) @classmethod def _uses_table_dispatch(cls) -> bool: - """Whether the class follows the ``_compute_*`` contract. + """Whether this class dispatches through ``_BACKEND_METHODS``. - The marker is its own ``_compute_numpy``: the reference implementation - is the one method every metric of the current contract provides. + Explicit when ``_dispatch_via_table`` is set. Otherwise inferred: a + class that overrides ``compute`` is taken to do its own dispatch, as + the contract was before ``compute`` became concrete. """ - return cls._compute_numpy is not BaseMetric._compute_numpy - - @classmethod - def _dispatches_itself(cls) -> bool: - """Whether the class follows the earlier contract: its own ``compute`` - and no ``_compute_numpy``.""" - return not cls._uses_table_dispatch() and cls.compute is not BaseMetric.compute + if cls._dispatch_via_table is not None: + return cls._dispatch_via_table + return cls.compute is BaseMetric.compute @classmethod def _cpu_fallback(cls) -> tuple: @@ -680,22 +696,25 @@ def compute( Notes ----- - Output precision follows the backend and the input: the Metal kernels - and torch on MPS compute in ``float32`` whatever the input, while the - other backends follow the precision of ``complex_signal``. - - A subclass of the earlier contract may call ``super().compute(...)`` - from its own ``compute``. The base method was then abstract with an - empty body and returned ``None``; it still does for such a subclass. + Output precision depends on the backend and on the metric. The Metal + kernels and torch on MPS compute in ``float32`` whatever the input; + for the other combinations see each metric. + + A subclass that does its own dispatch (see ``_dispatch_via_table``) + may call ``super().compute(...)`` from its own ``compute``. The base + method was then abstract with an empty body and returned ``None``; it + still does for such a subclass. """ if not self._uses_table_dispatch(): - if self._dispatches_itself(): - return None - raise NotImplementedError( - f"{type(self).__name__} must implement _compute_numpy " - f"(or override compute)." - ) + # Reached through super().compute() from a subclass that does its + # own dispatch: behave as the former abstract method did. + return None if not self.supports(self._backend): + if self._backend == "numpy": + raise NotImplementedError( + f"{type(self).__name__} must implement _compute_numpy " + f"(or override compute)." + ) implemented = [b for b in self._BACKEND_METHODS if self.supports(b)] raise ValueError( f"{self.name!r} cannot run on backend {self._backend!r}. " @@ -714,8 +733,8 @@ def _compute_numpy( accelerated backends are validated against, and the fallback target whenever a requested backend is unavailable or unimplemented. It is deliberately not an abstract method, so that a subclass written - against the earlier contract (overriding ``compute`` only) can still - be instantiated; this default raises ``NotImplementedError``. + against the earlier contract (its own ``compute``) can still be + instantiated; this default raises ``NotImplementedError``. Parameters ---------- diff --git a/hypyp/sync/ccorr.py b/hypyp/sync/ccorr.py index 33e3843..262745e 100644 --- a/hypyp/sync/ccorr.py +++ b/hypyp/sync/ccorr.py @@ -38,6 +38,7 @@ class CCorr(BaseMetric): """ name = "ccorr" + _dispatch_via_table = True def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/coh.py b/hypyp/sync/coh.py index 2bbea53..ee2b334 100644 --- a/hypyp/sync/coh.py +++ b/hypyp/sync/coh.py @@ -44,6 +44,7 @@ class Coh(BaseMetric): """ name = "coh" + _dispatch_via_table = True def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/envelope_corr.py b/hypyp/sync/envelope_corr.py index 1d9cc11..45da243 100644 --- a/hypyp/sync/envelope_corr.py +++ b/hypyp/sync/envelope_corr.py @@ -47,6 +47,7 @@ class EnvCorr(BaseMetric): """ name = "envcorr" + _dispatch_via_table = True def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/imaginary_coh.py b/hypyp/sync/imaginary_coh.py index 27a58dc..54a5b07 100644 --- a/hypyp/sync/imaginary_coh.py +++ b/hypyp/sync/imaginary_coh.py @@ -46,6 +46,7 @@ class ImCoh(BaseMetric): """ name = "imcoh" + _dispatch_via_table = True def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/pli.py b/hypyp/sync/pli.py index fcb0925..eec2f96 100644 --- a/hypyp/sync/pli.py +++ b/hypyp/sync/pli.py @@ -48,6 +48,7 @@ class PLI(BaseMetric): """ name = "pli" + _dispatch_via_table = True def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/plv.py b/hypyp/sync/plv.py index b909f91..682b3b2 100644 --- a/hypyp/sync/plv.py +++ b/hypyp/sync/plv.py @@ -44,6 +44,7 @@ class PLV(BaseMetric): """ name = "plv" + _dispatch_via_table = True def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/pow_corr.py b/hypyp/sync/pow_corr.py index 356f8fb..bfffcc1 100644 --- a/hypyp/sync/pow_corr.py +++ b/hypyp/sync/pow_corr.py @@ -49,6 +49,7 @@ class PowCorr(BaseMetric): """ name = "powcorr" + _dispatch_via_table = True def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/hypyp/sync/wpli.py b/hypyp/sync/wpli.py index e591196..7f0f627 100644 --- a/hypyp/sync/wpli.py +++ b/hypyp/sync/wpli.py @@ -47,6 +47,7 @@ class WPLI(BaseMetric): """ name = "wpli" + _dispatch_via_table = True def compute( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple diff --git a/tests/test_sync.py b/tests/test_sync.py index e95f8ed..2435646 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -26,6 +26,40 @@ from tests.accorr_reference import accorr_reference +#: Written by hand on purpose: the tests below must not derive their cases +#: from supports(), the function they check. +EXPECTED_BACKENDS = { + mode: {"numpy", "numba", "torch", "cuda_kernel"} + | ({"metal"} if mode in {"pli", "wpli", "accorr"} else set()) + for mode in ( + "plv", + "ccorr", + "accorr", + "coh", + "imcoh", + "pli", + "wpli", + "envcorr", + "powcorr", + ) +} +ALL_BACKENDS = ("numpy", "numba", "torch", "metal", "cuda_kernel") + + +def spy_on_kernel(module_name, function_name): + """ + Patch a kernel function of ``hypyp.sync.kernels`` with a spy that still + runs it, to prove the kernel itself was entered. The metrics import their + kernel inside the method, so patching the module attribute is seen. + """ + import importlib + + module = importlib.import_module(f"hypyp.sync.kernels.{module_name}") + return patch.object( + module, function_name, side_effect=getattr(module, function_name) + ) + + def spy_on(cls, method_name): """ Patch ``cls.`` with a spy that still runs the real method. @@ -711,10 +745,11 @@ def test_pli_metal_vs_numpy(self, complex_signal): complex_signal, n_samp, self.TRANSPOSE_AXES ) metric_metal = PLI(optimization="metal") - # Assert the kernel actually ran: a silent fallback to numpy would make - # this comparison numpy-vs-numpy and therefore vacuous. + # A silent fallback to numpy would make this comparison numpy-vs-numpy + # and therefore vacuous: check the backend, then that the Metal kernel + # function itself is entered. assert metric_metal._backend == "metal" - with spy_on(PLI, "_compute_metal") as spy: + with spy_on_kernel("metal_phase", "pli_metal") as spy: result_metal = metric_metal.compute( complex_signal, n_samp, self.TRANSPOSE_AXES ) @@ -734,7 +769,7 @@ def test_pli_metal_large_channels(self): n_samp = sig.shape[3] metric = PLI(optimization="metal") assert metric._backend == "metal" - with spy_on(PLI, "_compute_metal") as spy: + with spy_on_kernel("metal_phase", "pli_metal") as spy: result = metric.compute(sig, n_samp, self.TRANSPOSE_AXES) assert spy.call_count == 1 assert result.shape == (2, 1, 256, 256) @@ -1028,7 +1063,7 @@ def test_wpli_metal_vs_numpy(self, complex_signal): ) metric_metal = WPLI(optimization="metal") assert metric_metal._backend == "metal" - with spy_on(WPLI, "_compute_metal") as spy: + with spy_on_kernel("metal_phase", "wpli_metal") as spy: result_metal = metric_metal.compute( complex_signal, n_samp, self.TRANSPOSE_AXES ) @@ -1066,7 +1101,7 @@ def test_accorr_metal_vs_numpy(self, complex_signal): ) metric_metal = ACCorr(optimization="metal", show_progress=False) assert metric_metal._backend == "metal" - with spy_on(ACCorr, "_compute_metal") as spy: + with spy_on_kernel("metal_accorr", "accorr_metal") as spy: result_metal = metric_metal.compute( complex_signal, n_samp, self.TRANSPOSE_AXES ) @@ -1200,6 +1235,14 @@ class TestBackendCapability: METAL_CAPABLE = {"pli", "wpli", "accorr"} + def test_capability_matrix(self): + """supports() must answer exactly the hand-written support matrix.""" + assert set(METRICS) == set(EXPECTED_BACKENDS) + for mode, cls in METRICS.items(): + actual = {b for b in ALL_BACKENDS if cls.supports(b)} + assert actual == EXPECTED_BACKENDS[mode], mode + assert cls.supports("not_a_backend") is False + def test_supports_reflects_the_implemented_methods(self): """supports() must be derived from the code, not a hand-kept list.""" for mode, cls in METRICS.items(): @@ -1254,9 +1297,8 @@ def test_metal_request_never_silently_degrades(self, mode, complex_signal): "mode, backend", [ (mode, backend) - for mode in sorted(METRICS) - for backend in METRICS[mode]._BACKEND_METHODS - if METRICS[mode].supports(backend) + for mode in sorted(EXPECTED_BACKENDS) + for backend in sorted(EXPECTED_BACKENDS[mode]) ], ) def test_compute_routes_to_the_method_of_the_backend(self, mode, backend): @@ -1361,7 +1403,7 @@ def test_priority_fallback_warning_names_the_skipped_backend(self): assert any("no GPU backend of the priority list" in m for m in messages) @staticmethod - def _legacy_metric(): + def _legacy_metric(with_helper=False): """A third-party metric written against the pre-0.6.2 contract: it overrides ``compute``, branches on ``self._backend`` itself and has no ``_compute_*`` method.""" @@ -1374,7 +1416,13 @@ def compute(self, complex_signal, n_samp, transpose_axes): base_result = super().compute(complex_signal, n_samp, transpose_axes) return self._backend, base_result - return LegacyMetric + class LegacyWithHelper(LegacyMetric): + # Same contract, but the author happened to name a helper like the + # methods of the current contract. Still its own dispatch. + def _compute_numpy(self, complex_signal, n_samp, transpose_axes): + return "helper" + + return LegacyWithHelper if with_helper else LegacyMetric @pytest.mark.parametrize( "kwargs, expected", @@ -1387,13 +1435,16 @@ def compute(self, complex_signal, n_samp, transpose_axes): (dict(optimization="auto", priority=["torch"]), "torch"), ], ) - def test_legacy_subclass_keeps_its_own_dispatch(self, kwargs, expected): + @pytest.mark.parametrize("with_helper", [False, True]) + def test_legacy_subclass_keeps_its_own_dispatch( + self, kwargs, expected, with_helper + ): """ A subclass of the earlier contract is granted the backend it asks for, as before the capability check, and without a "no implementation" warning: the base class cannot see inside its ``compute``. """ - legacy_cls = self._legacy_metric() + legacy_cls = self._legacy_metric(with_helper) with ( patch("hypyp.sync.base.METAL_AVAILABLE", True), patch("hypyp.sync.base.TORCH_AVAILABLE", True), @@ -1457,6 +1508,28 @@ class EmptyMetric(BaseMetric): assert EmptyMetric.supports("numpy") is False assert BaseMetric.supports("numpy") is False + def test_torch_only_metric_reports_its_backends(self): + """A metric of the current contract without numpy runs the backend it + has and names what is missing otherwise.""" + from hypyp.sync.base import BaseMetric + + class TorchOnly(BaseMetric): + name = "torch_only" + + def _compute_torch(self, complex_signal, n_samp, transpose_axes): + return "torch result" + + metric = TorchOnly() + with pytest.raises(NotImplementedError, match="_compute_numpy"): + metric.compute(None, 0, None) + metric._backend = "torch" + assert metric.compute(None, 0, None) == "torch result" + metric._backend = "metal" + with pytest.raises( + ValueError, match=r"implemented for this metric: \['torch'\]" + ): + metric.compute(None, 0, None) + def test_missing_numpy_implementation_is_reported(self, complex_signal): """A metric with neither ``compute`` nor ``_compute_numpy`` says so.""" from hypyp.sync.base import BaseMetric From e8e7a32ed15c9ba5ede801dd7fd88c12ccf14ace Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sat, 3 Oct 2026 23:15:06 -0400 Subject: [PATCH 06/14] fix(sync): keep custom dispatch in subclasses of built-in metrics Third follow-up to the independent review. - The dispatch flag was inherited, so a third-party subclass of a built-in metric that handles a backend in its own compute (for example a Metal branch added to PLV) was still refused that backend with a false warning. The flag now vouches only for the compute of the class that sets it: a descendant that overrides compute again is no longer capability-checked, while its super().compute() still gets the table dispatch of its parent. - Selection (supports) and dispatch (compute) no longer share one predicate: compute looks at the methods that exist, supports also at who owns compute. - Tests: the routing test takes its method names from a hand-written table instead of the table under test; a subclass of a built-in metric with its own backend is covered. - Docstrings and changelog state the conditions as they are, including the one behaviour that does change for a subclass of a built-in metric: delegating to super().compute() with a backend the parent lacks now raises instead of computing in NumPy. Refs #299, #300 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 2 +- hypyp/sync/base.py | 98 +++++++++++++++++++++++++++++++++------------- tests/test_sync.py | 44 ++++++++++++++++++++- 3 files changed, 115 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f1d8c7..6b3229d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ - `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` ### Changed -- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, keeps working as before and is not subject to the capability check +- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. One behaviour does change for a subclass of a built-in metric: delegating to `super().compute()` with a backend the parent does not implement now raises a `ValueError` instead of computing in NumPy - The whole code base is formatted with `ruff format` (line length 88). The change is purely cosmetic: the syntax tree of every file is unchanged, apart from whitespace inside docstrings, and the Python examples of `hypyp/sync/README.md` are formatted too. The vendored `hypyp/ext` and the tutorial notebooks are left untouched. The formatting commit is listed in `.git-blame-ignore-revs`, so `git blame` skips it (run `git config blame.ignoreRevsFile .git-blame-ignore-revs` once in your clone; GitHub applies it automatically) - The CI now checks formatting (`ruff format --check`) and a small set of lint rules that only catch certain bugs: syntax errors, invalid comparisons and undefined names. Ruff comes from the new `lint` dependency group, which the `dev` group includes - `black` is removed from the `dev` dependency group, since `ruff format` replaces it diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 2ddf2e4..dc3390b 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -286,8 +286,9 @@ class BaseMetric(ABC): ``_compute_torch``, ``_compute_metal`` and ``_compute_cuda``. Backend selection, capability checks and dispatch are then handled here. - A subclass that overrides ``compute`` and does not set the flag follows the - earlier contract: it is granted whatever backend is requested and + A subclass that overrides ``compute`` and does not set the flag itself + follows the earlier contract, whether it derives from this class or from a + built-in metric: it is granted whatever backend is requested and available, and is itself responsible for honouring ``self._backend``. Parameters @@ -314,6 +315,10 @@ class BaseMetric(ABC): #: "inferred": yes, unless the class overrides ``compute``. The built-in #: metrics override ``compute`` only to carry a docstring, so they set #: the flag explicitly; so should a new metric written the same way. + #: The flag vouches for the ``compute`` of the class that sets it: a + #: descendant that overrides ``compute`` again may add backends of its + #: own there, so it is no longer capability-checked unless it sets the + #: flag too. _dispatch_via_table: Optional[bool] = None #: Maps a backend name to the method implementing it. This table is the @@ -358,9 +363,9 @@ def supports(cls, backend: str) -> bool: A subclass written against the earlier contract, which overrides ``compute`` and branches on ``self._backend`` itself, cannot be - inspected this way. Unless it sets ``_dispatch_via_table``, it is - trusted with every known backend, exactly as before the capability - check existed. + inspected this way. Unless it sets ``_dispatch_via_table`` itself, it + is trusted with every known backend, exactly as before the capability + check existed. This holds for a subclass of a built-in metric too. Parameters ---------- @@ -382,26 +387,60 @@ def supports(cls, backend: str) -> bool: method = cls._BACKEND_METHODS.get(backend) if method is None: return False - if not cls._uses_table_dispatch(): + if not cls._checks_capability(): return True - implementation = getattr(cls, method, None) - # The default _compute_numpy of this class only raises: it is not an - # implementation. + return cls._implements(backend) + + @classmethod + def _implements(cls, backend: str) -> bool: + """Whether the class has a real ``_compute_*`` method for ``backend``. + + The default ``_compute_numpy`` of this class only raises, and a + non-callable attribute is a placeholder: neither is an implementation. + """ + method = cls._BACKEND_METHODS.get(backend) + implementation = getattr(cls, method, None) if method else None return callable(implementation) and ( implementation is not BaseMetric._compute_numpy ) @classmethod - def _uses_table_dispatch(cls) -> bool: - """Whether this class dispatches through ``_BACKEND_METHODS``. + def _dispatch_owner(cls) -> tuple: + """The nearest class of the MRO that sets ``_dispatch_via_table``, and + the value it sets; ``(None, None)`` when no class does.""" + for klass in cls.__mro__: + flag = klass.__dict__.get("_dispatch_via_table") + if flag is not None: + return klass, flag + return None, None + + @classmethod + def _dispatches_via_table(cls) -> bool: + """Whether ``BaseMetric.compute`` dispatches for this class. + + Explicit when a class of the MRO sets ``_dispatch_via_table``. + Otherwise inferred: a class that overrides ``compute`` is taken to do + its own dispatch, as the contract was before ``compute`` became + concrete. + """ + owner, flag = cls._dispatch_owner() + if owner is None: + return cls.compute is BaseMetric.compute + return flag + + @classmethod + def _checks_capability(cls) -> bool: + """Whether backend selection may rely on the ``_compute_*`` methods. - Explicit when ``_dispatch_via_table`` is set. Otherwise inferred: a - class that overrides ``compute`` is taken to do its own dispatch, as - the contract was before ``compute`` became concrete. + True when the table dispatch is the only dispatch: the class uses it, + and ``compute`` has not been overridden below the class that set the + flag. A descendant that overrides ``compute`` again may handle + backends there that no ``_compute_*`` method reveals. """ - if cls._dispatch_via_table is not None: - return cls._dispatch_via_table - return cls.compute is BaseMetric.compute + if not cls._dispatches_via_table(): + return False + owner, _ = cls._dispatch_owner() + return owner is None or cls.compute is owner.compute @classmethod def _cpu_fallback(cls) -> tuple: @@ -684,10 +723,12 @@ def compute( Raises ------ NotImplementedError - If the metric provides neither ``_compute_numpy`` nor its own - ``compute``. + If the backend is numpy and the metric has no ``_compute_numpy`` + (a metric may implement an accelerated backend alone, but then + cannot serve the default ``optimization=None``). ValueError - If ``self._backend`` is not a backend this metric implements. The + If ``self._backend`` is any other backend the metric has no + ``_compute_*`` method for, or an unknown name. The message names the metric and the backends it does implement. This is deliberate: an earlier hand-written ``if/elif`` chain per metric ended in a bare ``return self._compute_numpy(...)``, so an @@ -700,22 +741,25 @@ def compute( kernels and torch on MPS compute in ``float32`` whatever the input; for the other combinations see each metric. - A subclass that does its own dispatch (see ``_dispatch_via_table``) - may call ``super().compute(...)`` from its own ``compute``. The base - method was then abstract with an empty body and returned ``None``; it - still does for such a subclass. + A subclass of ``BaseMetric`` that does its own dispatch (see + ``_dispatch_via_table``) may call ``super().compute(...)`` from its own + ``compute``. The base method was then abstract with an empty body and + returned ``None``; it still does for such a subclass. A subclass of a + built-in metric that overrides ``compute`` and delegates to + ``super().compute(...)`` gets the table dispatch of its parent, and + the errors above for a backend the parent does not implement. """ - if not self._uses_table_dispatch(): + if not self._dispatches_via_table(): # Reached through super().compute() from a subclass that does its # own dispatch: behave as the former abstract method did. return None - if not self.supports(self._backend): + if not self._implements(self._backend): if self._backend == "numpy": raise NotImplementedError( f"{type(self).__name__} must implement _compute_numpy " f"(or override compute)." ) - implemented = [b for b in self._BACKEND_METHODS if self.supports(b)] + implemented = [b for b in self._BACKEND_METHODS if self._implements(b)] raise ValueError( f"{self.name!r} cannot run on backend {self._backend!r}. " f"Backends implemented for this metric: {implemented}." diff --git a/tests/test_sync.py b/tests/test_sync.py index 2435646..39a0b69 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -45,6 +45,16 @@ } ALL_BACKENDS = ("numpy", "numba", "torch", "metal", "cuda_kernel") +#: Also by hand: the routing test must not read the method names from +#: BaseMetric._BACKEND_METHODS, the table it checks. +EXPECTED_METHODS = { + "numpy": "_compute_numpy", + "numba": "_compute_numba", + "torch": "_compute_torch", + "metal": "_compute_metal", + "cuda_kernel": "_compute_cuda", +} + def spy_on_kernel(module_name, function_name): """ @@ -1309,7 +1319,7 @@ def test_compute_routes_to_the_method_of_the_backend(self, mode, backend): stub. """ cls = METRICS[mode] - method_name = cls._BACKEND_METHODS[backend] + method_name = EXPECTED_METHODS[backend] metric = cls() metric._backend = backend sentinel = object() @@ -1488,6 +1498,38 @@ def _compute_numpy(self, complex_signal, n_samp, transpose_axes): n_samp = complex_signal.shape[3] assert metric.compute(complex_signal, n_samp, (0, 1, 3, 2)).shape == (1,) + def test_subclass_of_builtin_with_its_own_backend(self, complex_signal): + """ + A third-party subclass of a built-in metric that handles a backend in + its own ``compute`` and delegates the rest to its parent keeps working: + the backend is granted without warning, its own branch runs, and the + delegation still computes. + """ + from hypyp.sync.plv import PLV + + class CustomPLV(PLV): + def compute(self, complex_signal, n_samp, transpose_axes): + if self._backend == "metal": + return "custom Metal" + return super().compute(complex_signal, n_samp, transpose_axes) + + with patch("hypyp.sync.base.METAL_AVAILABLE", True): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + metric = CustomPLV(optimization="metal") + assert metric._backend == "metal" + assert not caught + assert metric.compute(None, 0, None) == "custom Metal" + + n_samp = complex_signal.shape[3] + axes = (0, 1, 3, 2) + np.testing.assert_array_equal( + CustomPLV().compute(complex_signal, n_samp, axes), + PLV().compute(complex_signal, n_samp, axes), + ) + # The built-in parent itself stays capability-checked. + assert PLV.supports("metal") is False + def test_supports_ignores_placeholders(self): """supports() must not count a non-callable attribute, nor the default ``_compute_numpy`` of the base class, as an implementation.""" From 69db323333fbd30de06b95d2ac430475fd7f50f1 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sat, 3 Oct 2026 23:28:52 -0400 Subject: [PATCH 07/14] fix(sync): do not assume the class that sets the dispatch flag defines compute Fourth follow-up to the independent review. - A mixin carrying _dispatch_via_table made supports() raise AttributeError, because the owner of the flag was assumed to expose compute. Capability checking now compares positions in the MRO: it applies when compute is not overridden below the class that sets the flag. - A descendant that overrides compute only to delegate could be handed numba by the automatic CPU fallback although no numba method exists. Under the table dispatch the fallback now requires the method itself. - The compatibility test compared two results that could both be None; it now checks the type and compares with the numpy method. - Docstrings: a flag reset to None does not restore the inference, and the dispatch looks methods up on the instance. Refs #299, #300 Co-Authored-By: Claude Fable 5.1 --- hypyp/sync/base.py | 32 ++++++++++++++++++++----- tests/test_sync.py | 59 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index dc3390b..deaf8d4 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -318,7 +318,9 @@ class BaseMetric(ABC): #: The flag vouches for the ``compute`` of the class that sets it: a #: descendant that overrides ``compute`` again may add backends of its #: own there, so it is no longer capability-checked unless it sets the - #: flag too. + #: flag too. Setting it back to ``None`` in a descendant does not restore + #: the inference: the nearest class of the MRO that sets ``True`` or + #: ``False`` decides. _dispatch_via_table: Optional[bool] = None #: Maps a backend name to the method implementing it. This table is the @@ -440,13 +442,29 @@ def _checks_capability(cls) -> bool: if not cls._dispatches_via_table(): return False owner, _ = cls._dispatch_owner() - return owner is None or cls.compute is owner.compute + if owner is None: + return True + # Compare positions in the MRO rather than methods: the class that + # sets the flag need not define compute itself (a mixin, or a metric + # that keeps the compute of this class). + mro = cls.__mro__ + compute_provider = next(k for k in mro if "compute" in k.__dict__) + return mro.index(compute_provider) >= mro.index(owner) @classmethod def _cpu_fallback(cls) -> tuple: """CPU backend used when no GPU backend can be selected: numba when it - is installed and the metric implements it, numpy otherwise.""" - if NUMBA_AVAILABLE and cls.supports("numba"): + is installed and the metric implements it, numpy otherwise. + + Nobody asked for numba by name here, so under the table dispatch the + method must really exist, even for a descendant that overrides + ``compute`` and is otherwise trusted with any backend it requests. + """ + if cls._dispatches_via_table(): + has_numba = cls._implements("numba") + else: + has_numba = cls.supports("numba") + if NUMBA_AVAILABLE and has_numba: return "numba", "cpu" return "numpy", "cpu" @@ -746,8 +764,10 @@ def compute( ``compute``. The base method was then abstract with an empty body and returned ``None``; it still does for such a subclass. A subclass of a built-in metric that overrides ``compute`` and delegates to - ``super().compute(...)`` gets the table dispatch of its parent, and - the errors above for a backend the parent does not implement. + ``super().compute(...)`` gets the table dispatch, which looks the + method up on the instance: a ``_compute_*`` method added by the + descendant is used, and the errors above are raised for a backend + that neither it nor its parent implements. """ if not self._dispatches_via_table(): # Reached through super().compute() from a subclass that does its diff --git a/tests/test_sync.py b/tests/test_sync.py index 39a0b69..6c1dbed 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1523,13 +1523,68 @@ def compute(self, complex_signal, n_samp, transpose_axes): n_samp = complex_signal.shape[3] axes = (0, 1, 3, 2) + delegated = CustomPLV().compute(complex_signal, n_samp, axes) + # Compared with the numpy method itself, and checked to be an array: + # two None results would otherwise compare equal. + assert isinstance(delegated, np.ndarray) np.testing.assert_array_equal( - CustomPLV().compute(complex_signal, n_samp, axes), - PLV().compute(complex_signal, n_samp, axes), + delegated, PLV()._compute_numpy(complex_signal, n_samp, axes) ) # The built-in parent itself stays capability-checked. assert PLV.supports("metal") is False + def test_dispatch_flag_set_by_a_mixin(self): + """ + The class that sets ``_dispatch_via_table`` need not define + ``compute``: a mixin can carry the flag. The metric is then + capability-checked like its built-in parent, and does not crash. + """ + from hypyp.sync.plv import PLV + + class DispatchPolicy: + _dispatch_via_table = True + + class MixedPLV(DispatchPolicy, PLV): + pass + + assert MixedPLV.supports("numpy") is True + assert MixedPLV.supports("metal") is False + with patch("hypyp.sync.base.METAL_AVAILABLE", True): + with pytest.warns(UserWarning, match="no Metal implementation"): + metric = MixedPLV(optimization="metal") + assert metric._backend == "numpy" + + def test_delegating_descendant_of_numpy_only_metric_never_gets_numba(self): + """ + A descendant that overrides ``compute`` only to delegate is trusted + with the backends it requests, but the automatic CPU fallback must + still not hand it a numba method that does not exist. + """ + from hypyp.sync.base import BaseMetric + + class NumpyOnly(BaseMetric): + name = "numpy_only" + _dispatch_via_table = True + + def _compute_numpy(self, complex_signal, n_samp, transpose_axes): + return "numpy result" + + class Delegating(NumpyOnly): + def compute(self, complex_signal, n_samp, transpose_axes): + return super().compute(complex_signal, n_samp, transpose_axes) + + with ( + patch("hypyp.sync.base.NUMBA_AVAILABLE", True), + patch("hypyp.sync.base.TORCH_AVAILABLE", False), + patch("hypyp.sync.base.MPS_AVAILABLE", False), + patch("hypyp.sync.base.CUDA_AVAILABLE", False), + ): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + metric = Delegating(optimization="auto") + assert metric._backend == "numpy" + assert metric.compute(None, 0, None) == "numpy result" + def test_supports_ignores_placeholders(self): """supports() must not count a non-callable attribute, nor the default ``_compute_numpy`` of the base class, as an implementation.""" From aa4955a79b3f4fa5ee5e9f87fab25779f13edb3b Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sat, 3 Oct 2026 23:42:13 -0400 Subject: [PATCH 08/14] fix(sync): keep the capability check when a subclass rebinds its parent's compute A subclass that sets compute = PLV.compute was taken for a class doing its own dispatch, because the check compared positions in the MRO. It now compares the functions, so the same function rebound lower in the hierarchy stays capability-checked. The changelog states the scope of the unchanged 'auto' selection and the numba restriction of the CPU fallback. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 4 ++-- hypyp/sync/base.py | 17 ++++++++++++----- tests/test_sync.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b3229d..2842882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## [Unreleased] ### Fixed -- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. `optimization='metal'` on these metrics now warns and falls back to NumPy, and a `priority` list skips Metal and moves on to its next backend. Only PLI, wPLI and ACCorr have a Metal kernel; the default selection of `optimization='auto'` is unchanged. No numerical implementation changes, but a request that used to run NumPy without saying so can now run the backend that comes next in its `priority` list: for example `priority=['metal', 'torch']` on PLV now runs torch, which computes in single precision on Apple GPUs (#299) +- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. `optimization='metal'` on these metrics now warns and falls back to NumPy, and a `priority` list skips Metal and moves on to its next backend. Only PLI, wPLI and ACCorr have a Metal kernel; the default selection of `optimization='auto'` is unchanged for the nine built-in metrics. No numerical implementation changes, but a request that used to run NumPy without saying so can now run the backend that comes next in its `priority` list: for example `priority=['metal', 'torch']` on PLV now runs torch, which computes in single precision on Apple GPUs (#299) - `hypyp.sync`: when a backend of a `priority` list was skipped because the metric does not implement it and no other GPU backend of the list could be used, the warning now says so (for example "'plv' has no Metal implementation") instead of "No GPU backend available" - `hypyp.sync`: a built-in metric whose backend is unknown or not implemented now raises a `ValueError` naming the metric and the backends it implements, instead of computing in NumPy without notice. Called through `compute_sync`, that error is still reworded as an unsupported metric (#306) - Tests: the six Metal tests that compared NumPy with NumPy are replaced by tests of the fallback itself (warning, backend and result), the dispatch of every metric to every backend it implements is checked without a GPU, and the four tests that run a Metal kernel now assert that the Metal method was called (#300) @@ -12,7 +12,7 @@ - `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` ### Changed -- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. One behaviour does change for a subclass of a built-in metric: delegating to `super().compute()` with a backend the parent does not implement now raises a `ValueError` instead of computing in NumPy +- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. Two behaviours do change for a subclass of a built-in metric. Delegating to `super().compute()` with a backend that neither the subclass nor its parents implement as a `_compute_*` method now raises a `ValueError` instead of computing in NumPy. And when `optimization='auto'` falls back to the CPU, it selects numba only if a `_compute_numba` method exists: a subclass that handles numba inside its own `compute` gets NumPy unless numba is requested by name - The whole code base is formatted with `ruff format` (line length 88). The change is purely cosmetic: the syntax tree of every file is unchanged, apart from whitespace inside docstrings, and the Python examples of `hypyp/sync/README.md` are formatted too. The vendored `hypyp/ext` and the tutorial notebooks are left untouched. The formatting commit is listed in `.git-blame-ignore-revs`, so `git blame` skips it (run `git config blame.ignoreRevsFile .git-blame-ignore-revs` once in your clone; GitHub applies it automatically) - The CI now checks formatting (`ruff format --check`) and a small set of lint rules that only catch certain bugs: syntax errors, invalid comparisons and undefined names. Ruff comes from the new `lint` dependency group, which the `dev` group includes - `black` is removed from the `dev` dependency group, since `ruff format` replaces it diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index deaf8d4..55ed189 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -444,12 +444,19 @@ def _checks_capability(cls) -> bool: owner, _ = cls._dispatch_owner() if owner is None: return True - # Compare positions in the MRO rather than methods: the class that - # sets the flag need not define compute itself (a mixin, or a metric - # that keeps the compute of this class). + # The flag vouches for the compute that its owner resolves to, which + # the owner need not define itself (a mixin, or a metric that keeps + # the compute of this class). Compare the functions rather than the + # classes that hold them: a descendant that rebinds the very same + # function (``compute = PLV.compute``) has not changed the dispatch. mro = cls.__mro__ - compute_provider = next(k for k in mro if "compute" in k.__dict__) - return mro.index(compute_provider) >= mro.index(owner) + + def first_compute(classes: tuple): + return next( + k.__dict__["compute"] for k in classes if "compute" in k.__dict__ + ) + + return first_compute(mro) is first_compute(mro[mro.index(owner) :]) @classmethod def _cpu_fallback(cls) -> tuple: diff --git a/tests/test_sync.py b/tests/test_sync.py index 6c1dbed..4ebd0bb 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1554,6 +1554,48 @@ class MixedPLV(DispatchPolicy, PLV): metric = MixedPLV(optimization="metal") assert metric._backend == "numpy" + def test_rebinding_the_parent_compute_keeps_the_capability_check(self): + """ + ``compute = PLV.compute`` in a subclass is the same function as the + one the flag of PLV vouches for, not a dispatch of its own: the + subclass is still capability-checked. + """ + from hypyp.sync.plv import PLV + + class Alias(PLV): + compute = PLV.compute + + assert Alias.supports("numpy") is True + assert Alias.supports("metal") is False + with patch("hypyp.sync.base.METAL_AVAILABLE", True): + with pytest.warns(UserWarning, match="no Metal implementation"): + metric = Alias(optimization="metal") + assert metric._backend == "numpy" + + def test_auto_does_not_guess_numba_handled_inside_compute(self): + """ + A descendant of a built-in metric that hides ``_compute_numba`` and + handles numba inside its own ``compute`` cannot be told apart from + one that only delegates. ``'auto'`` therefore falls back to numpy for + it, while an explicit request for numba is still granted. + """ + from hypyp.sync.plv import PLV + + class OwnNumba(PLV): + _compute_numba = None + + def compute(self, complex_signal, n_samp, transpose_axes): + if self._backend == "numba": + return "own numba" + return super().compute(complex_signal, n_samp, transpose_axes) + + with ( + patch("hypyp.sync.base.NUMBA_AVAILABLE", True), + patch("hypyp.sync.base.TORCH_AVAILABLE", False), + ): + assert OwnNumba._cpu_fallback() == ("numpy", "cpu") + assert OwnNumba(optimization="numba")._backend == "numba" + def test_delegating_descendant_of_numpy_only_metric_never_gets_numba(self): """ A descendant that overrides ``compute`` only to delegate is trusted From f7c6dd5bd05010fd94435902868470113d14f414 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sat, 3 Oct 2026 23:57:06 -0400 Subject: [PATCH 09/14] fix(sync): accept a dispatch-flag mixin listed after BaseMetric When the class that sets _dispatch_via_table comes after every class that defines compute, the lookup of the vouched function found nothing and raised StopIteration. It now falls back to the table dispatch of BaseMetric. The numba fallback test now builds the metric with 'auto' and computes, and the changelog states the exact condition of that restriction. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 2 +- hypyp/sync/base.py | 5 ++++- tests/test_sync.py | 48 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2842882..31306ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ - `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` ### Changed -- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. Two behaviours do change for a subclass of a built-in metric. Delegating to `super().compute()` with a backend that neither the subclass nor its parents implement as a `_compute_*` method now raises a `ValueError` instead of computing in NumPy. And when `optimization='auto'` falls back to the CPU, it selects numba only if a `_compute_numba` method exists: a subclass that handles numba inside its own `compute` gets NumPy unless numba is requested by name +- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. Two behaviours do change for a subclass of a built-in metric. Delegating to `super().compute()` with a backend that neither the subclass nor its parents implement as a `_compute_*` method now raises a `ValueError` instead of computing in NumPy. And when `optimization='auto'` falls back to the CPU, it selects numba only if a `_compute_numba` method exists: a subclass that handles numba inside its own `compute` and has no callable `_compute_numba` gets NumPy unless numba is requested by name - The whole code base is formatted with `ruff format` (line length 88). The change is purely cosmetic: the syntax tree of every file is unchanged, apart from whitespace inside docstrings, and the Python examples of `hypyp/sync/README.md` are formatted too. The vendored `hypyp/ext` and the tutorial notebooks are left untouched. The formatting commit is listed in `.git-blame-ignore-revs`, so `git blame` skips it (run `git config blame.ignoreRevsFile .git-blame-ignore-revs` once in your clone; GitHub applies it automatically) - The CI now checks formatting (`ruff format --check`) and a small set of lint rules that only catch certain bugs: syntax errors, invalid comparisons and undefined names. Ruff comes from the new `lint` dependency group, which the `dev` group includes - `black` is removed from the `dev` dependency group, since `ruff format` replaces it diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 55ed189..21ad0fc 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -452,8 +452,11 @@ def _checks_capability(cls) -> bool: mro = cls.__mro__ def first_compute(classes: tuple): + # A flag owner placed after every class that defines compute (a + # mixin listed after BaseMetric) vouches for the table dispatch. return next( - k.__dict__["compute"] for k in classes if "compute" in k.__dict__ + (k.__dict__["compute"] for k in classes if "compute" in k.__dict__), + BaseMetric.__dict__["compute"], ) return first_compute(mro) is first_compute(mro[mro.index(owner) :]) diff --git a/tests/test_sync.py b/tests/test_sync.py index 4ebd0bb..11230e0 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1572,7 +1572,7 @@ class Alias(PLV): metric = Alias(optimization="metal") assert metric._backend == "numpy" - def test_auto_does_not_guess_numba_handled_inside_compute(self): + def test_auto_does_not_guess_numba_handled_inside_compute(self, complex_signal): """ A descendant of a built-in metric that hides ``_compute_numba`` and handles numba inside its own ``compute`` cannot be told apart from @@ -1589,12 +1589,54 @@ def compute(self, complex_signal, n_samp, transpose_axes): return "own numba" return super().compute(complex_signal, n_samp, transpose_axes) + n_samp = complex_signal.shape[3] + axes = (0, 1, 3, 2) with ( patch("hypyp.sync.base.NUMBA_AVAILABLE", True), patch("hypyp.sync.base.TORCH_AVAILABLE", False), ): - assert OwnNumba._cpu_fallback() == ("numpy", "cpu") - assert OwnNumba(optimization="numba")._backend == "numba" + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + automatic = OwnNumba(optimization="auto") + requested = OwnNumba(optimization="numba") + assert automatic._backend == "numpy" + result = automatic.compute(complex_signal, n_samp, axes) + assert isinstance(result, np.ndarray) + np.testing.assert_array_equal( + result, PLV()._compute_numpy(complex_signal, n_samp, axes) + ) + assert requested._backend == "numba" + assert requested.compute(complex_signal, n_samp, axes) == "own numba" + + def test_dispatch_flag_set_by_a_mixin_listed_after_the_base(self): + """ + A mixin that carries the flag can come after ``BaseMetric`` in the + bases, where no class defines ``compute`` any more. A metric with its + own ``compute`` is then trusted with the backend it requests, and one + without is capability-checked; neither crashes. + """ + + class DispatchPolicy: + _dispatch_via_table = True + + class OwnDispatch(BaseMetric, DispatchPolicy): + name = "own_dispatch" + + def compute(self, complex_signal, n_samp, transpose_axes): + return self._backend + + class TableOnly(BaseMetric, DispatchPolicy): + name = "table_only" + + def _compute_numpy(self, complex_signal, n_samp, transpose_axes): + return "numpy" + + with patch("hypyp.sync.base.NUMBA_AVAILABLE", True): + assert OwnDispatch.supports("numba") is True + metric = OwnDispatch(optimization="numba") + assert metric.compute(None, 0, None) == "numba" + assert TableOnly.supports("numpy") is True + assert TableOnly.supports("numba") is False def test_delegating_descendant_of_numpy_only_metric_never_gets_numba(self): """ From b78d30c1127a7b3eefa2679fc0e51367045eb5f5 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 00:07:21 -0400 Subject: [PATCH 10/14] test(sync): make the numba fallback test independent of the GPU of the host On a CUDA node 'auto' rightly selected the CUDA kernel, so the test of the CPU fallback failed there. It now also declares MPS and CUDA unavailable. Co-Authored-By: Claude Fable 5.1 --- tests/test_sync.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_sync.py b/tests/test_sync.py index 11230e0..3207307 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1594,6 +1594,8 @@ def compute(self, complex_signal, n_samp, transpose_axes): with ( patch("hypyp.sync.base.NUMBA_AVAILABLE", True), patch("hypyp.sync.base.TORCH_AVAILABLE", False), + patch("hypyp.sync.base.MPS_AVAILABLE", False), + patch("hypyp.sync.base.CUDA_AVAILABLE", False), ): with warnings.catch_warnings(): warnings.simplefilter("ignore") From ad96ff59214c2d19dd6da7c524bda2e709259dd1 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 00:15:28 -0400 Subject: [PATCH 11/14] fix(sync): keep numpy when a priority list reaches an unimplemented backend The 0.6 series changes no computed value. A priority list that reached an available backend the metric does not implement used to compute in numpy silently; skipping to the next backend of the list could change the result (torch in single precision on Apple GPUs, or numba). The selection now ends in numpy as before, with a warning and a truthful backend attribute. Moving on to the next backend is left to 0.7.0. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 4 ++-- hypyp/sync/base.py | 29 ++++++++++++++++++++++++----- tests/test_sync.py | 32 +++++++++++++++++++------------- 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31306ca..c876e44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,8 @@ ## [Unreleased] ### Fixed -- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. `optimization='metal'` on these metrics now warns and falls back to NumPy, and a `priority` list skips Metal and moves on to its next backend. Only PLI, wPLI and ACCorr have a Metal kernel; the default selection of `optimization='auto'` is unchanged for the nine built-in metrics. No numerical implementation changes, but a request that used to run NumPy without saying so can now run the backend that comes next in its `priority` list: for example `priority=['metal', 'torch']` on PLV now runs torch, which computes in single precision on Apple GPUs (#299) -- `hypyp.sync`: when a backend of a `priority` list was skipped because the metric does not implement it and no other GPU backend of the list could be used, the warning now says so (for example "'plv' has no Metal implementation") instead of "No GPU backend available" +- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. The computation still runs in NumPy, so no computed value changes, but it now warns and the metric object reports `numpy`. This holds for `optimization='metal'` and for a `priority` list that reaches Metal: the backends that follow in the list are not tried, which is left to 0.7.0. Only PLI, wPLI and ACCorr have a Metal kernel; the default selection of `optimization='auto'` is unchanged for the nine built-in metrics (#299) +- `hypyp.sync`: when a backend of a `priority` list can neither run on the machine nor be computed by the metric, and no other GPU backend of the list can be used, the warning now says so (for example "'plv' has no Metal implementation") instead of "No GPU backend available" - `hypyp.sync`: a built-in metric whose backend is unknown or not implemented now raises a `ValueError` naming the metric and the backends it implements, instead of computing in NumPy without notice. Called through `compute_sync`, that error is still reworded as an unsupported metric (#306) - Tests: the six Metal tests that compared NumPy with NumPy are replaced by tests of the fallback itself (warning, backend and result), the dispatch of every metric to every backend it implements is checked without a GPU, and the four tests that run a Metal kernel now assert that the Metal method was called (#300) diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 21ad0fc..2c3627c 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -655,13 +655,32 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: # Backends of the priority list this metric has no implementation for, # remembered so the fallback warning can give the real reason. unimplemented = [] + available = { + "torch": TORCH_AVAILABLE, + "metal": METAL_AVAILABLE, + "cuda_kernel": CUPY_AVAILABLE, + } for backend in priority: - # Skip a backend this metric has no implementation for, so a - # priority list falls through to the next candidate instead of - # selecting a backend that would degrade to numpy at dispatch. if not cls.supports(backend): - if backend in cls._BACKEND_LABELS: - unimplemented.append(cls._BACKEND_LABELS[backend]) + label = cls._BACKEND_LABELS.get(backend) + # Earlier versions selected an available backend here even + # though the metric has no implementation for it, and the + # computation then ran in numpy without notice. The 0.6 + # series does not change computed values, so the selection + # still ends in numpy, now with a warning. Moving on to the + # next backend of the list instead is left to 0.7.0. + if label and available.get(backend): + warnings.warn( + f"{cls.name!r} has no {label} implementation: computing " + f"with numpy, as earlier versions did silently. The " + f"backends that follow in the priority list are not " + f"tried.", + UserWarning, + stacklevel=4, + ) + return "numpy", "cpu" + if label: + unimplemented.append(label) continue if backend == "torch" and TORCH_AVAILABLE: return cls._resolve_torch() diff --git a/tests/test_sync.py b/tests/test_sync.py index 3207307..557e082 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1330,21 +1330,25 @@ def test_compute_routes_to_the_method_of_the_backend(self, mode, backend): stub.assert_called_once_with(metric, "signal", 7, (0, 1, 3, 2)) @pytest.mark.parametrize("mode", sorted(set(METRICS) - {"pli", "wpli", "accorr"})) - def test_auto_priority_skips_unsupported_backend(self, mode): + @pytest.mark.parametrize("priority", [["metal", "torch"], ["metal"]]) + def test_auto_priority_unsupported_backend_stays_on_numpy(self, mode, priority): """ - A priority list must skip a backend the metric cannot run. + A priority list that reaches an available backend the metric cannot + run keeps computing in numpy, as before, but now says so. - priority=['metal', 'torch'] on an einsum metric should land on torch, - not on a metal that resolves to numpy behind the caller's back. + The 0.6 series changes no computed value: moving on to the next + backend of the list (torch, or the numba fallback) would. """ with ( patch("hypyp.sync.base.METAL_AVAILABLE", True), patch("hypyp.sync.base.TORCH_AVAILABLE", True), patch("hypyp.sync.base.MPS_AVAILABLE", True), + patch("hypyp.sync.base.NUMBA_AVAILABLE", True), ): - metric = get_metric(mode, optimization="auto", priority=["metal", "torch"]) - assert metric._backend == "torch", ( - f"{mode}: priority=['metal','torch'] resolved to {metric._backend!r}" + with pytest.warns(UserWarning, match="no Metal implementation"): + metric = get_metric(mode, optimization="auto", priority=priority) + assert (metric._backend, metric._device) == ("numpy", "cpu"), ( + f"{mode}: priority={priority} resolved to {metric._backend!r}" ) def test_unknown_backend_fails_closed(self, complex_signal): @@ -1384,17 +1388,19 @@ def test_unimplemented_backend_fails_closed(self, complex_signal): def test_priority_fallback_warning_names_the_skipped_backend(self): """ - When the only backend of a priority list has no implementation, the - fallback warning must give that reason. + When the only backend of a priority list has no implementation and + cannot run on the machine either, the fallback warning must give the + first reason. Before, priority=['metal'] on an einsum metric warned "No GPU backend - available" on a machine where a GPU backend was available, without - mentioning that Metal was skipped for lack of a kernel. + available" on a CUDA machine, where a GPU backend was available, + without mentioning that the metric has no Metal kernel. """ with ( - patch("hypyp.sync.base.METAL_AVAILABLE", True), + patch("hypyp.sync.base.METAL_AVAILABLE", False), patch("hypyp.sync.base.TORCH_AVAILABLE", True), - patch("hypyp.sync.base.MPS_AVAILABLE", True), + patch("hypyp.sync.base.MPS_AVAILABLE", False), + patch("hypyp.sync.base.CUDA_AVAILABLE", True), ): with warnings.catch_warnings(record=True) as caught: warnings.simplefilter("always") From 5b7f72b36fe3e8cccdf7914c75ac25b51d6dbfd0 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 00:31:10 -0400 Subject: [PATCH 12/14] fix(sync): keep the earlier results of subclasses that delegate to the dispatch The 0.6 series changes no computed value. A subclass of a built-in metric that overrides compute and delegates to super().compute() with a backend that has no method now computes in numpy with a warning, as the former if/elif chains did silently, instead of raising. The CPU fallback of 'auto' trusts such a subclass with numba again. Stale docstrings are updated. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 6 ++--- hypyp/sync/base.py | 61 +++++++++++++++++++++++++++++++--------------- tests/test_sync.py | 45 +++++++++++++++++++--------------- 3 files changed, 69 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c876e44..eecdc5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,16 +3,16 @@ ## [Unreleased] ### Fixed -- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. The computation still runs in NumPy, so no computed value changes, but it now warns and the metric object reports `numpy`. This holds for `optimization='metal'` and for a `priority` list that reaches Metal: the backends that follow in the list are not tried, which is left to 0.7.0. Only PLI, wPLI and ACCorr have a Metal kernel; the default selection of `optimization='auto'` is unchanged for the nine built-in metrics (#299) +- `hypyp.sync`: asking for the Metal backend on a metric that has no Metal kernel (PLV, CCorr, Coh, ImCoh, EnvCorr, PowCorr) used to run in NumPy silently, while the metric object reported `metal`. The computation still runs in NumPy, so no computed value changes, but it now warns and the metric object reports `numpy`. This holds for `optimization='metal'` and for a `priority` list that reaches Metal on a machine where Metal is available: the backends that follow in the list are not tried, which is left to 0.7.0. Only PLI, wPLI and ACCorr have a Metal kernel; the default selection of `optimization='auto'` is unchanged for the nine built-in metrics (#299) - `hypyp.sync`: when a backend of a `priority` list can neither run on the machine nor be computed by the metric, and no other GPU backend of the list can be used, the warning now says so (for example "'plv' has no Metal implementation") instead of "No GPU backend available" -- `hypyp.sync`: a built-in metric whose backend is unknown or not implemented now raises a `ValueError` naming the metric and the backends it implements, instead of computing in NumPy without notice. Called through `compute_sync`, that error is still reworded as an unsupported metric (#306) +- `hypyp.sync`: a metric whose `_backend` was set to a backend it does not implement used to compute in NumPy without notice. It still computes in NumPy, now with a warning. A `_backend` that is not the name of a backend raises a `ValueError` naming the metric and the backends it implements; called through `compute_sync`, that error is still reworded as an unsupported metric (#306) - Tests: the six Metal tests that compared NumPy with NumPy are replaced by tests of the fallback itself (warning, backend and result), the dispatch of every metric to every backend it implements is checked without a GPU, and the four tests that run a Metal kernel now assert that the Metal method was called (#300) ### Added - `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` ### Changed -- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. Two behaviours do change for a subclass of a built-in metric. Delegating to `super().compute()` with a backend that neither the subclass nor its parents implement as a `_compute_*` method now raises a `ValueError` instead of computing in NumPy. And when `optimization='auto'` falls back to the CPU, it selects numba only if a `_compute_numba` method exists: a subclass that handles numba inside its own `compute` and has no callable `_compute_numba` gets NumPy unless numba is requested by name +- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. A subclass of a built-in metric that overrides `compute` and delegates to `super().compute()` keeps the earlier results: a backend without a `_compute_*` method computes in NumPy, now with a warning, and the CPU fallback of `optimization='auto'` still selects numba for it when numba is installed. One thing is new for subclasses: a `_compute_*` method added to a subclass that keeps the inherited `compute` is now used, where it was ignored before - The whole code base is formatted with `ruff format` (line length 88). The change is purely cosmetic: the syntax tree of every file is unchanged, apart from whitespace inside docstrings, and the Python examples of `hypyp/sync/README.md` are formatted too. The vendored `hypyp/ext` and the tutorial notebooks are left untouched. The formatting commit is listed in `.git-blame-ignore-revs`, so `git blame` skips it (run `git config blame.ignoreRevsFile .git-blame-ignore-revs` once in your clone; GitHub applies it automatically) - The CI now checks formatting (`ruff format --check`) and a small set of lint rules that only catch certain bugs: syntax errors, invalid comparisons and undefined names. Ruff comes from the new `lint` dependency group, which the `dev` group includes - `black` is removed from the `dev` dependency group, since `ruff format` replaces it diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 2c3627c..c1062d6 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -279,7 +279,7 @@ def multiply_conjugate_time_torch(c, s): class BaseMetric(ABC): """ - Abstract base class for connectivity metrics. + Base class for connectivity metrics. A metric inherits from this class, sets ``_dispatch_via_table = True`` and implements ``_compute_numpy`` plus any of the optional ``_compute_numba``, @@ -296,7 +296,8 @@ class BaseMetric(ABC): optimization : str, optional Optimization strategy for computation. Options: - None: standard numpy (default) - - 'auto': best available (torch > numba > numpy) + - 'auto': best backend for this metric and platform (see + ``_resolve_auto`` and ``AUTO_PRIORITY``) - 'numba': numba JIT compilation (falls back to numpy if unavailable) - 'torch': PyTorch with auto-detected GPU (falls back gracefully) @@ -464,17 +465,14 @@ def first_compute(classes: tuple): @classmethod def _cpu_fallback(cls) -> tuple: """CPU backend used when no GPU backend can be selected: numba when it - is installed and the metric implements it, numpy otherwise. + is installed and the metric supports it, numpy otherwise. - Nobody asked for numba by name here, so under the table dispatch the - method must really exist, even for a descendant that overrides - ``compute`` and is otherwise trusted with any backend it requests. + A class that overrides ``compute`` is trusted with numba here, as it + was before the capability check: it may handle numba in its own + ``compute``. If it only delegates and no ``_compute_numba`` exists, + the dispatch computes in numpy with a warning. """ - if cls._dispatches_via_table(): - has_numba = cls._implements("numba") - else: - has_numba = cls.supports("numba") - if NUMBA_AVAILABLE and has_numba: + if NUMBA_AVAILABLE and cls.supports("numba"): return "numba", "cpu" return "numpy", "cpu" @@ -615,7 +613,9 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: Uses the ``AUTO_PRIORITY`` table compiled from Mac M4 Max and Narval A100 benchmarks. Iterates the priority list and returns - the first available backend. + the first available backend the metric implements. An available + backend the metric does not implement ends the search in numpy with + a warning (see the comment in the loop). Parameters ---------- @@ -774,13 +774,20 @@ def compute( (a metric may implement an accelerated backend alone, but then cannot serve the default ``optimization=None``). ValueError - If ``self._backend`` is any other backend the metric has no - ``_compute_*`` method for, or an unknown name. The - message names the metric and the backends it does implement. This - is deliberate: an earlier hand-written ``if/elif`` chain per metric - ended in a bare ``return self._compute_numpy(...)``, so an - unhandled backend was indistinguishable from the numpy default and - failed silently. Dispatching through the table fails loudly instead. + If ``self._backend`` is not the name of a backend, or is a backend + the metric has no ``_compute_*`` method for while it has no numpy + implementation either. The message names the metric and the + backends it does implement. + + Warns + ----- + UserWarning + If ``self._backend`` is a known backend the metric has no + ``_compute_*`` method for. The computation then runs in numpy, as + the earlier hand-written ``if/elif`` chain of each metric did + without notice. Backend selection never produces this state for a + built-in metric; it arises when ``_backend`` is set by hand, or in + a subclass that overrides ``compute`` and delegates here. Notes ----- @@ -795,7 +802,7 @@ def compute( built-in metric that overrides ``compute`` and delegates to ``super().compute(...)`` gets the table dispatch, which looks the method up on the instance: a ``_compute_*`` method added by the - descendant is used, and the errors above are raised for a backend + descendant is used, and the warning or errors above apply to a backend that neither it nor its parent implements. """ if not self._dispatches_via_table(): @@ -809,6 +816,20 @@ def compute( f"(or override compute)." ) implemented = [b for b in self._BACKEND_METHODS if self._implements(b)] + if self._backend in self._BACKEND_METHODS and "numpy" in implemented: + # The per-metric if/elif chains this dispatch replaces ended + # in the numpy implementation. The 0.6 series changes no + # computed value, so a known backend without a method still + # computes in numpy, now with a warning. + warnings.warn( + f"{self.name!r} has no " + f"{self._BACKEND_LABELS.get(self._backend, self._backend)} " + f"implementation: computing with numpy, as earlier " + f"versions did silently.", + UserWarning, + stacklevel=2, + ) + return self._compute_numpy(complex_signal, n_samp, transpose_axes) raise ValueError( f"{self.name!r} cannot run on backend {self._backend!r}. " f"Backends implemented for this metric: {implemented}." diff --git a/tests/test_sync.py b/tests/test_sync.py index 557e082..3f0acef 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1373,18 +1373,25 @@ def test_unknown_backend_fails_closed(self, complex_signal): assert "'not_a_backend'" in message assert "numpy" in message - def test_unimplemented_backend_fails_closed(self, complex_signal): + def test_unimplemented_backend_computes_in_numpy_with_a_warning( + self, complex_signal + ): """ - A known backend the metric does not implement must raise the same - clear error, not an AttributeError on the missing method. + A known backend the metric does not implement computes in numpy, as + before, but says so: the 0.6 series changes no computed value. """ from hypyp.sync.plv import PLV n_samp = complex_signal.shape[3] + axes = (0, 1, 3, 2) metric = PLV() metric._backend = "metal" - with pytest.raises(ValueError, match="cannot run on backend 'metal'"): - metric.compute(complex_signal, n_samp, (0, 1, 3, 2)) + with pytest.warns(UserWarning, match="'plv' has no Metal implementation"): + result = metric.compute(complex_signal, n_samp, axes) + assert isinstance(result, np.ndarray) + np.testing.assert_array_equal( + result, PLV()._compute_numpy(complex_signal, n_samp, axes) + ) def test_priority_fallback_warning_names_the_skipped_backend(self): """ @@ -1578,12 +1585,12 @@ class Alias(PLV): metric = Alias(optimization="metal") assert metric._backend == "numpy" - def test_auto_does_not_guess_numba_handled_inside_compute(self, complex_signal): + def test_auto_keeps_numba_handled_inside_compute(self, complex_signal): """ A descendant of a built-in metric that hides ``_compute_numba`` and - handles numba inside its own ``compute`` cannot be told apart from - one that only delegates. ``'auto'`` therefore falls back to numpy for - it, while an explicit request for numba is still granted. + handles numba inside its own ``compute`` still gets numba from the + CPU fallback of ``'auto'``, as before the capability check, and its + own branch runs. """ from hypyp.sync.plv import PLV @@ -1607,12 +1614,8 @@ def compute(self, complex_signal, n_samp, transpose_axes): warnings.simplefilter("ignore") automatic = OwnNumba(optimization="auto") requested = OwnNumba(optimization="numba") - assert automatic._backend == "numpy" - result = automatic.compute(complex_signal, n_samp, axes) - assert isinstance(result, np.ndarray) - np.testing.assert_array_equal( - result, PLV()._compute_numpy(complex_signal, n_samp, axes) - ) + assert automatic._backend == "numba" + assert automatic.compute(complex_signal, n_samp, axes) == "own numba" assert requested._backend == "numba" assert requested.compute(complex_signal, n_samp, axes) == "own numba" @@ -1646,11 +1649,12 @@ def _compute_numpy(self, complex_signal, n_samp, transpose_axes): assert TableOnly.supports("numpy") is True assert TableOnly.supports("numba") is False - def test_delegating_descendant_of_numpy_only_metric_never_gets_numba(self): + def test_delegating_descendant_of_numpy_only_metric_still_computes(self): """ A descendant that overrides ``compute`` only to delegate is trusted - with the backends it requests, but the automatic CPU fallback must - still not hand it a numba method that does not exist. + with numba by the automatic CPU fallback, like any class with its own + ``compute``. No numba method exists, so the dispatch computes in numpy + and warns instead of failing. """ from hypyp.sync.base import BaseMetric @@ -1674,8 +1678,9 @@ def compute(self, complex_signal, n_samp, transpose_axes): with warnings.catch_warnings(): warnings.simplefilter("ignore") metric = Delegating(optimization="auto") - assert metric._backend == "numpy" - assert metric.compute(None, 0, None) == "numpy result" + assert metric._backend == "numba" + with pytest.warns(UserWarning, match="no numba implementation"): + assert metric.compute(None, 0, None) == "numpy result" def test_supports_ignores_placeholders(self): """supports() must not count a non-callable attribute, nor the default From 03a1a8609a3aa11349ac3b0e647294700e6fe358 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 00:44:32 -0400 Subject: [PATCH 13/14] fix(sync): do not call a backend method a subclass adds without opting in The 0.6 series changes no computed value. Before the table dispatch, a _compute_* method added by a subclass of a built-in metric for a backend its parent does not implement was never called. It stays unused, and the request computes in numpy with a warning, unless the subclass sets _dispatch_via_table itself. A priority list no longer ends in numpy for a metric that has no numpy implementation. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 2 +- hypyp/sync/__init__.py | 4 ++-- hypyp/sync/base.py | 54 ++++++++++++++++++++++++++++++++++-------- tests/test_sync.py | 48 +++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eecdc5c..2aa7d1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ - `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` ### Changed -- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric (set `_dispatch_via_table = True` if the class also overrides `compute`). A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. A subclass of a built-in metric that overrides `compute` and delegates to `super().compute()` keeps the earlier results: a backend without a `_compute_*` method computes in NumPy, now with a warning, and the CPU fallback of `optimization='auto'` still selects numba for it when numba is installed. One thing is new for subclasses: a `_compute_*` method added to a subclass that keeps the inherited `compute` is now used, where it was ignored before +- `hypyp.sync`: backend dispatch now lives in `BaseMetric.compute`, and each built-in metric implements `_compute_numpy` plus the optional `_compute_numba`, `_compute_torch`, `_compute_metal` and `_compute_cuda`. This is the recommended way to write a new metric: set `_dispatch_via_table = True` in the class. A subclass written the earlier way, which overrides `compute` and does its own dispatch, is still granted the backend it requests and is not subject to the capability check, whether it derives from `BaseMetric` or from a built-in metric. A subclass of a built-in metric that overrides `compute` and delegates to `super().compute()` keeps the earlier results: a backend without a `_compute_*` method computes in NumPy, now with a warning, and the CPU fallback of `optimization='auto'` still selects numba for it when numba is installed. A `_compute_*` method that a subclass adds for a backend its parent does not implement was never called before and is still not, unless the subclass sets `_dispatch_via_table = True` itself; a method of the parent that the subclass overrides is used, as before - The whole code base is formatted with `ruff format` (line length 88). The change is purely cosmetic: the syntax tree of every file is unchanged, apart from whitespace inside docstrings, and the Python examples of `hypyp/sync/README.md` are formatted too. The vendored `hypyp/ext` and the tutorial notebooks are left untouched. The formatting commit is listed in `.git-blame-ignore-revs`, so `git blame` skips it (run `git config blame.ignoreRevsFile .git-blame-ignore-revs` once in your clone; GitHub applies it automatically) - The CI now checks formatting (`ruff format --check`) and a small set of lint rules that only catch certain bugs: syntax errors, invalid comparisons and undefined names. Ruff comes from the new `lint` dependency group, which the `dev` group includes - `black` is removed from the `dev` dependency group, since `ruff format` replaces it diff --git a/hypyp/sync/__init__.py b/hypyp/sync/__init__.py index 1b0b419..10a1321 100644 --- a/hypyp/sync/__init__.py +++ b/hypyp/sync/__init__.py @@ -7,8 +7,8 @@ Public API ---------- ``BaseMetric`` - Abstract base class. Concrete metrics inherit from it and implement - ``BaseMetric.compute``. + Base class. Concrete metrics inherit from it and implement + ``_compute_numpy`` plus the optional accelerated ``_compute_*`` methods. Concrete metric classes (one per file): diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index c1062d6..694a727 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -286,6 +286,10 @@ class BaseMetric(ABC): ``_compute_torch``, ``_compute_metal`` and ``_compute_cuda``. Backend selection, capability checks and dispatch are then handled here. + A subclass of an existing metric that adds a ``_compute_*`` method for a + backend its parent does not implement must set ``_dispatch_via_table = + True`` itself for that method to be used. + A subclass that overrides ``compute`` and does not set the flag itself follows the earlier contract, whether it derives from this class or from a built-in metric: it is granted whatever backend is requested and @@ -400,12 +404,37 @@ def _implements(cls, backend: str) -> bool: The default ``_compute_numpy`` of this class only raises, and a non-callable attribute is a placeholder: neither is an implementation. + + A backend also counts only if the class that adopted the table + dispatch (the one that sets ``_dispatch_via_table``) already had a + method for it. Before the table, the ``compute`` of each built-in + metric called a fixed set of ``_compute_*`` methods: a descendant + could override one of them, but a method it added for another backend + was never called. The 0.6 series changes no computed value, so such a + method stays unused until the descendant sets the flag itself. """ + + def is_implementation(candidate) -> bool: + return callable(candidate) and ( + candidate is not BaseMetric.__dict__["_compute_numpy"] + ) + method = cls._BACKEND_METHODS.get(backend) - implementation = getattr(cls, method, None) if method else None - return callable(implementation) and ( - implementation is not BaseMetric._compute_numpy - ) + if not method or not is_implementation(getattr(cls, method, None)): + return False + owner, _ = cls._dispatch_owner() + if owner is None: + return True + # The flag may sit on a mixin: the class that adopted the dispatch is + # then the metric class that brought the mixin in. + adopters = [ + k for k in cls.__mro__ if issubclass(k, BaseMetric) and owner in k.__mro__ + ] + mro = cls.__mro__ + for klass in mro[mro.index(adopters[-1]) :]: + if method in klass.__dict__: + return is_implementation(klass.__dict__[method]) + return False @classmethod def _dispatch_owner(cls) -> tuple: @@ -521,8 +550,10 @@ def _resolve_optimization( ----- Fallback cascade for ``'auto'`` (per-metric, per-platform): Iterates ``AUTO_PRIORITY[metric][platform]`` and returns the - first available backend. Falls back to numba → numpy if no - GPU backend is available. + first available backend the metric implements. An available + backend the metric does not implement ends the search in numpy + with a warning. Falls back to numba → numpy if no GPU backend + is available. Fallback cascade for explicit backends when unavailable: requested backend → numpy (with UserWarning) @@ -669,7 +700,9 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: # series does not change computed values, so the selection # still ends in numpy, now with a warning. Moving on to the # next backend of the list instead is left to 0.7.0. - if label and available.get(backend): + # (A metric without a numpy implementation could not exist + # in those versions, so for it the search simply goes on.) + if label and available.get(backend) and cls._implements("numpy"): warnings.warn( f"{cls.name!r} has no {label} implementation: computing " f"with numpy, as earlier versions did silently. The " @@ -801,9 +834,10 @@ def compute( returned ``None``; it still does for such a subclass. A subclass of a built-in metric that overrides ``compute`` and delegates to ``super().compute(...)`` gets the table dispatch, which looks the - method up on the instance: a ``_compute_*`` method added by the - descendant is used, and the warning or errors above apply to a backend - that neither it nor its parent implements. + method up on the instance, so a ``_compute_*`` method it overrides is + used. A method it adds for a backend its parent does not implement is + used only if the subclass sets ``_dispatch_via_table`` itself; + otherwise the warning or errors above apply to that backend. """ if not self._dispatches_via_table(): # Reached through super().compute() from a subclass that does its diff --git a/tests/test_sync.py b/tests/test_sync.py index 3f0acef..3355592 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1619,6 +1619,54 @@ def compute(self, complex_signal, n_samp, transpose_axes): assert requested._backend == "numba" assert requested.compute(complex_signal, n_samp, axes) == "own numba" + @pytest.mark.parametrize("delegates", [False, True]) + def test_backend_method_added_by_a_subclass_needs_the_flag( + self, complex_signal, delegates + ): + """ + Before the table dispatch, a ``_compute_metal`` added to a subclass of + PLV was never called: the request computed in numpy. That result is + kept, with a warning, whether the subclass inherits ``compute`` or + overrides it only to delegate. The added method is used once the + subclass sets ``_dispatch_via_table`` itself. + """ + from hypyp.sync.plv import PLV + + class AddsMetal(PLV): + def _compute_metal(self, complex_signal, n_samp, transpose_axes): + return "added Metal" + + def _compute_numpy(self, complex_signal, n_samp, transpose_axes): + # An overridden method of the parent is still honoured. + return 2 * super()._compute_numpy( + complex_signal, n_samp, transpose_axes + ) + + if delegates: + + class AddsMetal(AddsMetal): # noqa: F811 + def compute(self, complex_signal, n_samp, transpose_axes): + return super().compute(complex_signal, n_samp, transpose_axes) + + class Migrated(AddsMetal): + _dispatch_via_table = True + + n_samp = complex_signal.shape[3] + axes = (0, 1, 3, 2) + expected = 2 * PLV()._compute_numpy(complex_signal, n_samp, axes) + with patch("hypyp.sync.base.METAL_AVAILABLE", True): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + metric = AddsMetal(optimization="metal") + result = metric.compute(complex_signal, n_samp, axes) + migrated = Migrated(optimization="metal") + assert any("no Metal implementation" in str(w.message) for w in caught) + assert isinstance(result, np.ndarray) + np.testing.assert_array_equal(result, expected) + assert Migrated.supports("metal") is True + assert migrated._backend == "metal" + assert migrated.compute(complex_signal, n_samp, axes) == "added Metal" + def test_dispatch_flag_set_by_a_mixin_listed_after_the_base(self): """ A mixin that carries the flag can come after ``BaseMetric`` in the From 9e4d4fece8cdcbf07f30eca9a8a5c836a7811569 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 00:57:28 -0400 Subject: [PATCH 14/14] fix(sync): recognise a backend method declared as a classmethod The ownership lookup read the raw descriptor, which is not callable. Three docstrings now state that a metric without a numpy implementation does not end a priority list in numpy. Co-Authored-By: Claude Fable 5.1 --- hypyp/sync/base.py | 12 ++++++++---- tests/test_sync.py | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 694a727..68cb1a3 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -433,7 +433,9 @@ def is_implementation(candidate) -> bool: mro = cls.__mro__ for klass in mro[mro.index(adopters[-1]) :]: if method in klass.__dict__: - return is_implementation(klass.__dict__[method]) + # getattr, not __dict__: a classmethod or staticmethod is + # callable only once its descriptor is resolved. + return is_implementation(getattr(klass, method)) return False @classmethod @@ -552,7 +554,8 @@ def _resolve_optimization( Iterates ``AUTO_PRIORITY[metric][platform]`` and returns the first available backend the metric implements. An available backend the metric does not implement ends the search in numpy - with a warning. Falls back to numba → numpy if no GPU backend + with a warning, unless the metric has no numpy implementation, + in which case the search goes on. Falls back to numba → numpy if no GPU backend is available. Fallback cascade for explicit backends when unavailable: @@ -646,7 +649,8 @@ def _resolve_auto(cls, priority: Optional[list] = None) -> tuple: Narval A100 benchmarks. Iterates the priority list and returns the first available backend the metric implements. An available backend the metric does not implement ends the search in numpy with - a warning (see the comment in the loop). + a warning, unless the metric has no numpy implementation (see the + comment in the loop). Parameters ---------- @@ -875,7 +879,7 @@ def _compute_numpy( self, complex_signal: np.ndarray, n_samp: int, transpose_axes: tuple ) -> np.ndarray: """ - Reference implementation, in numpy. Always available. + Reference implementation, in numpy. Every metric should provide this: it is the correctness oracle the accelerated backends are validated against, and the fallback target diff --git a/tests/test_sync.py b/tests/test_sync.py index 3355592..32669df 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1730,6 +1730,22 @@ def compute(self, complex_signal, n_samp, transpose_axes): with pytest.warns(UserWarning, match="no numba implementation"): assert metric.compute(None, 0, None) == "numpy result" + def test_classmethod_implementation_is_recognised(self): + """A ``_compute_*`` method declared as a classmethod is an + implementation like any other.""" + from hypyp.sync.base import BaseMetric + + class ClassLevel(BaseMetric): + name = "class_level" + _dispatch_via_table = True + + @classmethod + def _compute_numpy(cls, complex_signal, n_samp, transpose_axes): + return "numpy result" + + assert ClassLevel.supports("numpy") is True + assert ClassLevel().compute(None, 0, None) == "numpy result" + def test_supports_ignores_placeholders(self): """supports() must not count a non-callable attribute, nor the default ``_compute_numpy`` of the base class, as an implementation."""