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

Create annotation type #123

Open
rob-luke opened this issue Aug 22, 2021 · 12 comments
Open

Create annotation type #123

rob-luke opened this issue Aug 22, 2021 · 12 comments

Comments

@rob-luke
Copy link
Owner

To replace triggers type. Annotations are more general and can be used to encode things beyond triggers, such as bad segments, blinks, etc

@behinger
Copy link
Contributor

I started looking through the code, but I'm still a julia newbie in many ways, so I cannot comment on style so much.

The topic of event structure is dear to me. I would strongly encourage getting rid of "simple" triggers (like events in MNE python) and just go all in for a Table/dataframe-like structure with arbitrary columns (mandatory "onset (in s)" "duration"). Maybe this is what you had in mind with Annotations already?

Translation from second to sample can be performed within the epoching function.

I found MNE's event - annotation - metadata approach always very confusing and imho a bit of historical baggage from the stimulus-channel times.

What do you think?

@rob-luke
Copy link
Owner Author

Interesting comment, thanks @behinger. I actually found the MNE annotation structure to be mind blowingly awesome when I used it. I found that being able to have channel and time specific annotations was fantastic, and because the annotations are not just trigger events but you can also mark artifacts or movements, blinks etc, then you can do things like only reject eye blinks in the frontal channels, or only reject movement artefact on the neck channels, very powerful. What is a situation where the MNE annotation type has not been sufficient for your uses?

Currently this package uses a dict rather than dataframe (to reduce dependencies). But it would allow arbitrary columns as you suggest. So we could add a column for "channel", and then the trigger event would only apply to that channel.

Maybe this is what you had in mind with Annotations already?

I was thinking to wrap the existing dict in a type, so you could excuse useful functions on it rather than just passing a dict around (seems more Julian way). So I think this would meet you requirements above, and doesn't really change the underlying code we have now, just adds a few wrapper functions. And we could allow adding more fields to the dictionary

But in the long run I was thinking you could have a Annotation type that contained an onset, duration, channels, name, etc. The you would have an array of these to describe your experiment. I think if you can highlight a few scenarios where the current MNE approach is not sufficient for you, then I can better consider your issues when designing this future enhancement.

@behinger
Copy link
Contributor

Channel-Wise annotation sounds super!

Indeed, I just want to have arbitrary columns, the annotation object in MNE officially only supports "onset","duration","description","ch_names" - afaik you could add my own columns, but I haven't done that.

I could, of course, put all my event descriptions in "description", but e.g. for an event "stimulus", I might have "conditionA: 'high'", "conditionB: 'dark'", "continuousA: 73", thus multiple values that I want to keep attached to this event (and the resulting epochs if any)

We could use Table.jl instead of dicts?

@rob-luke
Copy link
Owner Author

Ok I am starting to get an idea of how this might all come together, so I will assign this task to myself so no one duplicates work.

I still don't think I understand your use case @behinger. Could you spell it out for me. It sounds to me that you just wish to have additional metadata fields for each event/annotation/trigger. But then you also say that what you want to do is not possible with MNE, but what you describe is definitely possible using the metadata field in their epoch structure.

I would prefer to ensure I understand your use case before diving in to details such as which underlying data structure to use, so can you give me a clearer description of your use case please.

@behinger
Copy link
Contributor

Metadata are only for epochs, I'd need this for continuous EEG data, which is the basis of unfold. Having multiple concepts is quite confusing and I do not see the benefit

Multi-Column Event descriptions are also part of BIDS (I discuss this here:mne-tools/mne-bids#848). In MNE there was such a discussion as well (on the complications of using events/annotations/metadata all at the same time), but it was decided to keep the current situation.

@rob-luke
Copy link
Owner Author

"so can you give me a clearer description of your use case please."

@rob-luke
Copy link
Owner Author

rob-luke commented Sep 16, 2021

Metadata are only for epochs

Agreed, my bad, I was getting confused.

But I still dont understand what you are asking for. Can you please spell your use case out completely in a single comment.

@behinger
Copy link
Contributor

Use case regression ERPs:

mass univariate models allow fitting something like ERP ~ 1 + condition + luminance

For this, each epoch needs "metadata" on condition and luminance. I.e. a table per epoch to look up this information.

But in Unfold, we do not have epochs, we apply the regression model to continuous data. Thus, I need metadata per event, and not per epoch.

Use case ERPimages

An epoch could span multiple events, each with multiple metadata. E.g. ERPimages are often sorted by the metadata of later events (e.g. with eye-movement related events, sort events by the saccade amplitude of the following fixation). Importantly, those events might not be epochs. If metadata is attached only to epochs, this would complicate this usecase

E.g. here, (simulated) data are cut to stimulus onset & I sorted the data based on the next buttonpress event

grafik

In General, I think such metadata should be applied to events and noch epochs, because it is the more general concept. But the solution seems quite simple imho, just allow for arbitrary fields in the annotation/event/metadata/trigger (however you want to name it). I might be missing something though

@rob-luke
Copy link
Owner Author

Ok I think I finally got it. Thanks for sticking with me.

This will actually also work for GLM style analysis for other modalities such as fNIRS.

Once I get a working PR is it ok for me to ping you for a review @behinger ?

@behinger
Copy link
Contributor

absolutely!

Btw. Unfold.jl already supports basic hemodynamic response modelling (and FIR) - it is super easy to add different temporal basis functions as well i.e. extending the HRF with derivations etc.

@rob-luke
Copy link
Owner Author

Unfold.jl already supports basic hemodynamic response modelling (and FIR)

Might be worth a phone call some time to see where our research paths overlap, I think we have a bit in common.

@behinger
Copy link
Contributor

shoot me an email: rob-luke@benediktehinger.de :)

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

2 participants