search

mne-tools / mne-connectivity

Connectivity algorithms that leverage the MNE-Python API.
https://mne.tools/mne-connectivity/dev/index.html
BSD 3-Clause "New" or "Revised" License
68 stars 34 forks source link

Phase-slope index using spectral_connectivity_time instead of spectral_connectivity_epochs? #178

Open seqasim opened 8 months ago

seqasim commented 8 months ago

Describe the problem

Currently, the function for computing phase-slope index is hard-coded to be computed over epochs to produce time-resolved PSI, and there's no option for computing the phase-slope index over time to produce PSI per epoch. This would be useful if you think the average directionality between two signals over the entire timecourse is informative. You could compute this by binning your trials into categories (i.e. good trials v. bad trials) and then computing PSI, but there's no way to compute PSI per epoch and relate it to continuous trial-level predictors. This should be possible unless I'm missing something

Describe your solution

  1. Add a _cohy function to mne_connectivity.spectral.time by removing the absolute value from _coh

  2. Mimic conditional statements for coh for cohy in _pairwise_con and _parallel_con

  3. Create a new function phase_slope_index_time that mimics phase_slope_index but utilizes our new computation of cohy using spectral_connectivity_time instead of spectral_connectivity_epochs

Does this sound reasonable or am I missing some obvious reason not to do this? If so, I'm happy to implement this and test and make a pull request.

adam2392 commented 8 months ago

This seems reasonable to me. IIUC, the original PR just didn't add cohy to ere on the side of simplicity.

@tsbinns any thoughts?

tsbinns commented 8 months ago

I don't see any problems. With #163 merged spectral_connectivity_time can return complex values, would just need to list the new cohy method here alongside cacoh: https://github.com/mne-tools/mne-connectivity/blob/2af1398e7784350ec8f38c71061668e677fa27c0/mne_connectivity/spectral/time.py#L563-L569

adam2392 commented 8 months ago

SG. Feel free to submit a PR @seqasim

seqasim commented 5 months ago

Sorry for the delay - created a PR here: https://github.com/mne-tools/mne-connectivity/pull/210

seqasim commented 4 weeks ago

Also, I believe the same logic can be applied to dPLI?

tsbinns commented 2 weeks ago

Also, I believe the same logic can be applied to dPLI?

Seems like it. Perhaps that's worth a fresh PR to reduce individual diffs?