Skip to content

Phase 4: expose PMI/MIR through the MNE wrapper#190

Merged
neuromechanist merged 4 commits into
feature/issue-139-epic-mne-compatfrom
feature/issue-143-phase4-metrics
Jul 18, 2026
Merged

Phase 4: expose PMI/MIR through the MNE wrapper#190
neuromechanist merged 4 commits into
feature/issue-139-epic-mne-compatfrom
feature/issue-143-phase4-metrics

Conversation

@neuromechanist

Copy link
Copy Markdown
Member

Summary

Wires the #133 separation-quality metrics onto the MNE-facing wrapper: AMICAICA.mir(inst, model_idx=...) (Mutual Information Reduction, in nats) and AMICAICA.pmi(inst, model_idx=...) (pairwise mutual information between the fitted sources). Both extract the fitted channels from a Raw/Epochs and delegate to AMICA.mir/pmi (which already exist from #133), so MNE-side users get the same metrics as EEGLAB-side users.

Closes #143. Part of epic #139. This is the final phase.

What's new

  • AMICAICA.mir(inst, *, model_idx=0, nbins=None) and pmi(inst, *, model_idx=0, nbins=None). model_idx is keyword-only (consistent with the other consumer methods) and validated against the fitted model count; both guard unfitted/degenerate via _check_fitted. Epochs are concatenated along time.

Test plan

  • MNE wrapper (test-mne job, real EEG): mir/pmi reproduce AMICA.mir/pmi on the picked-channel array exactly, per model; pmi shape is (n_components, n_components); both work on Epochs; model_idx bounds and unfitted guards. Round-trip verified: wrapper MIR == array-API MIR (33.54 / 33.58 for the two models).
  • Local gates green: ruff, ty check ., mkdocs build --strict, typos, and the mne suite (44) on real sample EEG.

@neuromechanist

Copy link
Copy Markdown
Member Author

Review (2 Sonnet reviewers: code, tests)

Both confirmed the phase is thin, correct wiring; the code reviewer found nothing above its confidence bar (traced the full AMICAICA -> AMICA -> AMICATorchNG delegation chain, ran all 44 tests, verified the round trip). Test reviewer confirmed the per-model comparison genuinely discriminates (model 0 MIR 33.46 vs model 1 33.55). Actionable findings addressed:

  • pmi on Epochs was only shape-checked, not value-pinned, so a bug in _data_for's Epochs branch would slip through for pmi (caught for mir). Now value-pinned against AMICA.pmi.
  • Added an nbins passthrough test (matches AMICA with a non-default nbins, so a dropped forward is caught) and a degenerate-fit refusal test for mir/pmi (matching the convention used for the other consumer groups).
  • Made per-model discrimination explicit (assert not allclose(model0, model1)), self-documenting the model_idx-honored invariant.
  • Added mir's missing return annotation.

Local gates green: ruff, ty check ., mkdocs build --strict, typos, and the mne suite (46) on real sample EEG.

@neuromechanist
neuromechanist merged commit 8978514 into feature/issue-139-epic-mne-compat Jul 18, 2026
7 checks passed
@neuromechanist
neuromechanist deleted the feature/issue-143-phase4-metrics branch July 18, 2026 16:42
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