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
65 stars 34 forks source link

[BUG] Fix memory error spectral_connectivity_time #175

Closed tsbinns closed 4 months ago

tsbinns commented 4 months ago

Fix for #174.

In short, the array for storing spatial patterns of connectivity used by the MIC method was unnecessarily being created for all methods. This could lead to memory errors when bivariate connectivity methods were being called on large numbers of connections.

This fix makes it so that the patterns array will only be instantiated in methods that require it, otherwise will be set to None. This does not cause any change in behaviour, since for methods where patterns are not used, None was already being stored in the patterns attribute of the returned connectivity objects.

This is a problem specific to spectral_connectivity_time(); spectral_connectivity_epochs() was already using the fixed approach.

tsbinns commented 4 months ago

Currently the only method where spatial patterns are produced is MIC (not MIM or GC methods). I created a new variable _patterns_methods to specify those methods where the patterns array should be initialised: https://github.com/mne-tools/mne-connectivity/blob/93b2f83aaf068fca0e3b7eae2c2d77d14f8ad87c/mne_connectivity/spectral/epochs_multivariate.py#L820-L822

I could have hard-coded this, however I am just thinking ahead to when #163 is finished and the array should also be initialised for CaCoh.

tsbinns commented 4 months ago

Is there a unit-test that we can add to prevent this from occurring, or is this an internal detail?

Personally, I would consider this more of a one-off oversight in the implementation from a mistake on my end.

Tbh I wouldn't know where to start in defining an appropriate memory usage for a function such as this.