-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
ERP Measurement Tool for MNE #7848
Comments
hi @jdkoen my suggestion would be to start small with what you consider the most key feature of ERPLAB. What would that be? We'll be happy to support you. |
Thanks @agramfort! One of the key features (at least to me) is pulling out mean amplitude in a user defined time window. I will start with that, and post here if I have questions or need help. |
I would do this with a .mean() method on Evoked like we have a .mean()
method on stc
… |
Thank you for the suggestion.
I just noticed that the .get_peak() method returns the peak amplitude/time
over an entire class of channels (i.e., the peak across all channels).
Would it be useful to have a different function that extracts the peak
amplitude/voltage for a user-specified set of channels? I was unsure of the
rationale for taking a 'global' peak value/latency.
The reason I ask the above question, and more generally, is I wonder if it
will be useful to create a new class to deal with extracting data from
evoked objects. From what I have in mind, it would require ~5-6 new methods
on the Evoked class (with associated private functions).
Currently familiarizing myself with the code structure, but wanted to ask
this now before I start to code.
…On Fri, May 29, 2020 at 3:19 AM Alexandre Gramfort ***@***.***> wrote:
I would do this with a .mean() method on Evoked like we have a .mean()
method on stc
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7848 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXXPVF5ZEBID2KUPEH6NPLRT5OZNANCNFSM4NNL24NQ>
.
|
@jdkoen for a peak over a subset of channels, there are methods Note the use of |
Thanks @drammock for that feedback. That approach just slipped my mind. Makes a lot more sense now for how to format the mean signal measure I'm trying to add. |
Chaining can be useful here too: evoked.copy().crop(tmin=0.09, tmax=0.11).pick_channels(channel_list).data.mean() will give you average over specified time window and Maybe a good first step would be to add a short example/tutorial dealing with just a few of these measures and then we'll see if some turn out to be too complex to obtain with what is available in mne now. |
I think we should make a tutorial on ERP-style analyses that contain 90% of the analyses people would be interested in. Even if it's long, a |
@sappelhoff or @mmagnuski up for coding this in the next couple of weeks by any chance? |
I am pretty packed so would be grateful if somebody else can take the time. But I agree that this would be very useful to have. |
@larsoner Sorry, I won't be able to do this for the next release. But I will have more time in about 1.5 months - then I could help with such tutorial. :) |
Sounds good. Or @jdkoen if you've made some progress, you could also take a stab at it if you want! |
Thanks. I’m happy to take a go at it. Sorry for the delay getting back to
this.
…On Tue, Aug 25, 2020 at 7:23 PM Eric Larson ***@***.***> wrote:
Sounds good. Or @jdkoen <https://github.com/jdkoen> if you've made some
progress, you could also take a stab at it if you want!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7848 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXXPVE4BYDFCSVXVP2JHILSCRBWNANCNFSM4NNL24NQ>
.
|
Here are my thoughts on what measures to add for a tutorial:
1) Peak amplitude/latency for a single channel
2) Mean amplitude in a time interval
3) Fractional Area Latency (for an example, 50% area)
4) Fractional Peak Latency (to measure onsets)
There are some other measures (e.g., integral measures) that I am not
looking at here. Also, for peak measures (1, 3, and 4 above) there should
likely be options to find absolute, positive, and negative peaks.
Any thoughts on this?
…On Tue, Aug 25, 2020 at 7:25 PM Joshua Koen ***@***.***> wrote:
Thanks. I’m happy to take a go at it. Sorry for the delay getting back to
this.
On Tue, Aug 25, 2020 at 7:23 PM Eric Larson ***@***.***>
wrote:
>
>
> Sounds good. Or @jdkoen <https://github.com/jdkoen> if you've made some
> progress, you could also take a stab at it if you want!
>
>
>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#7848 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAXXPVE4BYDFCSVXVP2JHILSCRBWNANCNFSM4NNL24NQ>
> .
>
>
>
|
I would start with some minimal set and we can always add more later. No need to be feature-complete from the start. So this seems fine. One other one I see/use from time to time is area under the curve, but again we can add that later |
Just a cross-ref here that we should bring up the "head" coordinate frame standard in MNE in this tutorial as well (see #7455 for example) |
Pushing the milestone to 0.22 for this one so it's not a blocker, but @jdkoen there is still time to get this in for 0.21 if you have time to make a push in the next few days |
I will have to get to it for the 0.22 release. Hopefully I will have a
little more spare time at the end of this upcoming week.
…On Wed, Sep 9, 2020 at 9:32 AM Eric Larson ***@***.***> wrote:
Pushing the milestone to 0.22 for this one so it's not a blocker, but
@jdkoen <https://github.com/jdkoen> there is still time to get this in
for 0.21 if you have time to make a push in the next few days
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7848 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXXPVAV4J4GL2XOXTG4AODSE57WFANCNFSM4NNL24NQ>
.
|
@jdkoen we might push 0.22 up to a December rather than March release, feel free to give a PR a shot in the next week or so if you can, otherwise 0.23 is also useful! |
Will do. Going to start work on it tonight.
…On Tue, Dec 1, 2020 at 1:55 PM Eric Larson ***@***.***> wrote:
@jdkoen <https://github.com/jdkoen> we might push 0.22 up to a December
rather than March release, feel free to give a PR a shot in the next week
or so if you can, otherwise 0.23 is also useful!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7848 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXXPVGPA5W4K7JFBMBXMVTSSU33DANCNFSM4NNL24NQ>
.
|
Hello, I was recently reading the chapter about erp peak in the luck's book, and I am very interested in erplab's peak finding tool. But mne's peak finding tool may indeed be a bit unusable for me. When I used mne to analyze eeg data, I exported evoke to pandas after all the analysis was completed to do time slices and find peaks. I feel that this method does have a big problem. |
@agramfort to get at your question about implementation, one way would be to add methods to the
Ideally, there would also be some other functions as well to plot the results easily so that the data can be inspected. @agramfort does this approach fit within the scope/organization framework of MNE-Python development? Another approach is to make a plugin for MNE python with different classes that could have a |
for now we just a nice tutorial in
https://mne.tools/dev/auto_tutorials/evoked/30_eeg_erp.html#amplitude-and-latency-measures
and no public functions. I would not support adding more public methods to
Evoked but rather have (if more and more
people are interested in this) a dedicated module maybe in mne.stats?
… Message ID: ***@***.***>
|
@agramfort @jdkoen I would like to start working on this soon. Do you still suggest creating an entirely new module? |
@withmywoessner sure, I suggest adding a Please start small in terms of actual additions. For example the first PR could add just #10647. Or maybe starting with the four measures you mention in #12406 wouldn't be too onerous to review assuming they are mathematically very simple (and well documented in some paper). |
nowadays this should go in the |
Ahh right didn't realize we had lazy-loader-ized |
I'm torn because it doesn't seem useful to have two different functions because their functionalities are so similar, but it's hard to come up with a good name. Maybe |
I think I actually disagree here; we could think of |
What about naming it something like I think I may be leaning more towards one function because it means documentation is in one place. I will ask my coworker who doesn't use Python. EDIT: I Asked my coworker and he said he didn't care. |
No strong feeling from me, |
Okay, @drammock. I will defer to you if you think it would be better. |
The Evoked object only has methods to return peak amplitude and latency at present. Is there any interest in the development of additional amplitude and latency measures (e.g., average amplitude in a time window, fractional peak latency)? My thought would be to port the some of the measures implemented in ERPLAB's ERP Measurement Tool to MNE to help facilitate ERP analysis using MNE.
I am happy to lead development of this. These features could be added as methods to the Evoked object, or alternatively be included as a separate library. Is there a preference on which route is best?
Also, while I have coded in Python before, this will be my first time contributing to any Python software development. Thus, I would likely need some help and guidance on navigating development of unit tests and other aspects of contributing.
The text was updated successfully, but these errors were encountered: