Skip to content

share_comps compares and merges the wrong vectors (all backends) #334

Description

@neuromechanist

Found during epic #324 (component-orientation investigation). Affects n_models >= 2 with share_comps=True on all three array backends, whenever a merge fires. Same root cause as the doscaling orientation issue (stored columns are not components).

Symptom

The share metric compares the wrong vectors, and the merge ties the wrong parameters.

  • Metric: identify_shared_components / AMICATorchNG._identify_shared_comps / the MLX port compare de-sphered stored columns pinv(sphere) @ A[:, ci], but component i of model h is row i of the stored block (issue Verify NG init-basin vs Newton bit-parity via Fortran load_* init-matching #24 convention). On a real 2-model fit (sample EEG, 150 iterations) the |cosine| between a stored column and the true scalp map get_sensor_mixing_matrix(h)[:, i] has median 0.27 / 0.39 per model. At comp_thresh=0.95 the shipped scan merged a pair whose true maps have |cos| 0.34, while the real best matches (|cos| 0.95-0.96) scored 0.15-0.45 and never merged; with 5 models at 0.9, 30 merges reached |cos| 0.001. A planted exact duplicate component scores 0.74 and is not merged at 0.99.
  • Merge: folding stored column cj into ci ties one sphered channel's loadings across the two models' components (a row of each true mixing matrix), not the two components' mixing vectors, and the gm-weighted dAk/zeta average then averages the wrong things. Seeded from a merged state through Fortran's load_comp_list, the shipped update diverges from the reference immediately (A 0.19-0.20, LL 6.4e-4 after 3 iterations); a row-layout prototype matches it to the no-merge noise floor. Forcing the shipped fold on a planted identical pair changes the LL by -0.186 (the reference fold: exactly 0).
  • End to end (2 models, 300 iterations, share off LL -3.3439): shipped comp_thresh=0.95 ends at -3.4141 with Newton fallbacks; the prototype ends at -3.3416 with 3 merges of true pairs (|cos| 0.96-0.97).
  • At the default comp_thresh=0.99 nothing merged on the sample EEG, so default settings on that data were unaffected; other data can differ.

Fix (epic #324 Phase 8)

  • Store A with components as rows (shape (n_comps, n), comp_list indexes rows); every per-model block keeps today's n x n matrix, so every configuration without sharing stays byte-identical. Metric on pinv(sphere) @ A.T; merge ties rows; update and accessors follow. All three backends.
  • Tests that pin semantics, not just cross-backend agreement: grouped sources have identical component maps; a planted duplicate is merged and LL/transform/maps are unchanged; the metric vector equals get_sensor_mixing_matrix(h)[:, i]; a gated native oracle from a merged load_comp_list state.
  • Persistence: convert unmerged multi-model saves losslessly (format bump); refuse merged ones with a message to refit. The EEGLAB A export becomes the reference layout for n_models > 1.
  • Changelog and a note to refit fits made with share_comps=True where merges fired.

Activity

  1. neuromechanist commented on Sep 24, 2026

    @neuromechanist
    MemberAuthor

    @sunyuhongwr The component-sharing fix is on dev now (epic #324, merged in #364), and it affects the 5-model share_comps workflow you validated on the Elekta data. It is not in a PyPI release yet; the development version installs from source, as the README describes, or with uv pip install "pamica[mne] @ git+https://github.com/sccn/pAMICA@dev".

    What changed in sharing: share_comps now compares and ties each component's mixing vector, as identify_shared_comps does in the reference. Before, it compared stored columns of the mixing block, which are not components.

    • Fits where a merge fired: these used different components, so they are worth refitting.
    • Loading older models: a saved model in which components had merged is refused on load, with a message to refit.
    • Fits with sharing on but no merge: unchanged, apart from float round-off in the gradient norm.

    Default fits differ from 0.3.3 on every backend. A number of changes bring each iteration closer to amica15.f90:

    • component-row doscaling;
    • the normalized initial mixing matrix;
    • the reference's iteration order and A-freeze windows;
    • the single-precision density constants;
    • a shared lrate default of 0.1 across entry points.

    The changelog lists them. Against the reference binary, the single-model log-likelihood on a 70-channel EEG recording now agrees to 6e-6 (5 seeds, 2000 iterations), with mean component correlation 0.9996.

    If you have a chance to rerun the sharing workflow on your MEG data, that would be the most useful check we could get. On the bundled 32-channel EEG sample, the documented sharing example now merges one pair where it merged three, so how many merges fire on your data, and where, would tell us a lot. A fresh issue with what you see works best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions