methlabUZH / automagic

Automagic
GNU General Public License v3.0
94 stars 32 forks source link

ASR results not returned? #45

Closed ramBrain closed 3 years ago

ramBrain commented 3 years ago

Dear developers,

As stated in the wiki, the use of ASR and Window Criterion will return a modified version of the data right after the "Bad channel detection" section, whilst this is not the case if only channel rejection is performed. However, if I choose to only perform ASR (where by default time segment rejection is not selected) without Window Criterion, the cleaned version is not returned and the following preprocessing steps are performed on the original raw data.

Basically, the output of ASR will be ignored if ASR without signal rejection is performed.

I found that the possible bug is in \preprocessing\performCleanrawdata.m.

At line 81, the cleaned EEG is produced. However this will be assigned to EEG_out only if there is a time segment rejection, as tested at line 98:

% Remove the same time-windows from the EOG channels
if(isfield(EEGCleaned.etc, 'clean_sample_mask'))
  EEG_out = EEGCleaned;

         if(isfield(EEGCleaned.etc, 'clean_channel_mask'))
            removedMask = newMask;
            newToRemove = toRemove;
         end
[...]
end

I tried a possible fix, which replaces that test with:

% If ASR or Window criterion is performed, replace EEG_out with EEGCleaned

if ~isequal(params.BurstCriterion,'off') || isfield(EEGCleaned.etc, 'clean_sample_mask')
    EEG_out = EEGCleaned;

    % If portion of data have been rejected, reject from EOG too

    if isfield(EEGCleaned.etc, 'clean_sample_mask')

        % Remove the same time-windows from the EOG channels
        if(isfield(EEGCleaned.etc, 'clean_channel_mask'))
            removedMask = newMask;
            newToRemove = toRemove;
        end
[...]
    end
end

It seems to correctly work, but I would be happy to receive your feedback.

Please let me know if that was not an actual bug and I am missing something.

Thank you.

Ramtin

ksgfan commented 3 years ago

Hi Ramtin

it is a bug, nice catch! I also tested your version on our data and it works fine. I am going to look at the other issues and improve the IClabel over the course of this week.

Thanks for contributing to automagic! Best, Dawid

ramBrain commented 3 years ago

Hi Dawid,

That is great, thank you for considering my feedback!

FYI, this and the PREP fix are included in my pull request, together with the rough ICLabel fix.

B. Ramtin

ramBrain commented 3 years ago

Hi again,

I just noticed an inaccuracy in my fix. Specifically, at line 100 and below, it should be:

if ~isequal(params.BurstCriterion,'off') || isfield(EEGCleaned.etc, 'clean_sample_mask') 

EEG_out = EEGCleaned;

if(isfield(EEGCleaned.etc, 'clean_channel_mask'))
   removedMask = newMask;                
   newToRemove = toRemove;
end

% If portion of data have been rejected, reject from EOG too

if isfield(EEGCleaned.etc, 'clean_sample_mask')                    

% Remove the same time-windows from the EOG channels           

removed = EEGCleaned.etc.clean_sample_mask;
firsts = find(diff(removed) == -1) + 1;
seconds = find(diff(removed) == 1);

[...]

end
end

i.e., the removedMask and newToRemove variables should already be updated right after testing whether either ASR or Windows Criterion was applied, otherwise if channels are removed with clean_rawdata there will be a double attempt to remove the bad channels, which would produce a wrong output.

Sorry about that, but I only tested PREP before, which is why I did not spot this right away.

Thanks, Ramtin

ksgfan commented 3 years ago

Hi Ramtin

thanks for the fix!

best, Dawid