Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ICA #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix ICA #36

wants to merge 1 commit into from

Conversation

wmvanvliet
Copy link
Contributor

@wmvanvliet wmvanvliet commented Apr 27, 2018

ICA ignored the EEG channels, which means EOG artifacts where not cleaned from the EEG sensors, which means autoreject started rejecting a lot of data based on blinks in the EEG.

This should fix it. I'm re-running our conpy pipeline now (takes a while...) to see if we get decent results.

@larsoner
Copy link
Member

According to the comments we still don't remove EOG-related artifacts. Do you need to change later steps to add this?

@@ -150,6 +150,7 @@ def run_epochs(subject_id, tsss=False):
print(' Getting rejection thresholds')
reject = get_rejection_threshold(epochs.copy().crop(None, reject_tmax),
random_state=random_state)
del reject['eog'] # Don't reject epochs based on EOG
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably still should reject based on EOG because we don't want to use trials with blinks. At least this was what we thought before.

@larsoner
Copy link
Member

Actually 06-make_epochs.py already has ica.find_bads_eog, so I guess there is nothing else to do there.

Can you try leaving the EOG-based rejection in and see if this results in a reasonable number of trials? Or are too many rejected still?

@wmvanvliet
Copy link
Contributor Author

Let's see...

@@ -166,6 +166,6 @@ def run_epochs(subject_id, tsss=False):

# Here we use fewer N_JOBS to prevent potential memory problems
parallel, run_func, _ = parallel_func(run_epochs, n_jobs=max(N_JOBS // 4, 1))
parallel(run_func(subject_id) for subject_id in range(1, 20))
parallel(run_func(subject_id) for subject_id in range(6, 20))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@wmvanvliet
Copy link
Contributor Author

wmvanvliet commented Apr 27, 2018

Leaving the EOG rejection limits in gives me the following rejection rates:

subject #rejected/#total (rejection rate)
Subject sub002: 175/887 (19.7 %)
Subject sub003: 183/882 (20.7 %)
Subject sub004: 404/889 (45.4 %)
Subject sub006: 178/883 (20.2 %)
Subject sub007: 011/885 (01.2 %)
Subject sub008: 309/882 (35.0 %)
Subject sub009: 622/889 (70.0 %)
Subject sub010: 426/885 (48.1 %)
Subject sub011: 384/884 (43.4 %)
Subject sub012: 351/888 (39.5 %)
Subject sub013: 392/883 (44.4 %)
Subject sub014: 241/887 (27.2 %)
Subject sub015: 177/883 (20.0 %)
Subject sub017: 642/883 (72.7 %)
Subject sub018: 005/882 (00.6 %)
Subject sub019: 234/885 (26.4 %)

@larsoner
Copy link
Member

There is some reasonable scientific basis for rejecting trials where blinks occurred in such a visual paradigm. Even if the artifacts are rejected, the visual input is not the same. I am no visual expert, though, but this did play into our EOG-related decision IIRC

@larsoner
Copy link
Member

IIRC most subjects don't have a usable 3-layer BEM so EEG is not used for LCMV/dSPM. So this probably won't make a huge difference for what we show in the paper, except maybe the sensor plots.

@jasmainak
Copy link
Member

jasmainak commented Apr 27, 2018 via email

@wmvanvliet
Copy link
Contributor Author

Bwah. With the ICA properly cleaning EEG and autoreject being performed on MEG+EEG only, the rejection rates are as follows:

subject #rejected/#total (rejection rate)
Subject sub002: 069/887 (07.8 %)
Subject sub003: 171/882 (19.4 %)
Subject sub004: 354/889 (39.8 %)
Subject sub006: 178/883 (20.2 %)
Subject sub007: 011/885 (01.2 %)
Subject sub008: 175/882 (19.8 %)
Subject sub009: 576/889 (64.8 %)
Subject sub010: 306/885 (34.6 %)
Subject sub011: 337/884 (38.1 %)
Subject sub012: 232/888 (26.1 %)
Subject sub013: 308/883 (34.9 %)
Subject sub014: 241/887 (27.2 %)
Subject sub015: 001/883 (00.1 %)
Subject sub017: 574/883 (65.0 %)
Subject sub018: 008/882 (00.9 %)
Subject sub019: 234/885 (26.4 %)

That's still a lot of dropped epochs. And worse yet: the grand average STC makes no sense at all. Guess it's back to the drawing board for me on this.

@jasmainak
Copy link
Member

jasmainak commented Apr 30, 2018 via email

@wmvanvliet
Copy link
Contributor Author

I've played around a bit with the data. It seems the mags and EEG are just that noisy. In our Frontiers submission we've ended up setting fixed, lenient, rejection thresholds in order to retain enough data for connectivity analysis.

@agramfort
Copy link
Member

agramfort commented Jun 20, 2018 via email

@wmvanvliet
Copy link
Contributor Author

sorry for the confusion. We do apply ICA. We replaced autoreject with a fixed threshold.

@agramfort
Copy link
Member

agramfort commented Jun 21, 2018 via email

@wmvanvliet
Copy link
Contributor Author

Yes. I thought this was because ICA was not cleaning the EEG data (which it wasn't), but fixing the ICA did not resolve the issue.

@jasmainak
Copy link
Member

Just to be clear, this was autoreject (global), right? I think the issue might be that it does not include data augmentation, but I'll have to try it

@larsoner
Copy link
Member

Yes. I thought this was because ICA was not cleaning the EEG data (which it wasn't), but fixing the ICA

So there is something wrong with what ICA does currently that we could fix? If so, what is it / how did you fix it?

@wmvanvliet wmvanvliet force-pushed the fix_ica branch 2 times, most recently from 68845eb to f64adaf Compare June 21, 2018 17:01
@wmvanvliet
Copy link
Contributor Author

The ICA is currently not fitted to the EEG channels. This PR fixes this.

@agramfort
Copy link
Member

I am afraid this will change a bit the results of our paper. I don't have time to rerun all to check if it's the case... :(

@larsoner
Copy link
Member

Assuming MEG and EEG are fitted separately (?) then it should only affect the EEG sensor space estimates, as we could not do source imaging with EEG. But indeed someone will need to re-run from this step forward and upload the result to check if we want to incorporate.

The review is almost done (?) so if we do change it, we should make it clear in the README that a particular commit was used for the paper figures.

@agramfort
Copy link
Member

agramfort commented Jun 22, 2018 via email

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.

None yet

4 participants