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

Allow adding pipeline.flaggedChs to raw.info['bads'] #72

Open
scott-huberty opened this issue Mar 17, 2023 · 6 comments
Open

Allow adding pipeline.flaggedChs to raw.info['bads'] #72

scott-huberty opened this issue Mar 17, 2023 · 6 comments

Comments

@scott-huberty
Copy link
Member

scott-huberty commented Mar 17, 2023

I imagine there is going to be disagreement on this so I want to get the conversation started:

If I run the code in test/test_simulated.py I get this:

image

  • In the automated pipeline we flag time periods as annotations starting with _bad. Annotations are saved directly in the file when exporting to .edf.
  • We do not add flagged channels to raw.info['bads'], we only save them to pipeline.FlaggedChs

This is inconsistent and confusing. Right now the user will see some of the pipelines decision in their file and need to add extra boilerplate to see the flagged channels.

Also QCGUI does nothing with flaggedChs after loading, and MNEVisualizer only knows about raw.info['bads'], so the dashboard doesnt show the pipelines decisions on channels right now and if the user clicks on a channel, it is added to raw.info['bads'], not flaggedChs.

I think the closer we integrate with MNE here the better. I think the FlaggedChs.tsv file is necessary but lets not make everyone life harder with more boilerplate just to see decisions on their data that aren't even destructive, for the sake of purity.

@christian-oreilly
Copy link
Collaborator

I think these things should remain separate (for various reasons including the philosophy behind lossless and the fact that there is a degeneracy when we go from FlaggedChs to info["bads"]) but we should provide utilities to help to make the bridge (i.e., something like raw = mark_bad_channels(raw, kinds=["eog", "ch_sd"])). Also, I think that this issue is rehashing discussion we already had (e.g., with respect of how we distinguished "bads" in MNE vs as automatically marked by the pipeline vs manually marked as bad by the user). This also relates somehow to using HED tags (issue #70; i.e., how we want to carry information of a full annotation as opposed to a binary classification).

@scott-huberty
Copy link
Member Author

we should provide utilities to help to make the bridge

I was also thinking we could add a kwarg to run like add_bads that the user can flip on to add the flagged_chs to bads and to prepend _bad to annotations. saves them the extra step and I think many users would like this (even I wished I had this while testing). It also makes our treatment of flagged_epochs and flagged_chs more consistent.

I don't think any info is lost. The reason for it being flagged is still in flaggedChs.

Also +1 for a helper function.

@christian-oreilly
Copy link
Collaborator

The add_bad arguments would just trigger the execution of the mark_bad_channels(...) at the end of the pipeline. I'd make it either a boolean or a list of strings or a dictionary
False: Nothing gets marked as bad
True: Everythin gets marked as bad
["ch_sd"] : Mark as bad everything labeled with "ch_sd", regardless of whether it is for channels, epochs, of ICS
{"epochs": ["ch_sd"]} : Mark as bad every "epochs" labeld with "ch_sd" (not epochs really, but time windows).
This input could be passed directly to the kinds argument of the mark_bad_channels() function.

@scott-huberty
Copy link
Member Author

Yeah I think this is a nice solution. Glad we could find some common ground 😉

@Andesha
Copy link
Contributor

Andesha commented Jun 26, 2023

is there any updates to this now that flagging has changed formats slightly? @scott-huberty

@scott-huberty
Copy link
Member Author

scott-huberty commented Jun 26, 2023

is there any updates to this now that flagging has changed formats slightly? @scott-huberty

So We landed on following lossless's historical step of requiring a deliberate purge of annotations.

But practically speaking, so far all user (us) just want to purge the annotations.

So We need to add a helper function to the pipeline to convert flags to bads, and make the default behavior of the pipeline to call this function at the end of the run, that a user could set to False to disable.

The routine would be Something like:

flagged_chs=pipeline.flags['ch'].copy()

flagged_chs.pop('rank')
Pipeline.raw.info['bads'].extend(flagged_chs.values())

# ditto for ICs 
...

I started the code for this on another branch. I could push it up this week hopefully?

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

No branches or pull requests

3 participants