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 Artifact Blocking as an optional preprocessing step #73

Open
christian-oreilly opened this issue Mar 19, 2023 · 9 comments
Open

Add Artifact Blocking as an optional preprocessing step #73

christian-oreilly opened this issue Mar 19, 2023 · 9 comments
Labels
enhancement New feature or request
Milestone

Comments

@christian-oreilly
Copy link
Collaborator

Artifact blocking is described here: https://www.researchgate.net/publication/4248725_A_Simple_and_Fast_Algorithm_for_Automatic_Suppression_of_High-Amplitude_Artifacts_in_EEG_Data

This approach has been suggested has being advantageous compared to ICA for infants data. We propose it here as an omnibus artifact rejection optional step, not a replacement to ICA because beside rejecting artifact, ICA is also used to classify them.

@christian-oreilly christian-oreilly added the enhancement New feature or request label Mar 19, 2023
@christian-oreilly
Copy link
Collaborator Author

@dsrishyla FYI, here is the Python translation of the Matlab code you obtained from the authors: https://colab.research.google.com/drive/1LM_fkuxR2XiQRmT6kVupkcRlAHhZ1AsU

@Andesha
Copy link
Contributor

Andesha commented Mar 20, 2023

Just a note - any sort of extra preprocessing can be called as part of the staging script

does that have support currently?

@christian-oreilly
Copy link
Collaborator Author

@Andesha We initially put something for the "staging script" but then @scott-huberty added to the pipeline whatever was in there originally (e.g., dealing with recording gaps if I remember well)... and we were unsure about the "staging script" concept in general. As originally implemented, it would run whatever code is provided by the user by something analogous to an eval() call which is a well-know security risk... I am wondering if we should not think of something more like "modules" or "add-ons" or "plugins" (or whatever you want to call these) instead of a user-defined script.

@Andesha
Copy link
Contributor

Andesha commented Mar 20, 2023

Assuming you have perfect knowledge of what goes on in recordings (which never happens lol) you would do that sort of recording gap thing inside of the init procedure.

Staging is meant to be your "last chance before we actually interpret this signal" step. i.e., you find out that in a particular subject you NEED to manually mark out a channel but don't want to re-init it.

A second example would be wanting to mark more time as out of task as sort of outlined above. You could re-init (which sucks) or put some stuff into staging.

hope that helps!

@Andesha
Copy link
Contributor

Andesha commented Mar 20, 2023

and yes - security problems all over the place 😆

@christian-oreilly
Copy link
Collaborator Author

Well, if we are to use something like that, I would just allow the used to set a custom function as a callback, which seems safer to me than an eval() call on a script on the hdd...

def my_staging_function(...):
    ....

staging_kwargs = {...}


ll.set_staging_function(my_staging_functions, staging_kwargs)

@Andesha
Copy link
Contributor

Andesha commented Mar 20, 2023

That sounds great. Also as long as the docs mention something along the lines of "be very sure about your staging scripts, and don't run other people's without checking first" you're probably fine...

@christian-oreilly
Copy link
Collaborator Author

Yeah, but using a callback like this might somewhat by-pass the issue altogether since you can just have this function defined at the top of the script where you run the pipeline... or you import it from another file like any other functions, so I guess it can be argued not be more dangerous than any other import... although I am not a security guy :)

@scott-huberty
Copy link
Member

scott-huberty commented Apr 17, 2023

I'm +1 for adding this, preferable for v.02.

Based on my limited knowledge of AB, if my assumption is right that it can be applied to low-density EEG ( 32 or less channels), which ICA is said to not be so good for - I think this could be a good selling point for the pipeline as it would open it up to users with low-density systems.

I also think this could be a nice opportunity for Diksha to become involved!

@scott-huberty scott-huberty added this to the 0.03 milestone Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants