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

[ENH] Multiple improvements to spectral_connectivity_time #113

Closed
wants to merge 67 commits into from

Conversation

ruuskas
Copy link
Contributor

@ruuskas ruuskas commented Nov 24, 2022

PR Description

This PR addresses improvements mentioned in #104.

  • Add ciPLV metric.
  • Compute all connectivity measures at once, avoiding the repeated computation of the cross-spectral density.
  • Reinstitute the option to specify frequencies of interest also when using the multitaper method. The reasoning is that users should have control over the sparsity of the frequency grid.
  • Add the option to use parts of each epoch as padding. This allows for the mitigation of edge effects in the time-frequency decomposition. The edges are removed after time-frequency decomposition, but before connectivity computation.
  • Improve the function description to better describe the objective.

Unfortunately the Git log for this PR is a huge mess. The reason is that this was started before breaking changes were made in #104. Should probably have rebased instead of merging from main.

Merge checklist

Maintainer, please confirm the following before merging:

  • All comments resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • PR description includes phrase "closes <#issue-number>"

ruuskas and others added 30 commits August 31, 2022 12:14
…, then average

FIX: add working support for computation of multiple connectivity metrics at once, as indicated by existing docstring

FIX: correct calculation of PLV and coherence connectivity metrics

FIX: block_size parameter now actually corresponds to the size of blocks instead of number of blocks

ENH: add PLI and wPLI connectivity metrics

ENH: improve docstring and typechecks in code

ENH: streamline the public API with mne_connectivity.spectral_connectivity_epochs

ENH: enable averaging connectivity results over frequencies and epochs
Co-authored-by: Adam Li <[email protected]>
This change will minimize memory usage by default.
Add a short description for the block_size parameter to allow users to
better understand the memory usage of the spectral_connectivity_time
function.
Improve the readability of code by shortening variable names in
pairwise_pli.
Remove the regression test which tests against the spectral connectivity
 implementation in frites. The implementation in frites is erroneous,
 and therefore we should not test against it.
The block_size parameter is not useful, as testing shows that running
the computation in blocks of epochs does not have a meaningful effect on
 the speed of computation, but significantly increases memory usage.
Added a note on memory mapping in the docstring of
spectral_connectivity_time. Corrected some typos and inconsistent
backticks.
Removed redundant comments, clarified and fixed typos.
Co-authored-by: Adam Li <[email protected]>
ruuskas and others added 25 commits November 9, 2022 14:46
Revised documentation of spectral_connectivity_time.
Spectral connectivity computation failed if cwt_freqs was only a single
number or an array with a single entry due to invalid array slicing.

Fixed by incrementing the upper bound of the slice by one when
computing the average in a frequency band.
Compute a weighted average of the tapered cross spectra when using
the multitaper mode. Weighting is
derived from the concentration ratios between the DPSS windows.
This adds the option to use the edges of the signal at each epoch as
padding. The purpose of this is to avoid edge effects generated by the
time-frequency transformation methods.
Made the docstring more stylish, removed unnecessary things and
added better compliance with MNE-Python style guidelines.
The _spectral_connectivity function doesn't need defaults as these are
already spelled out in the main spectral_connectivity_time function
signature.
Sym is not a parameter of dpss_windows. (But is one of the underlying
scipy.signal.dpss)
This change will skip the rendering of the connectivity computation
progress bar if the logging level is not DEBUG. This is in line with
MNE-Python, where progress bars are not shown at INFO or higher logging
levels. Rendering the progress bar regardless of logging levels has the
potential to cause unnecessary clutter in users' log files.
Add a better description of the method + style nitpicks.
@ruuskas ruuskas changed the title Improve spectral time [ENH] Improve spectral time Nov 24, 2022
@ruuskas ruuskas changed the title [ENH] Improve spectral time [ENH] Multiple improvements to spectral_connectivity_time Nov 25, 2022
@ruuskas ruuskas closed this Nov 25, 2022
@ruuskas ruuskas deleted the improve_spectral_time branch November 25, 2022 08:59
@ruuskas
Copy link
Contributor Author

ruuskas commented Nov 25, 2022

I decided to close this PR and repost with the relevant commits properly cherry-picked on top of upstream/main.

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

Successfully merging this pull request may close these issues.

2 participants