[ENH] Add connectivity simulation function

[tsbinns]

Summary

Adds a new public function make_signals_in_freq_bands() inside mne_connectivity/datasets/frequency.py for a basic simulation of connectivity, following @adam2392's suggestion in #163.

API/Documentation

The function has parameters for what I feel are the most important simulation options.

I added a new "Dataset functions" section to the API page since I wasn't sure how well it fit under the existing headers and link to the function here. Very open to change this if you have better suggestions.

Unit tests

I added a new set of tests in mne_connectivity/tests/test_datasets.py.

test_make_signals_error_catch() checks that some obvious errors are caught. If you think there are some important checks I've missed please let me know.

test_make_signals_in_freq_bands checks that the simulations are actually working properly. I focused on Coh, ImCoh, and DPLI with: 1) no time delay in interaction between seeds and targets; 2) a positive time delay (i.e. seeds drive communication with targets); and 3) a negative time delay (i.e. targets drive communication with seeds). From this we expect:

• Case 1 (no time delay) - Coh should be high for the simulated frequency band, ImCoh should be low, DPLI should be ~0.5.
• Case 2 (positive delay) - Coh should be high, ImCoh should be high, DPLI should be > 0.5.
• Case 3 (negative delay) - Coh should be high, ImCoh should be high, DPLI should be < 0.5.

I test these with a higher and lower SNR, different number of seeds and targets, and the different CSD computation modes. I realise it's not an exhaustive check, but I think the tests nicely capture what we expect to see from undirected (Coh, ImCoh) and directed (DPLI) connectivity methods, and also methods that do not capture zero time-lag interactions (ImCoh).

Use elsewhere in the package

This simulation function is similar (but not identical) to those used for many of the unit tests in spectral.py. Out of interest I wanted to see if this function could be used to replace these simulations. In a number of cases it works, but in others it would require redefining what is considered acceptable connectivity values for particular method/CSD mode combinations.

This could be done, but it would require a considerable amount of effort tweaking these thresholds which I can't really invest the time for at the moment.

How would people feel about just having this new function which can be used going forward for e.g. the CaCoh examples in #163 without changing the existing unit test stimulations?

[larsoner]

This could be done, but it would require a considerable amount of effort tweaking these thresholds which I can't really invest the time for at the moment

Can you just add a # TODO: Use <whatever> for this after tweaking tolerances so we don't forget that this is a possibility? Seems okay to leave it to future work

[adam2392]

How would people feel about just having this new function which can be used going forward for e.g. the CaCoh examples in #163 without changing the existing unit test stimulations?

Makes sense to me!

[tsbinns]

@larsoner @adam2392 Thanks both for the feedback!

I replaced the simulations where no values were being checked (e.g. for error catching parameters, saving & loading, etc...), and marked 5 tests where the simulations could be replaced with make_signals_in_freq_bands following some tweaks to the acceptable values.

[adam2392]

Some minor changes, but this overall looks good to me

[tsbinns]

Looks like some of the thresholds were just on the boundary and updating the way in which the random numbers were generated caused these to fail, so tweaked the thresholds slightly.

[adam2392]

Nice!

[adam2392]

I'll let @larsoner and @drammock take a quick look.

Perhaps @ruuskas is also interested given he found a bug with the previous implementation? Hopefully this will simplify life for generating simulated data with expected correlations in the frequency domain

[adam2392]

Perhaps something to help move this forward is refactoring #163 to use this function? (I.e. merge changes might work?...)

I would keep a local copy in case git messes up.

Then we can see the simulation function work as intended for the new PR?

If that's a bit hairy, I'm okay merging for now. Wdyt @larsoner

[tsbinns]

@adam2392 Sure, I'm open to this. Will be away at a conference rest of this week, but can start on Monday.

[adam2392]

No rush! Enjoy!

[tsbinns]

@adam2392 I've refactored #163 to use the new simulation function in the CaCoh and coherency method comparison examples where I was previously using a function within each example file.
Everything still works, no problems switching to the dedicated function.

[adam2392]

Ah very cool! I think this LGTM as a start to simplifying data generation in our connectivity analyses.