[ENH, MRG] Add Automatic Muscle Artifact ICA Component Detection #10534
Conversation
|
Looks pretty good to me! Great that someone has already published on it and it wasn't too hard to implement. https://output.circle-artifacts.com/output/job/a105e6c5-2ecf-41c2-b56f-a20761376a3d/artifacts/0/dev/auto_examples/preprocessing/muscle_ica.html?highlight=bad%20muscle |
| assert pos.shape[0] == components.shape[0] # pos for each component | ||
| dists = np.linalg.norm(pos, axis=1) | ||
| dists /= dists.max() | ||
| focus_dists = np.dot(dists, components_norm) |
There was a problem hiding this comment.
if you have multiple channel types in inst what do you expect this code to do?
There was a problem hiding this comment.
Should be fine, it's by component normalized so it's not in channel units
There was a problem hiding this comment.
agreed but the notion of distance across channel types is weird no?
There was a problem hiding this comment.
It's just distance of the focus for the topomap, whether it's MEG or EEG if the red and blue blobs are far from the vertex that's a high score, it doesn't matter what channel type they are
|
Okay @agramfort, I checked on EEGBCI and just going through the code, I was accidentally using the intercepts instead of the slopes and I failed to normalize the smoothness properly which is why it wasn't working. Now it works awesome, feel free to remove the eegbci part if you want to reduce computation @agramfort but those work great as well. I tried on subject 3 and got rather odd ICA components but I don't think that was really an issue with the algorithm so I didn't include it and stopped there. |
| assert pos.shape[0] == components.shape[0] # pos for each component | ||
| dists = np.linalg.norm(pos, axis=1) | ||
| dists /= dists.max() | ||
| focus_dists = np.dot(dists, components_norm) |
There was a problem hiding this comment.
agreed but the notion of distance across channel types is weird no?
For the focus distance it doesn't matter but good point for the smoothness, it should be by channel type |
|
For now I would say that if it’s supported for EEG only it’s good enough. We can iterate later
|
|
Ok sounds good, it's complicated because you'd have to append distance matrices and pad them with |
|
There was just a small docstring error so everything should pass now. I left in the EEGBCI as well as the MNE sample data in the example. The execution time is less than 20 seconds so I think it would be nice to leave everything in because it shows the variability in muscle components based on setup; if the subject is in a relaxed state (probably with a comfortable chair and a task that isn't too demanding/motoric) there might only be one small eigenvalue component but if they have to use more head and neck muscles, half of the components might be from all around the head. I think this example as is covers that diversity really nicely but if you prefer @agramfort or others, I don't feel that strongly and I'm happy to take them out. EDIT: The two additional EEGBCI calls add about 2 seconds of computation time each so doesn't seem so bad but again not minimal necessary. |
saved version with smoothness from paper, implementing KL divergence save kl version working version [skip ci] revert versionadded must be in notes two more small typos add test for scores empty push, Qt segfault fixed several errors, checked with eegbci add ref in tutorial, fix tests fix docstring alex review
|
Thanks for the review, I incorporated your suggestions :) |
|
can you share the new rendered doc when you have it? thx |
|
Did someone change the artifact uploading recently? I'm not able to access the circle artifact. |
* upstream/main: Correct documented default number of CV splits [skip azp][skip actions] (mne-tools#10548) MRG: Add error checking to prevent the creation of montage with invalid [x, y, z] (mne-tools#10547)
No, and it seems to work on other PRs like: Assuming you didn't enable CircleCI on your fork or anything like that, it's hopefully just a temporary glitch. I did a merge with |
|
Just a small comment from me (I did not look at the code really), but @agramfort here are the renderings https://output.circle-artifacts.com/output/job/e9368859-4bf6-4352-8929-6f1cb3a9d16b/artifacts/0/dev/generated/mne.preprocessing.ICA.html#mne.preprocessing.ICA.find_bads_muscle |
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
|
Thanks @larsoner, the CIs are running now so maybe it's just a glitch |
|
Hey it worked again, maybe just a temporary glitch https://output.circle-artifacts.com/output/job/80147f55-85cf-4314-a0fd-91749909b67c/artifacts/0/dev/auto_examples/preprocessing/muscle_ica.html |
There was a problem hiding this comment.
thx @alexrockhill
I let you merge after fixing conflict with latest.inc and provided CIs are green.
🙏 🙏 🙏
* upstream/main: MAINT: Extra test for coreg (mne-tools#10549) BUG: Fix annot meas_date / crop (mne-tools#10491) Update latest.inc Use liblinear solver instead of lbgfs in all decodung examples [skip azp][skip actions] (mne-tools#10552) STY: Hotfix for HTML [ci skip] Allow making inverse solutions from fixed-orientation discrete forward models, enabling multi-dipole modeling (mne-tools#10464)
|
Thanks @alexrockhill ! |
…10520 * upstream/main: MAINT: Respect activation (mne-tools#10546) [ENH, MRG] Add Automatic Muscle Artifact ICA Component Detection (mne-tools#10534)
…10533 * upstream/main: MAINT: Respect activation (mne-tools#10546) [ENH, MRG] Add Automatic Muscle Artifact ICA Component Detection (mne-tools#10534)
Fixes #10514. The topomaps are still magnetometers and you can't toggle but I'll write a followup PR for that.
Will link to the example that shows how it works when Circle builds.