diff --git a/CHANGELOG.md b/CHANGELOG.md index 470188f..ea0a813 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ - `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 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 (#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) +- `import hypyp.analyses` failed when the import of torch, numba, cupy or the Metal bindings raised anything other than `ImportError`, for example the `OSError` of a missing shared library. The backend is now reported as unavailable, with a warning that gives the original error. An installed package that fails with an `ImportError` used to be skipped silently and now gets the same warning; a package that is not installed is still skipped silently. A package that imports but is broken further down is not covered +- `hypyp.sync.get_metric` rejected the names `envelope_corr`, `pow_corr` and `imaginary_coh`, which `compute_sync` accepts and the documentation uses. It now accepts them; the mapping is `hypyp.sync.METRIC_ALIASES`, and a metric registered in `METRICS` under one of these names takes precedence +- `hypyp.sync`: the warnings for a missing numba or torch suggested a `poetry install` command that no longer exists; they now give `pip install "hypyp[numba]"` and `pip install "hypyp[torch]"` +- `hypyp.fnirs.Recording.load_raw` raised a `TypeError` for a `Raw` object without `subject_info`, as built in memory with `mne.io.RawArray`, and a `KeyError` when `subject_info` had no `his_id`. Both now get the random label that was intended +- `hypyp.fnirs.Study.compute_wtcs_shuffle` raised a `TypeError` when given `with_intra=False`, so the keyword arguments of `compute_wtcs` could not be reused. That redundant argument is now accepted; `with_intra=True` is still refused +- `XDFImport(select_matches=[...])`: an integer that is not the id of a stream raised a bare `KeyError`; it now raises the same `ValueError` as an unknown stream name. A numpy integer is recognised as a stream id - `compute_sync` and `pair_connectivity` reported every `ValueError` as `Metric type "..." not supported`, so an unknown `optimization` was announced as an unsupported metric. Only an unknown metric name gets that message now; any other error keeps its own (#306) - `pair_connectivity` crashed with an `IndexError` when the frequencies were given as a list and the data had a single epoch, a single channel per participant or a single frequency. Results for data without such a dimension are unchanged - `utils.normalizing` raised an `UnboundLocalError` for a `type` name other than `'Zscore'` or `'Logratio'`; it now raises a `ValueError` naming the two valid types diff --git a/hypyp/fnirs/recording.py b/hypyp/fnirs/recording.py index 3686f1d..2eea5b8 100644 --- a/hypyp/fnirs/recording.py +++ b/hypyp/fnirs/recording.py @@ -172,7 +172,9 @@ def preprocess_step_choices(self): def _fill_subject_label(self): if self.subject_label == "": - self.subject_label = self.mne_raw.info["subject_info"]["his_id"] + # subject_info is None for a Raw built in memory, and his_id is optional + subject_info = self.mne_raw.info["subject_info"] or {} + self.subject_label = subject_info.get("his_id") or "" if self.subject_label == "": self.subject_label = generate_random_label(10) diff --git a/hypyp/fnirs/study.py b/hypyp/fnirs/study.py index 4610aa5..ae3f29f 100644 --- a/hypyp/fnirs/study.py +++ b/hypyp/fnirs/study.py @@ -191,11 +191,18 @@ def get_dyads_shuffle(self) -> List[Dyad]: # TODO add as argument the number of shuffle dyads def compute_wtcs_shuffle(self, *args, **kwargs): """ - Wraps the `compute_wtcs` of all the dyads_shuffle. Arguments are directly passed to the dyads_shuffle method + Wraps the `compute_wtcs` of all the dyads_shuffle. Arguments are directly passed to the dyads_shuffle method, + which is always called with `with_intra=False`. Passing `with_intra=False` here is accepted and changes nothing. Returns: self: the Study object itself. Useful for chaining operations """ + # with_intra=False is passed below; drop a redundant one from the caller, + # so that the keyword arguments of compute_wtcs can be reused as they are + if kwargs.get("with_intra") is False: + kwargs = { + key: value for key, value in kwargs.items() if key != "with_intra" + } self.dyads_shuffled = self.get_dyads_shuffle() for dyad_shuffle in self.dyads_shuffled: dyad_shuffle.compute_wtcs(*args, **kwargs, with_intra=False) diff --git a/hypyp/sync/__init__.py b/hypyp/sync/__init__.py index 10a1321..dd27bd8 100644 --- a/hypyp/sync/__init__.py +++ b/hypyp/sync/__init__.py @@ -74,6 +74,13 @@ "powcorr": PowCorr, } +# Alternative names of the metrics, accepted by get_metric and compute_sync +METRIC_ALIASES = { + "envelope_corr": "envcorr", + "pow_corr": "powcorr", + "imaginary_coh": "imcoh", +} + __all__ = [ # Base classes and utilities "BaseMetric", @@ -94,6 +101,7 @@ "PowCorr", # Registry "METRICS", + "METRIC_ALIASES", "get_metric", ] @@ -108,7 +116,8 @@ def get_metric( ---------- mode : str Name of the connectivity metric. One of: 'plv', 'ccorr', 'accorr', - 'coh', 'imcoh', 'pli', 'wpli', 'envcorr', 'powcorr'. + 'coh', 'imcoh', 'pli', 'wpli', 'envcorr', 'powcorr'. The aliases + 'imaginary_coh', 'envelope_corr' and 'pow_corr' are accepted too. optimization : str, optional Optimization strategy. Options: None, 'auto', 'numba', 'torch', 'metal', 'cuda_kernel'. See BaseMetric for fallback behavior. @@ -134,6 +143,10 @@ def get_metric( ... priority=['numba', 'metal']) # custom priority """ mode_lower = mode.lower() + # a name registered in METRICS wins, so an alias never hides a metric + # that a user registered under that name + if mode_lower not in METRICS: + mode_lower = METRIC_ALIASES.get(mode_lower, mode_lower) if mode_lower not in METRICS: available = ", ".join(METRICS.keys()) raise ValueError(f"Unknown metric mode '{mode}'. Available: {available}") diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 68cb1a3..516624e 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -50,23 +50,45 @@ # Check optional dependency availability +def _is_absent(exc: BaseException, package: str) -> bool: + """True if `exc` only says that the optional `package` is not installed. + + Any other failure (a missing shared library, an incompatible version, an + error while probing a device) means the package is installed but + unusable, which deserves a warning instead of a silent fallback. + """ + return isinstance(exc, ModuleNotFoundError) and exc.name == package + + try: import torch TORCH_AVAILABLE = True MPS_AVAILABLE = torch.backends.mps.is_available() CUDA_AVAILABLE = torch.cuda.is_available() -except ImportError: +except Exception as exc: TORCH_AVAILABLE = False MPS_AVAILABLE = False CUDA_AVAILABLE = False + if not _is_absent(exc, "torch"): + warnings.warn( + "torch is installed but could not be loaded " + f"({type(exc).__name__}: {exc}). The torch backends are disabled.", + UserWarning, + ) try: import numba NUMBA_AVAILABLE = True -except ImportError: +except Exception as exc: NUMBA_AVAILABLE = False + if not _is_absent(exc, "numba"): + warnings.warn( + "numba is installed but could not be loaded " + f"({type(exc).__name__}: {exc}). The numba backend is disabled.", + UserWarning, + ) # Custom kernel backends from .kernels import METAL_AVAILABLE, CUPY_AVAILABLE @@ -593,7 +615,7 @@ def _resolve_optimization( return "numba", "cpu" warnings.warn( "numba not installed, falling back to numpy. " - "Install with: poetry install --with optim_numba", + 'Install with: pip install "hypyp[numba]"', UserWarning, stacklevel=3, ) @@ -604,7 +626,7 @@ def _resolve_optimization( return cls._resolve_torch() warnings.warn( "torch not installed, falling back to numpy. " - "Install with: poetry install --with optim_torch", + 'Install with: pip install "hypyp[torch]"', UserWarning, stacklevel=3, ) diff --git a/hypyp/sync/kernels/__init__.py b/hypyp/sync/kernels/__init__.py index 7c71bed..9399bcf 100644 --- a/hypyp/sync/kernels/__init__.py +++ b/hypyp/sync/kernels/__init__.py @@ -6,20 +6,38 @@ (e.g., PLI, wPLI — non-linear per-timepoint operations). """ +import warnings + +# A package that is not installed is skipped silently; one that is installed +# but fails to load (a missing shared library, for instance) disables the +# backend with a warning instead of making `import hypyp` fail. + # Metal availability (Apple Silicon via PyObjC) try: import Metal as _Metal METAL_AVAILABLE = True -except ImportError: +except Exception as exc: METAL_AVAILABLE = False + if not (isinstance(exc, ModuleNotFoundError) and exc.name == "Metal"): + warnings.warn( + "The Metal bindings are installed but could not be loaded " + f"({type(exc).__name__}: {exc}). The Metal backend is disabled.", + UserWarning, + ) # CUDA availability (NVIDIA via CuPy) try: import cupy as _cp CUPY_AVAILABLE = True -except ImportError: +except Exception as exc: CUPY_AVAILABLE = False + if not (isinstance(exc, ModuleNotFoundError) and exc.name == "cupy"): + warnings.warn( + "cupy is installed but could not be loaded " + f"({type(exc).__name__}: {exc}). The CUDA kernel backend is disabled.", + UserWarning, + ) __all__ = ["METAL_AVAILABLE", "CUPY_AVAILABLE"] diff --git a/hypyp/xdf/xdf_import.py b/hypyp/xdf/xdf_import.py index f87bf35..f5f6f81 100644 --- a/hypyp/xdf/xdf_import.py +++ b/hypyp/xdf/xdf_import.py @@ -203,9 +203,13 @@ def select_streams_by_matches(self, keyword_matches: list): for keyword_match in keyword_matches: # match stream_id found_idx = None - if type(keyword_match) == int: - stream_id = keyword_match - found_idx = self.map_id_to_idx[stream_id] + # a numpy integer is a stream id too; a bool is not + if isinstance(keyword_match, (int, np.integer)) and not isinstance( + keyword_match, bool + ): + stream_id = int(keyword_match) + # None for an unknown id, reported below like an unknown name + found_idx = self.map_id_to_idx.get(stream_id) # match stream name else: diff --git a/tests/test_fnirs.py b/tests/test_fnirs.py index 559dcbe..40363a2 100644 --- a/tests/test_fnirs.py +++ b/tests/test_fnirs.py @@ -821,3 +821,63 @@ def test_download_demos(): # def test_load_lionirs(): + + +def _raw_array_without_subject_info(): + info = mne.create_info(["Foo 1", "Foo 2"], sfreq=10, ch_types="eeg") + return mne.io.RawArray(np.zeros((2, 100)), info) + + +def test_load_raw_without_subject_info_gets_random_label(): + raw = _raw_array_without_subject_info() + assert raw.info["subject_info"] is None + + recording = Recording().load_raw(raw, preprocess=False) + assert len(recording.subject_label) == 10 + + # a second recording gets its own label + other = Recording().load_raw(_raw_array_without_subject_info(), preprocess=False) + assert other.subject_label != recording.subject_label + + +def test_load_raw_subject_info_without_his_id_gets_random_label(): + raw = _raw_array_without_subject_info() + raw.info["subject_info"] = dict(id=1) + + recording = Recording().load_raw(raw, preprocess=False) + assert len(recording.subject_label) == 10 + + +def test_load_raw_keeps_given_label_and_his_id(): + raw = _raw_array_without_subject_info() + assert ( + Recording(subject_label="s1").load_raw(raw, preprocess=False).subject_label + == "s1" + ) + + raw.info["subject_info"] = dict(his_id="from_file") + assert Recording().load_raw(raw, preprocess=False).subject_label == "from_file" + + +def test_study_wtc_shuffle_accepts_redundant_with_intra(): + from pandas.testing import assert_frame_equal + + dyads = [Dyad(*get_test_recordings()) for _ in range(3)] + wtcs_kwargs = dict(ch_match=get_test_ch_match_one()) + + reference = Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs) + assert len(reference.dyads_shuffled) == 6 + + # the keyword arguments of compute_wtcs can be reused as they are + study = Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs, with_intra=False) + assert len(study.dyads_shuffled) == 6 + for shuffled, expected in zip(study.dyads_shuffled, reference.dyads_shuffled): + assert len(expected.wtcs) == 1 + assert len(expected.df) > 0 + assert shuffled.label == expected.label + assert_frame_equal(shuffled.df, expected.df) + np.testing.assert_array_equal(shuffled.wtcs[0].W, expected.wtcs[0].W) + + # with_intra=True is still refused, as before + with pytest.raises(TypeError, match="with_intra"): + Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs, with_intra=True) diff --git a/tests/test_sync.py b/tests/test_sync.py index 32669df..e64960b 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1798,3 +1798,125 @@ class EmptyMetric(BaseMetric): n_samp = complex_signal.shape[3] with pytest.raises(NotImplementedError, match="_compute_numpy"): EmptyMetric().compute(complex_signal, n_samp, (0, 1, 3, 2)) + + +# --------------------------------------------------------------------------- +# Robustness of the optional-dependency probes and of the metric registry +# --------------------------------------------------------------------------- + + +def _import_hypyp_with_fake_package(tmp_path, module, flag, init_source): + """Import hypyp in a fresh interpreter where `module` resolves to a fake + package whose `__init__` is `init_source`. Returns the availability flag + and the messages of the warnings that name the module.""" + import json + import os + import subprocess + import sys + + package = tmp_path / module + package.mkdir() + (package / "__init__.py").write_text(init_source) + code = ( + "import json, warnings\n" + "with warnings.catch_warnings(record=True) as caught:\n" + " warnings.simplefilter('always')\n" + " import hypyp.analyses\n" + " from hypyp.sync import base, kernels\n" + f"messages = [str(w.message) for w in caught if '{module}' in str(w.message)]\n" + f"print('RESULT', json.dumps([bool({flag}), messages]))\n" + ) + env = dict(os.environ) + env["PYTHONPATH"] = os.pathsep.join( + [str(tmp_path)] + [p for p in [env.get("PYTHONPATH")] if p] + ) + result = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True, env=env + ) + assert result.returncode == 0, result.stderr[-2000:] + line = [l for l in result.stdout.splitlines() if l.startswith("RESULT ")][-1] + return json.loads(line[len("RESULT ") :]) + + +OPTIONAL_PACKAGES = [ + ("torch", "base.TORCH_AVAILABLE"), + ("numba", "base.NUMBA_AVAILABLE"), + ("cupy", "kernels.CUPY_AVAILABLE"), + ("Metal", "kernels.METAL_AVAILABLE"), +] + + +@pytest.mark.parametrize("module, flag", OPTIONAL_PACKAGES) +@pytest.mark.parametrize( + "error", + [ + 'OSError("simulated broken install: missing shared library")', + 'ImportError("simulated broken install: cannot load a symbol")', + 'ModuleNotFoundError("simulated broken install", name="a_dependency")', + 'RuntimeError("simulated broken install: version mismatch")', + ], +) +def test_broken_optional_dependency_does_not_break_import( + tmp_path, module, flag, error +): + """An optional package that is installed but fails to load must leave + hypyp importable, with the backend reported as unavailable and a warning + that names the package and gives the original error.""" + available, messages = _import_hypyp_with_fake_package( + tmp_path, module, flag, f"raise {error}\n" + ) + assert available is False + assert len(messages) == 1 + assert "simulated broken install" in messages[0] + + +@pytest.mark.parametrize("module, flag", OPTIONAL_PACKAGES) +def test_absent_optional_dependency_stays_silent(tmp_path, module, flag): + """A package that is simply not installed disables its backend without + any warning, as before.""" + available, messages = _import_hypyp_with_fake_package( + tmp_path, + module, + flag, + f'raise ModuleNotFoundError("No module named {module!r}", name="{module}")\n', + ) + assert available is False + assert messages == [] + + +@pytest.mark.parametrize( + "alias, mode", + [("envelope_corr", "envcorr"), ("pow_corr", "powcorr"), ("imaginary_coh", "imcoh")], +) +def test_get_metric_accepts_the_aliases_of_compute_sync(alias, mode): + assert type(get_metric(alias)) is METRICS[mode] + assert type(get_metric(alias.upper())) is METRICS[mode] + + +def test_get_metric_registered_name_wins_over_alias(): + """A metric that a user registered under the name of an alias is still + the one returned.""" + + class Custom(METRICS["plv"]): + pass + + with patch.dict(METRICS, {"pow_corr": Custom}): + assert type(get_metric("pow_corr")) is Custom + assert type(get_metric("pow_corr")) is METRICS["powcorr"] + + +def test_get_metric_unknown_mode_still_raises(): + with pytest.raises(ValueError, match="Unknown metric mode 'nope'"): + get_metric("nope") + + +@pytest.mark.parametrize("backend", ["numba", "torch"]) +def test_missing_backend_hint_names_the_pip_extra(backend): + flag = f"hypyp.sync.base.{backend.upper()}_AVAILABLE" + with patch(flag, False): + with pytest.warns(UserWarning) as record: + metric = get_metric("plv", optimization=backend) + assert metric._backend == "numpy" + messages = " ".join(str(w.message) for w in record) + assert f'pip install "hypyp[{backend}]"' in messages + assert "poetry" not in messages diff --git a/tests/test_xdf.py b/tests/test_xdf.py index c16a017..30b3153 100644 --- a/tests/test_xdf.py +++ b/tests/test_xdf.py @@ -316,3 +316,17 @@ def test_montage(): ] ) xdf.set_montage("standard_1020") + + +def test_match_unknown_stream_id_raises_value_error(): + with pytest.raises(ValueError, match="No stream matching keyword '99'"): + XDFImport(file_path, select_matches=[99]) + + +def test_match_stream_by_numpy_integer_id(): + import numpy as np + + expected = XDFImport(file_path, select_matches=[STREAM_ID_EEG_1]) + for stream_id in (np.int64(STREAM_ID_EEG_1), np.int32(STREAM_ID_EEG_1)): + xdf = XDFImport(file_path, select_matches=[stream_id]) + assert xdf.selected_stream_indices == expected.selected_stream_indices