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

Allow Spectrum objects to support complex coefficients #12728

Closed
tsbinns opened this issue Jul 17, 2024 · 1 comment
Closed

Allow Spectrum objects to support complex coefficients #12728

tsbinns opened this issue Jul 17, 2024 · 1 comment
Labels

Comments

@tsbinns
Copy link
Contributor

tsbinns commented Jul 17, 2024

Describe the new feature or enhancement

Discussed briefly with @larsoner.

We are considering changes in MNE-Connectivity where the spectral coefficients can be passed directly to the connectivity estimation functions in the form of EpochsSpectrum and EpochsTFR objects, as returned from Epochs.compute_psd/tfr(output="complex").

While the complex output is supported for TFR objects, Spectrum objects do not allow complex outputs from the various PSD functions:

if method_kw.get("output", "") == "complex":
raise ValueError(
f"Complex output is not supported in {type(self).__name__} objects. "
f"Please use mne.time_frequency.{psd_funcs[method].__name__}() instead."
)

Describe your proposed implementation

Remove the ValueError for output="complex" from Spectrum classes.

There are a couple places where tweaks would be required, e.g. in _check_values() and handling PSD func outputs in _compute_spectra(), but similar solutions are already present in the TFR classes.

Same situation for the plotting methods, but again TFR classes also have solutions for this already which could be followed:

# complex amplitude → real power; real-valued data is already power (or ITC)
if np.iscomplexobj(data):
data = (data * data.conj()).real

One thing that would need to be added is storage for weights in the Spectrum class when complex output is requested from the multitaper mode, which as I understand is not the case for the TFR multitaper.

Describe possible alternatives

N/A

Additional context

No response

@tsbinns
Copy link
Contributor Author

tsbinns commented Aug 9, 2024

Addressed in #12747

@tsbinns tsbinns closed this as completed Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant