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

add excluded channels back to data after preprocessing #66

Open
wants to merge 17 commits into
base: development
Choose a base branch
from

Conversation

dominikwelke
Copy link
Contributor

@dominikwelke dominikwelke commented Jul 7, 2022

closes #61

I mimicked the treatment of the EOG channels for EXCLUDED channels

Up until now, we were simply splitting the data and saving ECG, etc. temporarily on disc (or e.g. in EEG.ecg) before preprocessing in automagic.

where and how is this done?
I didnt find it in the code (only the code where EOG is seperated in systemDependentParse.m)

Additionally, one needs to be careful, when providing channel location file, because if the dimensions of data and channels do not agree, automagic won't load the channel information.

let's check this with the new PR i did not encounter problems so far..

@dominikwelke
Copy link
Contributor Author

dominikwelke commented Jul 7, 2022

as for the GUI implementation:
so far I set the parameter hardcoded in preprocess.p

% add settings parameter for readding excluded channels - 
% i am setting this manually for now, but should be taken from gui or defaults
addBackExcluded = true;
if ~isempty(params.ChannelReductionParams)
    params.ChannelReductionParams.readdExcludedChans = addBackExcluded;
    ChannelReductionParams.readdExcludedChans = addBackExcluded;
end

should be easy for you to retrieve it from defaults or the GUI :)

@dominikwelke
Copy link
Contributor Author

the code also handles Eye tracking data now (it can add the loaded channels and events to the output data, without taking it into account for the quality assessment etc).

maybe this feature isnt really ready to merge, as performETguidedICA.m isnt fully functional right now (e.g. you deactivated the saccade computation, and parameter are hardcoded in the file.. ). i'm making it work for myself in a seperate branch. if you are interested to include some of these fixes let me know :)

Copy link
Collaborator

@ksgfan ksgfan left a comment

Choose a reason for hiding this comment

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

where and how is this done?

In a separate script outside of Automagic : )

Regarding the first few commits: What if someone wants to definitely remove specific channels and reattach only a part of them? For example, high density EEG systems have channels located on a chin or neck. Some researchers exclude those channels already before preprocessing and do not reattach later.

@ksgfan
Copy link
Collaborator

ksgfan commented Jul 22, 2022

Further, if there are any ECG etc. channels, they should be removed before importing channel information in systemDependentParse, otherwise readlocs won't import the chanlocs - ECG don't have coordinates in .loc file. The mismatch in data/chanlocs dimension will result in an error.

@dominikwelke
Copy link
Contributor Author

thanks for your comments @ksgfan !

Regarding the first few commits: What if someone wants to definitely remove specific channels and reattach only a part of them? For example, high density EEG systems have channels located on a chin or neck. Some researchers exclude those channels already before preprocessing and do not reattach later.

partial reattachment would be straight forward to implement, just need an explicit field for indices in the gui / as input parameter.
I personally am not sure it's worth the effort.. either a user wants to keep some channels that are not to be preprocessed (because they are no EEG, in wrong locations, or whatever) - then automagic can reattach everything, and the user later decides what to do with those they dont need. or one only cares for the preprocessed EEG, then you do not reattach anything.

Further, if there are any ECG etc. channels, they should be removed before importing channel information in systemDependentParse, otherwise readlocs won't import the chanlocs - ECG don't have coordinates in .loc file. The mismatch in data/chanlocs dimension will result in an error.

actually, i cannot reproduce this behavior: working with BIDS data in BrainVision format, all channels I do not explicitly exclude via automagic are imported and part of the data/EEG structure. this includes ECG, EOG etc. the channel locations are set to NaN, but eeglab functions can work with this..
(this obviously means I have to explicitly exclude these channels via argument, as i cannot just leave them in. they would contribute to the quality ratings, channel interpolation etc. or more likely be marked as bad).

but maybe i misunderstand what you mean?

systemDependentParse seemed to me the first step where you actually access and split up the data, in the end this is also where you seperate your EOG from the EEG, and these dont have channel locations either, am i wrong?

most of the time it shouldnt be a problem, anyway, as the EXCLUDEDchannels are not part of the main EEG structure during the entire preprocessing.
excluded channels are only reattached after the whole preprocessing, and I took care of them not being considered in the quality assessment and later in the channel interpolation.

the problem with eeglab complaining about dimension mismatch appeared to me only with the ET channels during channel interpolation (as here the chanlocs are actually empty []) - I tackled this with one of the commits. this should also cover excluded channels with empty location info..

