Skip to content

Phase 1: core MNE wrapper (single-model)#187

Merged
neuromechanist merged 7 commits into
feature/issue-139-epic-mne-compatfrom
feature/issue-140-phase1-core-wrapper
Jul 18, 2026
Merged

Phase 1: core MNE wrapper (single-model)#187
neuromechanist merged 7 commits into
feature/issue-139-epic-mne-compatfrom
feature/issue-140-phase1-core-wrapper

Conversation

@neuromechanist

Copy link
Copy Markdown
Member

Summary

Adds pamica.mne_compat.AMICAICA, an MNE-Python-facing wrapper that fits AMICA directly from an mne.io.Raw/Epochs and interoperates with the standard MNE ICA consumer surface (get_sources, apply, get_components, plot_components). Its core value-add is to_mne_ica(), which maps pamica's fitted mean/sphere/unmixing into a fully-populated mne.preprocessing.ICA; the other methods delegate to that object so MNE performs the export/plot/back-projection machinery unchanged.

This is purely additive (issue #139 scope): the scikit-learn-style AMICA API and the byte-identical EEGLAB I/O are untouched. MNE is an optional extra (pip install pamica[mne]), imported lazily exactly like the mlx extra, so import pamica never requires it.

Closes #140. Part of epic #139.

The mapping (single-model)

pamica computes S = W_fort @ sphere @ (X - mean) (the per-model center c is exactly zero for one model). MNE computes S = unmixing_matrix_ @ pca_components_ @ (X - pca_mean_) with a unit pre-whitener. Writing the symmetric-ZCA sphere as V diag(1/sqrt(e)) V^T with V orthonormal, the export sets pca_components_ = V^T, unmixing_matrix_ = W_fort @ sphere @ V, pca_mean_ = mean. Keeping pca_components_ orthonormal is what makes MNE's get_components() (channel-space maps inv(sphere) @ inv(W_fort)) come out right, since MNE assumes orthonormal PCA rows.

Measured on real sample EEG: get_sources vs AMICA.transform agree to 4e-14, apply() with no exclusions reconstructs to 1e-18, and get_components() matches inv(sphere) @ inv(W_fort) to 3e-20.

Test plan

  • New pamica/tests/mne_tests/ on the real bundled eeglab_data.set (no mocks), 11 tests: the get_sources == transform round trip (single-model crux), channel-space scalp maps, apply reconstruct/exclude, epochs and channel-subset fits, plotting, and unfitted/bad-input guards.
  • The module self-skips without the mne extra (importorskip), so the base test job skips it; a dedicated test-mne CI job installs pamica[mne] and runs it for real on Linux (mne is pip-installable there, unlike the Apple-only mlx extra).
  • pamica/mne_compat is omitted from base coverage (base env has no mne), same treatment as mlx_impl.
  • Local gates green: ruff check/format, ty check ., the 11 mne tests, and mkdocs build --strict.

Docs

New docs/api/mne-compat.md (usage + to_mne_ica interop + mkdocstrings), nav + API-index entries, and a ## 0.3.0 changelog section.

@neuromechanist

Copy link
Copy Markdown
Member Author

Review (5 Sonnet reviewers: code, tests, comments, silent-failure, type-design)

All findings were real (no false positives); every one is addressed in commits 77bb814 / 9d8fff3 / 8365412. Reviewers confirmed the core mapping math is correct and independently re-derived it.

Correctness / robustness (core.py)

  • CRITICAL, non-atomic fit() (type-design + silent-failure, reproduced): fit() wrote info_/ch_names_/n_components_/_fit_kind to self before the fallible amica.fit(), so a failing refit left the old model paired with the new attempt's metadata, and to_mne_ica() could then build a shape-mismatched (or silently mislabeled) ICA. Fixed by computing everything in locals and publishing to self only after all fallible steps succeed, mirroring AMICA.fit. Pinned by test_failed_refit_leaves_prior_state_intact.
  • CRITICAL, PCA-reduced fits silently produced garbage (comments + code, reproduced 1e26 values): pcakeep/pcadb forwarded through **fit_kwargs leave the sphere rank-deficient, and 1/sphere_evals**2 then blows up with no error. fit() now rejects PCA reduction with a clear ValueError (same posture as mir()/identify_shared_comps). Pinned by test_fit_rejects_pca_reduction.
  • CRITICAL, to_mne_ica() omitted reject_/n_samples_ (code, reproduced): ICA.save() and plot_properties() crashed with AttributeError. Now set (reject_=None, matching read_ica_eeglab; n_samples_ = samples fit). Pinned by test_exported_ica_saves_and_reloads.
  • HIGH, non-finite input → opaque LAPACK error: fit() now validates np.isfinite at the boundary and names the offending channels. Pinned by test_fit_rejects_non_finite_data.
  • HIGH, start/stop silently ignored for Epochs; stop unbounded for Raw: now raise ValueError. Pinned by test_fit_rejects_start_stop_on_epochs, test_fit_rejects_out_of_range_stop, test_fit_with_start_stop_subranges_raw.
  • MEDIUM, bare asserts as guards: replaced with real RuntimeErrors carrying context.
  • MEDIUM, __repr__ / _check_fitted degenerate handling: the wrapper now surfaces converged_/stop_reason_, _check_fitted raises ValueError when unfitted (matching AMICA) and RuntimeError when degenerate (explicit, not incidental), and __repr__ labels a degenerate fit. Pinned by test_degenerate_fit_is_refused_and_labeled and test_unfitted_calls_raise.

Tests

  • Eigenvector ordering was untested (tests, reproduced): because V @ Vᵀ = I cancels V, get_sources/get_components are invariant to the eigenbasis order, so a mis-sort would have passed. Added test_pca_components_orthonormal_and_variance_ordered: orthonormality, descending pca_explained_variance_, and an independent oracle (per-row variance of the projected data equals the explained variance in order).
  • Added coverage for apply on Epochs, plot_sources, and seed determinism.

Docs / comments

  • Added plot_sources to the surface enumerations, documented the PCA-reduction limitation and the rank-deficient-data (e.g. average-reference) caveat, and noted method="infomax" is inert (we never call ICA.fit).
  • Removed the dead single-model if np.any(c) branch (c is provably zero for one model); the multi-model center handling belongs to Phase 2: multi-model exposure for MNE users #141, where it will land with a non-zero-c test.

Local gates green after fixes: ruff, ty check ., mkdocs build --strict, and 23 mne tests (real sample EEG).

@neuromechanist
neuromechanist merged commit 9a5933e into feature/issue-139-epic-mne-compat Jul 18, 2026
7 checks passed
@neuromechanist
neuromechanist deleted the feature/issue-140-phase1-core-wrapper branch July 18, 2026 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant