Repository navigation
share_comps compares and merges the wrong vectors (all backends) #334
Copy link
Copy link
Closed
Labels
bugSomething isn't workingSomething isn't working
Description
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 columnspinv(sphere) @ A[:, ci], but componentiof modelhis rowiof 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 mapget_sensor_mixing_matrix(h)[:, i]has median 0.27 / 0.39 per model. Atcomp_thresh=0.95the 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
cjintocities 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-weighteddAk/zetaaverage then averages the wrong things. Seeded from a merged state through Fortran'sload_comp_list, the shipped update diverges from the reference immediately (A0.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.95ends 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.99nothing merged on the sample EEG, so default settings on that data were unaffected; other data can differ.
Fix (epic #324 Phase 8)
- Store
Awith components as rows (shape(n_comps, n),comp_listindexes rows); every per-model block keeps today'sn x nmatrix, so every configuration without sharing stays byte-identical. Metric onpinv(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 mergedload_comp_liststate. - Persistence: convert unmerged multi-model saves losslessly (format bump); refuse merged ones with a message to refit. The EEGLAB
Aexport becomes the reference layout forn_models > 1. - Changelog and a note to refit fits made with
share_comps=Truewhere merges fired.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working
Activity
Describe component-row sharing in the notes
Phase 8: store components as rows and fix sharing (#342)
Fix the EEGLAB guide's scalp-map example and multi-model note
Correct the differences guide's Newton row and backend table
Bring the context plan and progress summary up to date
Say which default fits #335 and #334 reach, in the changelog
Phase 16: audit the documentation against the epic (#353)
neuromechanist commentedon Sep 24, 2026
@sunyuhongwr The component-sharing fix is on
devnow (epic #324, merged in #364), and it affects the 5-modelshare_compsworkflow 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 withuv pip install "pamica[mne] @ git+https://github.com/sccn/pAMICA@dev".What changed in sharing:
share_compsnow compares and ties each component's mixing vector, asidentify_shared_compsdoes in the reference. Before, it compared stored columns of the mixing block, which are not components.Default fits differ from 0.3.3 on every backend. A number of changes bring each iteration closer to
amica15.f90:doscaling;lratedefault 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.