if there are other crucial positions where channel locations are needed, please let me know!

In a separate script outside of Automagic : )

jup, that's what this pr is about :)

@dominikwelke
Copy link
Contributor Author

dominikwelke commented Jul 22, 2022

The other PR will take me more time, I have to discuss some of the changes (e.g. ET data) with our team.

yes, please dont merge yet! i strongly advice for testing first ;)
the ET related commits could be largely seperated from those on EXCLUDED channels.

@dominikwelke
Copy link
Contributor Author

..talking about channel interpolation:
so far you skip interpolation only if all channels are marked bad.
isn't this a bit too liberal? shouldnt you also stop if there is, like, only 1 channel left to interpolate the rest from? or complain if there is only ~10% good channels?

maybe a question for another issue, though :)

@ksgfan
Copy link
Collaborator

ksgfan commented Jul 25, 2022

If the number of rows in EEG.data don't match the number of rows in e.g. chanlocs.loc file, the .loc file will not be imported. The mismatch might occur, because e.g. Biosemi64 has 8 Ext channels (72 channels in EEG.data in total), but only 64 channels in .loc file. Similar, if some EEG systems have VEOG or HEOG channels in EEG.data, but no coordinates in the .loc file, the loc file won't be imported. On the other hand, EEG system such as EGI has coordinates for the EOG channels, so there is no problem with the .loc (or in this case .sfp) import.
That is way in the past I was removing the Ext channels outside of Automagic and reattaching them after the preprocessing was fully done.

so far you skip interpolation only if all channels are marked bad.
isn't this a bit too liberal? shouldnt you also stop if there is, like, only 1 channel left to interpolate the rest from? or complain if there is only ~10% good channels?

The number of required channels for meaningful interpolation may vary based on EEG.system (number of electrodes) and position of removed channels. I think it should be the users responsibility to decide, whether the interpolation results make sense. Btw, if the number of removed channels exceeds a given threshold, the file will be marked as 'bad' and should be excluded from analysis.

@dominikwelke
Copy link
Contributor Author

If the number of rows in EEG.data don't match the number of rows in e.g. chanlocs.loc file, the .loc file will not be imported. The mismatch might occur, because e.g. Biosemi64 has 8 Ext channels (72 channels in EEG.data in total), but only 64 channels in .loc file. Similar, if some EEG systems have VEOG or HEOG channels in EEG.data, but no coordinates in the .loc file, the loc file won't be imported. On the other hand, EEG system such as EGI has coordinates for the EOG channels, so there is no problem with the .loc (or in this case .sfp) import.
That is way in the past I was removing the Ext channels outside of Automagic and reattaching them after the preprocessing was fully done.

ok, kind of makes sense..
do you have a small example dataset (with misc/ext channels) that i could use to troubleshoot? i'm sure its possible to catch these types of errors - from a user perspective it would be much desireable to just plug in your data as raw as possible without any pre-pre-processing and post-pre-processing :)

in the case of my BIDS data it wasnt a problem, as i digitized individual channel locations and these are loaded without problems.

what i still dont understand from your explanation is, how the code can work with EOG channels then? shouldnt this cause the same errors you expect?

@ksgfan
Copy link
Collaborator

ksgfan commented Jul 26, 2022

do you have a small example dataset (with misc/ext channels) that i could use to troubleshoot?

Sure: https://www.dropbox.com/sh/osbe0qm07a2eb8k/AADDVpDWzHthQoLM70lU6ijca?dl=0

EOG works in some EEG systems (e.g. EGI), because they have coordinates for EOG electrodes. For other systems it is necessary to add coordinates of the EOG electrodes to the .loc file manually.

In my opinion your solution adds to much complexity to the code. It would be much simpler, if the misc channels would be separated and stored in e.g. EEG.misc just after loading the data, and reattached by pressing another button after all preprocessing, interpolation and quality assessment is done. There would be no problems with channel indices, interpolation, quality assessment (or multiple quality assessment). All we need would be a new button to reattach selected channels.

@dominikwelke
Copy link
Contributor Author

Sure: (...)

thanks!

In my opinion your solution adds to much complexity to the code. It would be much simpler, if the misc channels would be separated and stored in e.g. EEG.misc just after loading the data ...

i think you're right.
honestly, i didnt know know enough about the code sequence when i started working on this (talking about complexity), so i just mimincked the treatment of EOG. didnt expect this only to work in special cases :)

i'll change the code and get back to you soon!

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.

2 participants