-
Notifications
You must be signed in to change notification settings - Fork 8
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
Re-referencing concerns #134
Comments
Poking through a bit more of the code it seems like it's done via the |
I accidentally deleted my response. I'll post it again: robust average pylossless/pylossless/pipeline.py Lines 520 to 521 in b507983
Here:
MOST pipeline steps will call The exceptions are pylossless/pylossless/pipeline.py Lines 1070 to 1083 in b507983
above,
The next time robust average referencing occurs is @Andesha @jadesjardins if this deviates from the MATLAB code, let us know. |
The pipeline was designed to re-reference after every identification of bad channels. flag_chan_bridge is a detection of bad channels so should be followed by re-referencing (excluding the marked channels). The rank channel is not an artifact detection, and the channel flagged by flag_ch_rank should not be removed from the scalp channels. The rank channel remains in the scalp channel data, but this channel is not included in the ICA decomposition. Re-referencing after identifying the rank channel should not do anything because this channel is not marked for rejection (the average reference would be identical to before marking the rank channel). |
Thanks @jadesjardins ! From what I can tell, MATLAB lossless didn't re-reference directly after Looking at the MATLAB code and my description of pylossless above, it seems like we directly translated the MATLAB code. Maybe this was a bug in MAT Lossless that we ported over to Python. But when I think about it, we exclude channels from the average so that noisy channels don't skew the ref. Unless you feel strongly that skipping the re-ref was a bug that pyLossless should correct (or unless I'm reading the matlab code incorrectly) I'm tempted to consider this a feature request and not a bug fix. In that case I'm definitely +1 to add another re-reference if others think it's important, but personally I won't have time to do it I don't think. but if anyone is really pressing for this step and implements it in their local fork, a PR is welcome |
We are excluding |
Totally, but this is done after finishing flagging low_r, bridged, and rank channels in one go. This is the same thing we do in pylossless. |
I have not looked at the Matlab lossless scripts in a long time... and I could definitely picture that the re-referencing would only happen at the end of a sequence of channel flagging procedures in a row. That being said, I would recommend implementing the feature of re-referencing after every channel rejection flagging procedure individually. I agree that it is likely to have little impact (so can be low priority), but it would be the correct thing to do. |
This is just a concern, but I've been working a lot more handson with data coming from the pipeline and am not sure that after we are rejecting a source (channels) that we are rereferencing the data.
I see that there's a function for it for channels here but it does not appear to be used after things like the rank channel are thrown out...
The text was updated successfully, but these errors were encountered: