search

voytekresearch / pacpy

Calculate phase-amplitude coupling in Python (and Matlab).
MIT License
24 stars 12 forks source link

Contributing a high-level PAC function #45

Open choldgraf opened 8 years ago

choldgraf commented 8 years ago

Hey folks - I've got a PR right now in the MNE-sandbox repo, which adds a high-level function to do a bunch of PAC stuff between signals. It lets you:

  1. Give an input that is shape (n_signals x time), and specify pairs of indices that you want to calculate PAC between.
  2. Give a list of phase frequencies and amplitude frequencies to calculate PAC between.

It will then pre-filter the proper channels only one time per pair of low/high frequencies, so that we don't duplicate filtering etc.

You can also:

  1. Give a list of indices corresponding to events
  2. Give a list of tmin / tmax pairs either relative to event onsets, or to the whole dataset

It will then calculate PAC using the metric of choice for each pair of channel indices, phase/amp frequencies, and tmin/tmax window. With the option of either calculating this per epoch, or concatenating across epochs first.

So this way, you could make PAC plots with this function like this:

image

Or like this:

image

Or do a connectivity plot w/ lots of pairs of electrodes.

In the PR we've been discussing whether it would make more sense to contribute this to PacPy, since there's a hidden function that's basically designed to not be specific for MNE objects, but instead operate on arrays. Would you all be interested in this? I'd be willing to take some time porting over the tests and the visualization functions (or whatever subset of that you're interested in). However I know that you guys generally want to keep this repo restricted to just the PAC metrics themselves.

LMKWYT. Pinging @srcole and @parenthetical-e in case you guys have thoughts ;)

srcole commented 8 years ago

I think it'd be great to add to pacpy! This new function would definitely be an improvement to the current 'comodulogram' function that currently repeats filtering and also doesn't have these nice visualizations.

choldgraf commented 8 years ago

The visualizations are pretty simple to implement, and I don't think they'd need to be part of a PR. The benefit of the function is that it can output PAC as an array of shape (n_pac_metrics, n_epochs, n_channel_pairs, n_frequency_pairs, n_time_windows) so that you can then slice and dice the output however you like :)

srcole commented 8 years ago

True. Yeah, I think this will be good to provide that functionality for people analyzing trial data sets with many simultaneous recordings.

choldgraf commented 8 years ago

@parenthetical-e what do you think? I believe you were the most hesitant to start adding extra functionality to pacpy (which I think has reasonable arguments behind it). IMO this is a close-enough extension to the functionality that's already there, so doesn't deviate too much from the overall goal of pacpy. What do you think?

parenthetical-e commented 8 years ago

I am all for adding the ability to work with trials and 2-d arrays of data, but am very much against the making a monolithic function like,

phase_amplitude_coupling(..., method='plv')

Something like,

from pacpy.pac import plv
trials(lo, hi, f_lo, f_hi, trial_index, pacfn=plv)
channels(X, f_lo, f_hi, lo_index, high_index, pacfn=plv) # where X is 2d
trials_and_channels(X, f_lo, f_hi, trial_index, lo_index, high_index, pacfn=plv)

is in the spirit of the pacpy interface, maintaining the semi-functional and transparent implementation I've tried hard to imbue. In sum, I'd much rather pass simple functions than flags. I'd be happy to adapt what you've got to this kind of interface, and help in wrapping that up for mne. Is that a workable/useful/interesting compromise?

As for plotting routines, I'm neutral. As long as your functions return easily plot-able things, explicit routines seem to me to be unneeded. But peoplenotme seem to like them, so sure. Why not?

parenthetical-e commented 8 years ago

(Sorry. Had no intent to close this. I've thumb hands.)

choldgraf commented 8 years ago

@parenthetical-e I think I'm +1 to that, but let's chat about implementation etc. The reason that I've got all those flags is because I'm assuming that the input to the function is always the same type (channels x time) and it always does the filtering on that data first, before it does any slicing and dicing. That's why I don't have, e.g., an extra function for trials-shaped data. (and why this one is kind of a mammoth)

choldgraf commented 8 years ago

if you wanna skype and chat about the function etc, then I'm down for that. Only catch is that I'll be out of communication from Thursday to ~Tuesday (I'll be in Yellowstone hanging out with elks and stuff :D )

parenthetical-e commented 8 years ago

Let's just chat when you get back. Thursday the 9th? 10:30 AM?

choldgraf commented 8 years ago

wellllll, I might be leaving for knight lab camping on Thursday the 9th, so wednesday would be better for me if that works for you.

On Tue, May 31, 2016 at 3:57 PM, Erik notifications@github.com wrote:

Let's just chat when you get back. Thursday the 9th? 10:30 AM?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/voytekresearch/pacpy/issues/45#issuecomment-222846081, or mute the thread https://github.com/notifications/unsubscribe/ABwSHfeb3ZaHWdfOlnuGnFdMYKWitKgWks5qHLzVgaJpZM4Iq3iY .

parenthetical-e commented 8 years ago

Sure. 10 AM ok?

choldgraf commented 8 years ago

Sounds good to me, Wed June 8th, 10AM.

On Tue, May 31, 2016 at 3:58 PM, Erik notifications@github.com wrote:

Sure. 10 AM ok?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/voytekresearch/pacpy/issues/45#issuecomment-222846322, or mute the thread https://github.com/notifications/unsubscribe/ABwSHbpRcGI128I--E-lJkhHwyMNQaYcks5qHL0sgaJpZM4Iq3iY .

srcole commented 8 years ago

Works for me too

parenthetical-e commented 8 years ago

Cools.

On May 31, 2016, at 4:01 PM, Chris Holdgraf notifications@github.com wrote:

Sounds good to me, Wed June 8th, 10AM.

On Tue, May 31, 2016 at 3:58 PM, Erik notifications@github.com wrote:

Sure. 10 AM ok?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/voytekresearch/pacpy/issues/45#issuecomment-222846322, or mute the thread https://github.com/notifications/unsubscribe/ABwSHbpRcGI128I--E-lJkhHwyMNQaYcks5qHL0sgaJpZM4Iq3iY .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/voytekresearch/pacpy/issues/45#issuecomment-222846692, or mute the thread https://github.com/notifications/unsubscribe/ABIxh2dqXRSk2r55OMLHJcoyJBxwzjM-ks5qHL20gaJpZM4Iq3iY.

choldgraf commented 8 years ago

Hey guys - I just realized that I had already opened this. Let's discuss integrating PAC efficiency etc into pacpy here.