search

voytekresearch / pacpy

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

Reason for normalizing to time series length after taking absolute value? #21

Closed hoycw closed 9 years ago

hoycw commented 9 years ago

In line 112 of pac.py in the plv function, after converting to exponential form, y'all take the sum, then absolute value, then divide by the length of the time series. Is there a reason for this implementation, rather than just taking the mean of the exponential form followed by the absolute values?

In code form, why is it pac = np.abs(np.sum(np.exp(1j * (lo - hi)))) / len(lo) instead of pac = np.abs(np.mean(np.exp(1j * (lo - hi)))) ?

srcole commented 9 years ago

The current implementation is how I mentally imagine the calculation. But I understand that's not a justification for coding that way ;) Will change it. Thanks!

hoycw commented 9 years ago

Ah okay, I figured it was something to do with data types or some other coding consideration, basically to make the operation safe. I imagine both methods are equivalent if there aren't any coding concerns (I asked because I couldn't think of any).

One benefit of using the mean version would be that in the case of epoched data and event-related PAC (what I'm doing), you can more easily switch the axis over which you take the average (across trials vs. across time). That might take more reworking with the current sum then divide implementation. Then again, we seem to have decided this implementation isn't for event-related stuff anyways, so it probably doesn't matter! :)

roemervandermeij commented 9 years ago

In order to interpret the PLV as going from 0 and 1 (which is how the PLV is defined), it is necessary to divide by the number of samples (in the case of a time-resolved PLV, this would be the number of trials).

Another option is to take the mean of course :p.

roemervandermeij commented 9 years ago

Um, sorry for stating the obvious, I replied based on the title. Still getting used to commenting on github (lousy excuse)

srcole commented 9 years ago

I changed my mind. I like the code how it is because of how it aligns intuitively with the calculation.

parenthetical-e commented 9 years ago

I changed my mind. I like the code how it is because of how it aligns intuitively with the calculation.

How is obscuring the intent and the math intuitive?

srcole commented 9 years ago

Actually, both methods follow 2 mental imaginations of calculating PLV that are equally valid. I just didn't see why we should make the change to support the other.

But now that we're thinking of supporting trial based stuff (Issue #20 ), then this change makes more sense

parenthetical-e commented 9 years ago

Changed to np.mean() with commit 2d2e2af