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

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

Open
seqasim opened this issue Mar 25, 2024 · 6 comments

Comments

@seqasim
Copy link

seqasim commented Mar 25, 2024

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
Copy link
Member

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
Copy link
Collaborator

tsbinns commented Mar 26, 2024

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:

for m in method:
# CaCoh complex-valued, all other methods real-valued
if m == "cacoh":
con_scores_dtype = np.complex128
else:
con_scores_dtype = np.float64
conn[m] = np.zeros((n_epochs, n_cons, n_freqs), dtype=con_scores_dtype)

@adam2392
Copy link
Member

SG. Feel free to submit a PR @seqasim

@seqasim
Copy link
Author

seqasim commented Jun 28, 2024

Sorry for the delay - created a PR here: #210

@seqasim
Copy link
Author

seqasim commented Oct 31, 2024

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

@tsbinns
Copy link
Collaborator

tsbinns commented Nov 11, 2024

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?

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

3 participants