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

[WIP] Allow all tfr_functions to operate on Raw object #8781

Closed
wants to merge 17 commits into from

Conversation

adam2392
Copy link
Member

@adam2392 adam2392 commented Jan 24, 2021

Reference issue

Addresses tfr discussion in: #8773

What does this implement/fix?

  1. Allows tfr functions to operate on Raw object that returns and AverageTFR object.
  2. Adds an API to AverageTFR for plotting a spectrogram and averaging frequencies between two ranges

Additional information

API is draft just to show what it would be like. I have not implemented 2. yet, and can take that out if you think it is not necessary/useful.

Related discussion on discourse: https://mne.discourse.group/t/is-there-a-way-to-obtain-the-time-points-from-time-frequency-stft/2682/3

@adam2392
Copy link
Member Author

@agramfort this is something along what I would propose for what we disucced in #8773

@agramfort agramfort marked this pull request as draft January 25, 2021 08:04
@adam2392 adam2392 marked this pull request as ready for review February 16, 2021 22:13
@adam2392 adam2392 changed the title [WIP] Add direct raw -> AverageTFR function (essentially spectrogram) [WIP] Allow all tfr_functions to operate on Raw object Feb 16, 2021
@adam2392
Copy link
Member Author

@agramfort just wanted to check in with you to see if this is going in the right direction? The main point is to allow tfr functions to operate on Raw objects with a disclaimer in the docstring about size/runtime.

Then a few questions regarding unimplemented functions:

  • average_freqs: the intention is to provide a way to average within the frequency range too for things like computing "gamma band power", or something like that.
  • plot_spectrogram: the intention here is to provide a plotting mechanism for plotting channels X time with frequency power (possibly in dB) as the color

^ Are these two functionalities desirable in the _BaseTFR class?

@agramfort
Copy link
Member

agramfort commented Feb 19, 2021 via email

@adam2392
Copy link
Member Author

The main point is to allow tfr functions to operate on Raw objects with a disclaimer in the docstring about size/runtime. Then a few questions regarding unimplemented functions: - average_freqs: the intention is to provide a way to average within the frequency range too for things like computing "gamma band power", or something like that.
for this usecase I would raw.filter(...).apply_hilbert(envelope=False) it's much faster than an STFT and averaging. It's also much easier to explain.

I might not be an expert in time-freq signal processing, but aren't there some advantages to using FFT like techniques and then averaging (https://mne.tools/stable/auto_examples/time_frequency/plot_time_frequency_simulated.html), or is Hilbert Transform envelope considered the best for showing things like frequency bands?

I might be lacking some basic working knowledge here... Lmk if instead you would prefer I post a more general question on the Discourse.

  • plot_spectrogram: the intention here is to provide a plotting mechanism for plotting channels X time with frequency power (possibly in dB) as the color ^ Are these two functionalities desirable in the _BaseTFR class?
    how would it look like? you would display minutes of data? would you have a way to scroll?

It would just be a static heatmap

@agramfort
Copy link
Member

agramfort commented Feb 21, 2021 via email

@adam2392
Copy link
Member Author

Okay sounds good! Thanks @agramfort for answering questions. I've updated the PR now to just reflect the title of allowing TFR funcs to operate on Raw objects. Lmk if this is good?

@adam2392 adam2392 changed the title [WIP] Allow all tfr_functions to operate on Raw object [MRG] Allow all tfr_functions to operate on Raw object Feb 22, 2021
Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you miss a what's new entry and I would strongly recommend to update a tutorial on time frequency to document this option

mne/time_frequency/tfr.py Outdated Show resolved Hide resolved
mne/time_frequency/tfr.py Outdated Show resolved Hide resolved
adam2392 and others added 3 commits February 25, 2021 10:28
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
@adam2392 adam2392 changed the title [MRG] Allow all tfr_functions to operate on Raw object [WIP] Allow all tfr_functions to operate on Raw object Feb 25, 2021
if isinstance(inst, BaseEpochs):
data = inst.get_data()
elif isinstance(inst, BaseRaw):
# make data into 3D to play nice with epochs x ch x time
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should you also do this:

if return_itc:

I am thinking that maybe you can use the same code path for Evoked and Raw? They basically both contain 2D data arrays

Copy link
Member Author

@adam2392 adam2392 Feb 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like what's done here: c14d080?

Is this what you meant?

@adam2392
Copy link
Member Author

Added an extension to an existing tutorial. Lmk if you think this is good, or if I should add more?

@agramfort
Copy link
Member

@adam2392 can you share some rendered doc page when you have one on CI? thx

@adam2392 adam2392 closed this Aug 25, 2021
@adam2392 adam2392 deleted the tfr branch August 25, 2021 01:31
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.

None yet

2 participants