Skip to content

EEGLAB drop-in output parity (#92) - #100

Merged
neuromechanist merged 4 commits into
mainfrom
feature/issue-92-eeglab-dropin-output
Jul 11, 2026
Merged

EEGLAB drop-in output parity (#92)#100
neuromechanist merged 4 commits into
mainfrom
feature/issue-92-eeglab-dropin-output

Conversation

@neuromechanist

@neuromechanist neuromechanist commented Jul 11, 2026

Copy link
Copy Markdown
Member

Closes #92.

Summary

Makes pyAMICA a drop-in replacement for EEGLAB's AMICA: a PyTorch NG fit can now
be written as the exact loadmodout15 binary output directory, so it loads in
EEGLAB with the components in the same order and orientation, with no manual
re-sorting, sign-flipping, or reformatting.

Previously only the legacy NumPy backend could write this format; the primary
PyTorch backend could only torch.save a private dict.

What changed

  • Shared writer write_amicaout() in numpy_impl/load.py (co-located with its
    loadmodout reader counterpart): emits gm, W, S, mean, c, alpha, mu, sbeta, rho, comp_list, LL. numpy_impl/core.py:_write_results now delegates to it
    (byte-identical, no duplication).
  • AMICATorchNG.write_amica_output(outdir) and the AMICA.write_amica_output
    wrapper (with the _check_usable degenerate-fit guard, AMICA wrapper marks is_fitted_ even on a degenerate (nan_ll/singular_ll) fit #50).
  • variance_order() on backend + wrapper: EEGLAB back-projected-variance order
    (IC1 = highest), using the same W convention real MATLAB loadmodout15.m uses.
  • Docs: docs/guides/eeglab.md (EEGLAB interop + round-trip), wired into the nav.

Validation (real data + Fortran binary, no mocks)

  • NumPy-vs-Fortran parity tests still pass (writer refactor is byte-identical).
  • New round-trip tests (test_amica_ng_wrapper.py): byte-fidelity of every written
    file, loadmodout readability + shapes, variance ordering, and the requires-fit
    guard. Full non-slow suite green (166 passed).
  • MATLAB loadmodout15 round-trip (column-major, exactly as MATLAB reads): on
    a converged fit (LL -3.399), variance_order() equals the reader's origord,
    svar is descending, and per-component mixture proportions sum to 1.0 -- i.e. the
    written directory is read correctly by real EEGLAB with the components in the
    right order. Matched-config IC equivalence vs Fortran stays the ~0.997 from
    validate_implementations.py.
  • A format defect was caught and fixed here (see review-response comment): the
    mixture params were written C-order but EEGLAB/Fortran store column-major, so
    MATLAB would have read scrambled mixture proportions. Fixed writer + both numpy
    readers to Fortran order; single-model output is now byte-faithful to Fortran.

Notes

  • Single-model output is byte-identical to the Fortran reference. Multi-model output
    is self-consistent (round-trips through loadmodout15) but not byte-identical to a
    native multi-model AMICA run (issue Multi-model NG partition matching vs Fortran (>0.95 cross-corr) #27 scope).
  • The Python port loadmodout reads C-order where MATLAB reads column-major (a
    pre-existing latent transpose in the reader, out of this writer's scope); recorded
    in .context/scratch_history.md. The written bytes are correct for real EEGLAB.

Tested: yes (see Validation).

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.22034% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.75%. Comparing base (f97594b) to head (ba4b6ba).

Files with missing lines Patch % Lines
pyAMICA/numpy_impl/load.py 91.30% 1 Missing and 1 partial ⚠️
pyAMICA/torch_impl/core.py 92.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #100      +/-   ##
==========================================
+ Coverage   79.44%   79.75%   +0.30%     
==========================================
  Files          14       14              
  Lines        1975     2010      +35     
  Branches      337      339       +2     
==========================================
+ Hits         1569     1603      +34     
  Misses        301      301              
- Partials      105      106       +1     
Files with missing lines Coverage Δ
pyAMICA/amica.py 91.50% <100.00%> (+0.50%) ⬆️
pyAMICA/numpy_impl/core.py 82.67% <100.00%> (-0.08%) ⬇️
pyAMICA/numpy_impl/data.py 66.66% <100.00%> (ø)
pyAMICA/numpy_impl/load.py 72.48% <91.30%> (+2.31%) ⬆️
pyAMICA/torch_impl/core.py 93.42% <92.00%> (-0.06%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@neuromechanist

Copy link
Copy Markdown
Member Author

Review round 1 (Sonnet) + a MATLAB round-trip surfaced a real format bug

The /manuscript-grade "no shortcuts" standard paid off here: verifying against real MATLAB loadmodout15 exposed a genuine format defect that the Python-only tests missed.

Format bug (found + fixed)

pyAMICA wrote the non-square mixture params (alpha/mu/sbeta/rho, shape (num_mix, num_comps)) and c/comp_list in C-order, but Fortran/EEGLAB store them column-major. So real MATLAB loadmodout15.m (column-major reads) would have read scrambled mixture params, breaking the variance ordering and density model. Diagnostic: reading the committed genuine-Fortran amicaout/alpha column-major gives per-component mixture proportions summing to exactly 1.0; C-order gives garbage [0.49, 1.31].

Fixed the writer (write_amicaout) and both numpy readers (loadmodout, data.py:load_results) to use Fortran (order="F") layout for those arrays. W (square) stays C-order and is byte-identical to Fortran (the #24 transpose cancels against column-major storage); S is symmetric; mean/gm/LL are 1-D. This also fixes a latent bug where the numpy reader mis-read genuine Fortran mixture params.

Verification: re-ran the live MATLAB-faithful (column-major) round-trip on a converged fit (LL -3.399): variance_order() now equals the reader's origord, svar is descending, and mixture proportions sum to 1.0. Matched-config IC equivalence remains the ~0.997 from validate_implementations.py.

Review findings (all addressed)

  1. Important - LL/keep_best mismatch: when the Multi-model NG log-likelihood is ~0.02 lower and more variable than Fortran #51 safeguard restores an earlier iterate, the written LL file now ends at that kept iterate (LL[-1] == final_ll_), not at a later discarded overshoot. New deterministic test test_write_amica_output_ll_matches_kept_iterate (n_models=2 run that reliably triggers a restore).
  2. Important - guard-test gap: write_amica_output and variance_order added to test_degenerate_fit_refuses_output; new test_variance_order_requires_fit.
  3. Suggestion - tautological ordering assertion: replaced with a meaningful column-major check (mixture proportions per component sum to 1 after the loadmodout round-trip).

Regression safety

NumPy-vs-Fortran parity suite green (the reader change affects how genuine Fortran amicaout is read); full torch wrapper suite green (16 passed); byte-fidelity test updated to read the column-major arrays with order="F".

@neuromechanist
neuromechanist merged commit 5fe18e7 into main Jul 11, 2026
7 checks passed
@neuromechanist
neuromechanist deleted the feature/issue-92-eeglab-dropin-output branch July 11, 2026 07:45
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.

Drop-in EEGLAB AMICA replacement: equivalent output, no extra interpretation

1 participant