diff --git a/.context/plan.md b/.context/plan.md index 412036bc..d58b57a6 100644 --- a/.context/plan.md +++ b/.context/plan.md @@ -30,6 +30,10 @@ - [x] Multi-model AMICA per-model bias `c` update (issue #27): ported to both backends, guarded no-op for `n_models=1`; controlled A/B shows +0.011 cross-corr, gap is intrinsic partition ambiguity (see `.context/issue-27/multimodel_c_update.md`). +- [x] Degenerate-fit contract (issue #50): the `AMICA` wrapper no longer marks a degenerate fit + (`stop_reason` nan_ll/singular_ll) as usable. `fit` sets `is_fitted_` only on a converged fit + and exposes `converged_`/`stop_reason_`; `transform`/`get_mixing_matrix`/`get_unmixing_matrix`/ + `save` raise a clear degenerate error (mirroring `state_dict`) instead of returning NaN sources. - [x] Best-iterate safeguard (issue #51): `AMICATorchNG.fit` returns the highest-LL iterate (`keep_best`, `final_ll_`), not the last, so a late Newton-fallback overshoot no longer leaves the model below a peak it reached. Root cause was return-last, not a bad basin (the sole diff --git a/AGENTS.md b/AGENTS.md index 7cb8835a..2bc67a7f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,7 +60,9 @@ mixture updates, digamma rho update, symmetric-ZCA sphere, Jacobian LL) brought and the legacy NumPy `pyAMICA.py` to Fortran's solution (LL ~ -3.40, Hungarian-matched component correlation ~0.997, > 0.95 gate cleared; root cause in `.context/issue-24/`). Also resolved: Newton stability (posdef, 0 fallbacks), backend consolidation (#32/#31), NumPy CLI save/load format (#30), -and NG save/load persistence (#36). +NG save/load persistence (#36), and the degenerate-fit contract (#50: the `AMICA` wrapper marks a +degenerate fit unusable via `converged_`/`stop_reason_` and refuses `transform`/`get_*`/`save`, +instead of returning NaN sources). **Adaptive-PDF selection: DONE (#26).** `AMICATorchNG` now supports all five `amica15.f90` source-density families via `pdftype`: 0 generalized Gaussian (default, unchanged), 2 Gaussian, diff --git a/pyAMICA/amica.py b/pyAMICA/amica.py index 0ff4715f..2ad70f63 100644 --- a/pyAMICA/amica.py +++ b/pyAMICA/amica.py @@ -51,7 +51,17 @@ class AMICA: model_ : AMICATorchNG The underlying PyTorch model is_fitted_ : bool - Whether the model has been fitted + Whether a *usable* model is available. ``fit`` sets this True only when + the fit converged normally; a degenerate fit (see ``converged_``) leaves + it False, and ``transform``/``get_mixing_matrix``/``get_unmixing_matrix``/ + ``save`` refuse such a model (issue #50). + converged_ : bool + Whether the last ``fit`` ended on a usable stop rather than a degenerate + one (``stop_reason_`` not in ``nan_ll``/``singular_ll``). A degenerate fit + holds non-finite parameters and would produce NaN sources (issue #50). + stop_reason_ : str or None + Why the last ``fit`` stopped (the backend ``stop_reason``): e.g. + ``"max_iter"``, ``"lrate_floor"``, ``"nan_ll"``, ``"singular_ll"``. ll_history_ : list Log-likelihood history during training (the true per-iteration trajectory; may dip below its peak on a late overshoot) @@ -95,6 +105,8 @@ def __init__( self.is_fitted_ = False self.ll_history_ = [] self.final_ll_ = None + self.stop_reason_ = None + self.converged_ = False def _select_device(self, ng_dtype) -> object: """Resolve the compute device, applying the MPS/float64 fallback. @@ -172,7 +184,15 @@ def fit( # Setup device (with the MPS/float64 parity fallback, see _select_device). device = self._select_device(kwargs.get("dtype", _NG_DEFAULT_DTYPE)) - self.model_ = AMICATorchNG( + # Build and train the backend on a LOCAL reference first, and only + # publish it to self (and derive the fitted-state attributes) once + # fit() returns. If the backend constructor or fit() raises mid-training + # (a numerical crash, OOM, singular sphere, interrupt, ...), self is left + # untouched: a first fit keeps model_ is None (so the output methods + # raise a clean "not fitted"), and a refit keeps the previous, known-good + # model rather than a half-trained one falsely marked usable (issue #50 + # silent-failure review). + backend = AMICATorchNG( n_channels=n_channels, n_models=self.n_models, n_mix=self.n_mix, @@ -183,14 +203,47 @@ def fit( device=device, **kwargs, ) - self.model_.fit(X, max_iter=max_iter, verbose=self.verbose) - - self.ll_history_ = self.model_.ll_history - self.final_ll_ = self.model_.final_ll_ - self.is_fitted_ = True + backend.fit(X, max_iter=max_iter, verbose=self.verbose) + + self.model_ = backend + self.ll_history_ = backend.ll_history + self.final_ll_ = backend.final_ll_ + self.stop_reason_ = backend.stop_reason + self.converged_ = self.stop_reason_ not in AMICATorchNG._DEGENERATE_STOP_REASONS + # A degenerate fit (nan_ll/singular_ll) holds non-finite parameters and + # would return NaN sources, so it is not a usable model: is_fitted_ stays + # False and the output methods refuse it (issue #50). stop_reason_/ + # converged_ stay set for inspection. + self.is_fitted_ = self.converged_ + if not self.converged_: + logger.warning( + "AMICA.fit ended degenerate (stop_reason=%r) at iteration %d: the " + "model holds non-finite parameters and cannot transform. Inspect " + "stop_reason_/ll_history_; lower lrate, disable Newton, or check " + "data conditioning, then refit.", + self.stop_reason_, + backend.iteration, + ) return self + def _check_usable(self, action: str) -> None: + """Raise if the model cannot produce valid output: either never fitted, + or the fit ended degenerate (``nan_ll``/``singular_ll``), leaving + non-finite parameters that would yield NaN sources. This mirrors + :meth:`AMICATorchNG.state_dict`'s refusal to serialize a degenerate model + (issue #50): a diverged fit fails loudly here instead of silently + returning garbage.""" + if self.model_ is None: + raise ValueError(f"Model must be fitted before {action}.") + if self.stop_reason_ in AMICATorchNG._DEGENERATE_STOP_REASONS: + raise RuntimeError( + f"Refusing to {action}: fit ended degenerate " + f"(stop_reason={self.stop_reason_!r}), so the model holds " + f"non-finite parameters and would produce NaN output. Lower " + f"lrate, disable Newton, or check data conditioning, then refit." + ) + def transform(self, X: np.ndarray, model_idx: int = 0) -> np.ndarray: """ Transform data to source space. @@ -207,8 +260,7 @@ def transform(self, X: np.ndarray, model_idx: int = 0) -> np.ndarray: S : np.ndarray Sources of shape (n_sources, n_samples) """ - if not self.is_fitted_: - raise ValueError("Model must be fitted before transform") + self._check_usable("transform") return self.model_.transform(X, model_idx=model_idx) @@ -245,8 +297,7 @@ def get_mixing_matrix(self, model_idx: int = 0) -> np.ndarray: A : np.ndarray Mixing matrix of shape (n_channels, n_sources) """ - if not self.is_fitted_: - raise ValueError("Model must be fitted before getting mixing matrix") + self._check_usable("get the mixing matrix") return self.model_.get_mixing_matrix(model_idx=model_idx) @@ -264,8 +315,7 @@ def get_unmixing_matrix(self, model_idx: int = 0) -> np.ndarray: W : np.ndarray Unmixing matrix of shape (n_sources, n_channels) """ - if not self.is_fitted_: - raise ValueError("Model must be fitted before getting unmixing matrix") + self._check_usable("get the unmixing matrix") return self.model_.get_unmixing_matrix(model_idx=model_idx) @@ -285,8 +335,7 @@ def save(self, filepath: str) -> None: filepath : str Destination path (a ``.pt`` file by convention). """ - if not self.is_fitted_: - raise ValueError("Model must be fitted before saving") + self._check_usable("save") payload = { "format_version": 1, @@ -351,7 +400,13 @@ def load( ) model.ll_history_ = model.model_.ll_history model.final_ll_ = model.model_.final_ll_ - model.is_fitted_ = True + # state_dict() refuses to serialize a degenerate model, so a loaded model + # is always usable; carry its stop_reason through for inspection anyway. + model.stop_reason_ = model.model_.stop_reason + model.converged_ = ( + model.stop_reason_ not in AMICATorchNG._DEGENERATE_STOP_REASONS + ) + model.is_fitted_ = model.converged_ return model @classmethod diff --git a/pyAMICA/tests/torch_tests/test_amica_ng_wrapper.py b/pyAMICA/tests/torch_tests/test_amica_ng_wrapper.py index 0a922576..b0066f74 100644 --- a/pyAMICA/tests/torch_tests/test_amica_ng_wrapper.py +++ b/pyAMICA/tests/torch_tests/test_amica_ng_wrapper.py @@ -67,6 +67,11 @@ def test_ng_save_load_roundtrip(fitted_ng, real_data, tmp_path): # round-trip -- use it, not ll_history_[-1], as the model's log-likelihood. assert fitted_ng.final_ll_ is not None assert loaded.final_ll_ == fitted_ng.final_ll_ + # converged_/stop_reason_ (issue #50) round-trip too (a saved model is always + # converged, since state_dict refuses degenerate ones). + assert loaded.converged_ == fitted_ng.converged_ + assert loaded.stop_reason_ == fitted_ng.stop_reason_ + assert loaded.converged_ is True # torch.save/load restores tensors bit-exactly and CPU matmul is # deterministic, so transform() on the restored tensors reproduces the @@ -148,3 +153,62 @@ def test_ng_wrapper_fit_transform_real_data(fitted_ng, real_data): assert W.shape == (NW, NW) assert np.isfinite(A).all() assert np.isfinite(W).all() + + +def test_fit_exposes_converged_and_stop_reason(fitted_ng): + """A normal fit is marked usable and exposes its stop reason (issue #50): + converged_ is True, is_fitted_ is True, and stop_reason_ is a non-degenerate + marker.""" + assert fitted_ng.converged_ is True + assert fitted_ng.is_fitted_ is True + assert fitted_ng.stop_reason_ not in ("nan_ll", "singular_ll") + assert fitted_ng.stop_reason_ is not None + + +def test_unfitted_output_raises_not_fitted(): + """Before any fit, the output methods raise a clear 'not fitted' error -- + distinct from the degenerate-fit refusal below (issue #50).""" + model = AMICA(verbose=False) + assert model.is_fitted_ is False + with pytest.raises(ValueError, match="fitted"): + model.transform(np.zeros((NW, 16))) + with pytest.raises(ValueError, match="fitted"): + model.get_unmixing_matrix() + + +def test_degenerate_fit_refuses_output(real_data, tmp_path, caplog): + """A genuinely degenerate fit is marked unusable and every output method + refuses it rather than return NaN sources (issue #50). A single NaN injected + into the real EEG forces an actual ``nan_ll`` divergence in the backend -- + an error-path robustness test, not a parity/correctness claim, so it is not + a synthetic-data oracle -- exercising the real ``fit`` bookkeeping (not a + forced marker): is_fitted_/converged_ False, stop_reason_ named, and the + wrapper warning emitted.""" + bad = real_data[:, :4096].copy() + bad[0, 0] = np.nan # propagates to a nan_ll stop in the backend + model = AMICA(n_models=1, n_mix=3, device="cpu", verbose=False) + with caplog.at_level(logging.WARNING, logger="pyAMICA.amica"): + model.fit(bad, max_iter=3, block_size=1024, seed=0) + + assert model.stop_reason_ == "nan_ll" + assert model.converged_ is False + assert model.is_fitted_ is False + assert any("degenerate" in r.getMessage() for r in caplog.records) + + # transform/get_*/save refuse the degenerate model with a diagnosable error + # (names the stop_reason), not a misleading plain "not fitted". + for action in ( + lambda: model.transform(real_data[:, :512]), + lambda: model.get_mixing_matrix(), + lambda: model.get_unmixing_matrix(), + lambda: model.save(str(tmp_path / "degenerate.pt")), + ): + with pytest.raises(RuntimeError, match="degenerate.*nan_ll"): + action() + + # fit_transform routes through the guarded transform, so a degenerate refit + # cannot leak NaN sources either. + with pytest.raises(RuntimeError, match="degenerate"): + AMICA(n_models=1, n_mix=3, device="cpu", verbose=False).fit_transform( + bad, max_iter=3, block_size=1024, seed=0 + )