lina-usc / pylossless

🧠 EEG Processing pipeline that annotates continuous data
https://pylossless.readthedocs.io/en/latest/
MIT License
20 stars 8 forks source link

bug in `pipeline.flag_epoch_low_r` and `pipeline.flag_ch_low_r` #55

Closed scott-huberty closed 1 year ago

scott-huberty commented 1 year ago

Like the other pipeline methods that call marks_array2flags, as was the case for #45 and #54 I think the wrong dimension is being passed into flag_dim argument.

here:

https://github.com/lina-usc/pylossless/blob/4c3ae7108da82a1985f4548394d472dbbfb7db37/pylossless/pipeline.py#L922-L943

and here: https://github.com/lina-usc/pylossless/blob/4c3ae7108da82a1985f4548394d472dbbfb7db37/pylossless/pipeline.py#L992-L1011

but it would be nice to confirm this using our simulated raw file.

scott-huberty commented 1 year ago

Trying to write tests for these methods is brutal....

Let me explain why I think it is weird that I can't get flag_ch_low_r to flag any channels at all no matter what I try with my simulated file.....

So look at this montage. Chs 13 and 14 look like they should be "nearest neighbors" (i.e. ch 13 is one of the 3 closest channels to ch 14).. right?

Sensor_positions_(eeg)

Now I make channel 13 completely flat. So now the assumption is that channel 13 will have an incredibly low correlation with it's neighbors (i.e. channel 14 and also channel 12).. because no matter the voltage of ch 12 and 14, ch 13 will ALWAYS be 0.. right?

sim

We can confirm that with a simple correlation:

channel 13 has a tiny correlation with it's neighbor 14 (first cell), and with it's neighbor ch 12 (second cell).

channel 14 (just a regular channel) has a relatively larger correlation with its other neighbors (3rd cell).

channels 28 and 29 were forced to be the same and have a perfect correlation (last cell: just for demonstration).

Untitled

I would expect channel 13 to be flagged by flag_ch_low_r, wouldn't you??? yet it is not.

Untitled

I can't tell if this is a limitation of my simulation but I'm out of ideas.

scott-huberty commented 1 year ago

FYI - I looked at the MATLAB Lossless output for the FACE13 dataset (the dataset used for the lossless paper) - while sub-01 MATLAB-lossless flags 2 channesl as low_r, pylossless flags NONE

Then again - it looks like there is little (if any) agreement between MATLAB and python Lossless on the flagged channels.