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

[MRG] VIZ/DOC/ENH: add time frequency gfp example #4139

Merged
merged 20 commits into from Apr 1, 2017

Conversation

Projects
None yet
5 participants
@dengemann
Member

dengemann commented Mar 30, 2017

This is using some old school methods in new school ways to explore spectral temporal evolution.
The (latest) output looks like this:

screenshot 2017-03-31 16 09 21

cc @Eric89GXL @agramfort @jona-sassenhagen

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 30, 2017

Member

Advanced example of a better way of doing #3123.

Member

dengemann commented Mar 30, 2017

Advanced example of a better way of doing #3123.

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 30, 2017

Member

image output updated.

Member

dengemann commented Mar 30, 2017

image output updated.

dengemann added some commits Mar 30, 2017

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
subtract the avarage and finally rectify the signals prior to averaging.
Then the GFP is computed as described in [2], using the
sum of the squares devided by the true degrees of freedom.
Baselinging is

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

Baselining

@agramfort

agramfort Mar 30, 2017

Member

Baselining

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
# Then we prepare a bootstrapping function to estimate confidence intervals
def get_gfp_ci(average, rank=rank):

This comment has been minimized.

@jona-sassenhagen

jona-sassenhagen Mar 30, 2017

Contributor

Nice, will have to use this one for plot_compare_evokeds :)

@jona-sassenhagen

jona-sassenhagen Mar 30, 2017

Contributor

Nice, will have to use this one for plot_compare_evokeds :)

This comment has been minimized.

This comment has been minimized.

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 30, 2017

Contributor

Not much time now, but it looks pretty cool at a glance!

Contributor

jona-sassenhagen commented Mar 30, 2017

Not much time now, but it looks pretty cool at a glance!

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
Then the GFP is computed as described in [2], using the
sum of the squares devided by the true degrees of freedom.
Baselinging is
applied to make the GFPs comparabel between frequencies.

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

comparable

@agramfort

agramfort Mar 30, 2017

Member

comparable

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
applied to make the GFPs comparabel between frequencies.
The procedure is then repeated for each frequency band of interest and
all GFPs are visualized. The non-parametric confidence intervals are computed
as described in [3].

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

computed using bootstrap as described

@agramfort

agramfort Mar 30, 2017

Member

computed using bootstrap as described

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
The procedure is then repeated for each frequency band of interest and
all GFPs are visualized. The non-parametric confidence intervals are computed
as described in [3].
The advantage of this method over summarizing the the Space x Time x Frequency

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

the the

@agramfort

agramfort Mar 30, 2017

Member

the the

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
# set epoching parameters
event_id, tmin, tmax = 1, -1., 3.
baseline = None
frequency_map = list()

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

insert empty line

@agramfort

agramfort Mar 30, 2017

Member

insert empty line

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
baseline = None
frequency_map = list()
for band, fmin, fmax in iter_freqs:

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

remove line

@agramfort

agramfort Mar 30, 2017

Member

remove line

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
frequency_map = list()
for band, fmin, fmax in iter_freqs:
raw = mne.io.read_raw_fif(raw_fname, preload=True)

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

raw has been read above

@agramfort

agramfort Mar 30, 2017

Member

raw has been read above

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
# remove evoked response and get analytic signal (envelope)
data = epochs.get_data()
mean = np.mean(data, axis=0, keepdims=True)
epochs._data = np.abs(data - mean)

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

use epochs.subtract_evoked() method

@agramfort

agramfort Mar 30, 2017

Member

use epochs.subtract_evoked() method

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
rank = raw.estimate_rank()
print('The numerical rank is: %i' % rank)
rng = np.random.RandomState(42)
mne.utils.set_log_level('warning')

This comment has been minimized.

@agramfort

agramfort Mar 30, 2017

Member

don't put this in an example

@agramfort

agramfort Mar 30, 2017

Member

don't put this in an example

dengemann added some commits Mar 30, 2017

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Mar 30, 2017

Codecov Report

Merging #4139 into master will increase coverage by 0.27%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4139      +/-   ##
==========================================
+ Coverage   86.18%   86.46%   +0.27%     
==========================================
  Files         354      276      -78     
  Lines       63739    60597    -3142     
  Branches     9709     9597     -112     
==========================================
- Hits        54935    52395    -2540     
+ Misses       6128     5741     -387     
+ Partials     2676     2461     -215

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b53c5bd...74f92fa. Read the comment docs.

codecov-io commented Mar 30, 2017

Codecov Report

Merging #4139 into master will increase coverage by 0.27%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4139      +/-   ##
==========================================
+ Coverage   86.18%   86.46%   +0.27%     
==========================================
  Files         354      276      -78     
  Lines       63739    60597    -3142     
  Branches     9709     9597     -112     
==========================================
- Hits        54935    52395    -2540     
+ Misses       6128     5741     -387     
+ Partials     2676     2461     -215

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b53c5bd...74f92fa. Read the comment docs.

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
return ci_low, ci_up
# Now we can track the emergence of spatial patterns compared to baseline
# for each freqyenct band

This comment has been minimized.

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

frequency ... also spatial patterns?

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

frequency ... also spatial patterns?

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

what is the problem? GFP indicates variance across sensors.

@dengemann

dengemann Mar 31, 2017

Member

what is the problem? GFP indicates variance across sensors.

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

but thanks for pointing me to a typo :)

@dengemann

dengemann Mar 31, 2017

Member

but thanks for pointing me to a typo :)

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
###############################################################################
# Now we can compute the Global Field Power
# We first estimae the rank as this data is rank-reduced as SSS was applied.

This comment has been minimized.

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

estimate :D

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

estimate :D

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member

@jona-sassenhagen you want to run it on your laptop to confirm that it is relatively fast / runs on a weakly performant architecture (I developed this on a 12" MB)? Final verdict?

Member

dengemann commented Mar 31, 2017

@jona-sassenhagen you want to run it on your laptop to confirm that it is relatively fast / runs on a weakly performant architecture (I developed this on a 12" MB)? Final verdict?

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
fig, axes = plt.subplots(4, 1, figsize=(10, 7),
sharex=True, sharey=True)
colors = [plt.cm.viridis(ii) for ii in (0.1, 0.35, 0.75, 0.95)]

This comment has been minimized.

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

colors = plt.cm.viridis((0.1, 0.35, 0.75, 0.95))?

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

colors = plt.cm.viridis((0.1, 0.35, 0.75, 0.95))?

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

indeed ...

@dengemann

dengemann Mar 31, 2017

Member

indeed ...

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
gfps_bs = np.array(gfps_bs)
gfps_bs = mne.baseline.rescale(gfps_bs, average.times, baseline=(None, 0))
ci_low = np.percentile(gfps_bs, 2.5, axis=0)
ci_up = np.percentile(gfps_bs, 97.5, axis=0)

This comment has been minimized.

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

Can't you do ci = np.percentile(gfps_bs, (2.5, 97.5), axis=0)
and then later

ax.fill_between(times, *(gfp + ci), color=color, alpha=0.3)
or something like that

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

Can't you do ci = np.percentile(gfps_bs, (2.5, 97.5), axis=0)
and then later

ax.fill_between(times, *(gfp + ci), color=color, alpha=0.3)
or something like that

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

maybe. But the code I present here is at least more explicit. Let me at least avoid two calls to np.percentile.

@dengemann

dengemann Mar 31, 2017

Member

maybe. But the code I present here is at least more explicit. Let me at least avoid two calls to np.percentile.

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

I don't have Somato here and the wifi is very bad .... I ran it on our server and it took a while. It would be good to optimize it further, if at all possible.

Contributor

jona-sassenhagen commented Mar 31, 2017

I don't have Somato here and the wifi is very bad .... I ran it on our server and it took a while. It would be good to optimize it further, if at all possible.

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member

I ran it on our server and it took a while. It would be good to optimize it further, if at all possible.

It's difficult to believe this. It takes less then one minute on my 12" macbook (remember it's gold on top of it) that does not even have an intel i5 chip but something that's closer to an iPad ...

Member

dengemann commented Mar 31, 2017

I ran it on our server and it took a while. It would be good to optimize it further, if at all possible.

It's difficult to believe this. It takes less then one minute on my 12" macbook (remember it's gold on top of it) that does not even have an intel i5 chip but something that's closer to an iPad ...

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

Well 'a while' < 1 min :)

Contributor

jona-sassenhagen commented Mar 31, 2017

Well 'a while' < 1 min :)

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member

Well 'a while' < 1 min :)

Ok but this is then indeed ok. Filtering four time takes a moment. It can only get faster on more serious computers (no gold).

Member

dengemann commented Mar 31, 2017

Well 'a while' < 1 min :)

Ok but this is then indeed ok. Filtering four time takes a moment. It can only get faster on more serious computers (no gold).

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
gfp_bs = np.sum(average.data[bs_indices] ** 2, 0) / rank
gfps_bs.append(gfp_bs)
gfps_bs = np.array(gfps_bs)
gfps_bs = mne.baseline.rescale(gfps_bs, average.times, baseline=(None, 0))

This comment has been minimized.

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor
def get_gfp_ci(average, rank=rank, n_draws=2000):
    """get confidence intervals from non-parametric bootstrap"""
    indices = np.arange(len(average.ch_names), dtype=int)
    gfps_bs = np.zeros((n_draws, average.data.shape[1]))
    for iter_ in range(n_draws):
        bs_indices = rng.choice(indices, replace=True, size=len(indices))
        gfps_bs[iter_, :] = (average.data[bs_indices] ** 2).sum(0) / rank
    gfps_bs = mne.baseline.rescale(gfps_bs, average.times, baseline=(None, 0))
    ci_low, ci_up = np.percentile(gfps_bs, (2.5, 97.5), axis=0)
    return ci_low, ci_up

?

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor
def get_gfp_ci(average, rank=rank, n_draws=2000):
    """get confidence intervals from non-parametric bootstrap"""
    indices = np.arange(len(average.ch_names), dtype=int)
    gfps_bs = np.zeros((n_draws, average.data.shape[1]))
    for iter_ in range(n_draws):
        bs_indices = rng.choice(indices, replace=True, size=len(indices))
        gfps_bs[iter_, :] = (average.data[bs_indices] ** 2).sum(0) / rank
    gfps_bs = mne.baseline.rescale(gfps_bs, average.times, baseline=(None, 0))
    ci_low, ci_up = np.percentile(gfps_bs, (2.5, 97.5), axis=0)
    return ci_low, ci_up

?

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

let's be a bit softer here. This function in the middle of the example is already enough I feel ... making a list and then an array is a stable pattern in research code. For a module level version I'd prefer your code.

@dengemann

dengemann Mar 31, 2017

Member

let's be a bit softer here. This function in the middle of the example is already enough I feel ... making a list and then an array is a stable pattern in research code. For a module level version I'd prefer your code.

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

Ok, I tried to take into account your proposal.

@dengemann

dengemann Mar 31, 2017

Member

Ok, I tried to take into account your proposal.

This comment has been minimized.

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

I'll probably steal it to turn it into a private function anyways for plot_compare evokeds, so then I can golf it all I want :D

@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

I'll probably steal it to turn it into a private function anyways for plot_compare evokeds, so then I can golf it all I want :D

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member
Member

dengemann commented Mar 31, 2017

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

Maybe a better title would be "Explore event-related synchronisation/desynchronisation for multiple bands" so people who do that kind of stuff can easily find it?

Contributor

jona-sassenhagen commented Mar 31, 2017

Maybe a better title would be "Explore event-related synchronisation/desynchronisation for multiple bands" so people who do that kind of stuff can easily find it?

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member
Member

dengemann commented Mar 31, 2017

@dengemann dengemann changed the title from VIZ/DOC/ENH: add time frequency gfp example to [MRG] VIZ/DOC/ENH: add time frequency gfp example Mar 31, 2017

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member

@jona-sassenhagen good for you? @Eric89GXL wanna have a look?

Member

dengemann commented Mar 31, 2017

@jona-sassenhagen good for you? @Eric89GXL wanna have a look?

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
The objective is to show you how to explore spectrally localized
effects. For this purpose we adapt the method described in [1] and use it on
the somato dataset. The idea is to track the band-limited temporal evolution
of spatial patterns by using the Global Field Power (GFP).

This comment has been minimized.

@larsoner

larsoner Mar 31, 2017

Member

Need newline between paragraphs to render properly

@larsoner

larsoner Mar 31, 2017

Member

Need newline between paragraphs to render properly

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
=============================================
The objective is to show you how to explore spectrally localized
effects. For this purpose we adapt the method described in [1] and use it on

This comment has been minimized.

@larsoner

larsoner Mar 31, 2017

Member

[1]_

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
We first bandpass filter the signals and then apply a Hilbert transform. To
reveal oscillatory activity the evoked response is then subtracted from every
single trial. Finally, we rectify the signals prior to averaging across trials.
Then the GFP is computed as described in [2], using the

This comment has been minimized.

@larsoner

larsoner Mar 31, 2017

Member

[2]_

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
# bandpass filter and compute Hilbert
raw.filter(fmin, fmax, n_jobs=1) # use more jobs to speed up.
raw.apply_hilbert(n_jobs=1, envelope=False)

This comment has been minimized.

@larsoner

larsoner Mar 31, 2017

Member

why envelope=False but epochs._data = abs later? For baseline purposes?

@larsoner

larsoner Mar 31, 2017

Member

why envelope=False but epochs._data = abs later? For baseline purposes?

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

I thought it mattered for the subtraction of the evoked response to first subtract and then rectify.

@dengemann

dengemann Mar 31, 2017

Member

I thought it mattered for the subtraction of the evoked response to first subtract and then rectify.

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

to be able to subtract the evoked

@agramfort

agramfort Mar 31, 2017

Member

to be able to subtract the evoked

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
effects. For this purpose we adapt the method described in [1] and use it on
the somato dataset. The idea is to track the band-limited temporal evolution
of spatial patterns by using the Global Field Power (GFP).
We first bandpass filter the signals and then apply a Hilbert transform. To

This comment has been minimized.

@larsoner

larsoner Mar 31, 2017

Member

... and take the magnitude

@larsoner

larsoner Mar 31, 2017

Member

... and take the magnitude

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
frequencies.
The procedure is then repeated for each frequency band of interest and
all GFPs are visualized. To estimate uncertainty, non-parametric confidence
intervals are computed as described in [3] across channels.

This comment has been minimized.

@larsoner

larsoner Mar 31, 2017

Member

[3]_

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
raw.pick_types(meg='grad', eog=True) # we just look at gradiometers
# bandpass filter and compute Hilbert
raw.filter(fmin, fmax, n_jobs=1) # use more jobs to speed up.

This comment has been minimized.

@larsoner

larsoner Mar 31, 2017

Member

these filters will have different transition bandwidths, because l_trans_bandwidth='auto' mode makes the bandwidth depend on the freq. To make them more comparable, set all these to the same value (maybe 1?)

@larsoner

larsoner Mar 31, 2017

Member

these filters will have different transition bandwidths, because l_trans_bandwidth='auto' mode makes the bandwidth depend on the freq. To make them more comparable, set all these to the same value (maybe 1?)

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

good call, checked it out and it looks good.

@dengemann

dengemann Mar 31, 2017

Member

good call, checked it out and it looks good.

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
sharex=True, sharey=True)
colors = plt.cm.viridis((0.1, 0.35, 0.75, 0.95))
for ((freq_name, fmin, fmax), average), color, ax in zip(
frequency_map, colors, reversed(axes.ravel())):

This comment has been minimized.

@larsoner

larsoner Mar 31, 2017

Member

why not just axes[::-1, 0]?

@larsoner

larsoner Mar 31, 2017

Member

why not just axes[::-1, 0]?

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

reversed is more explicit.

@dengemann

dengemann Mar 31, 2017

Member

reversed is more explicit.

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
reject=dict(grad=4000e-13, eog=350e-6), preload=True)
# remove evoked response and get analytic signal (envelope)
epochs.subtract_evoked()
epochs._data = np.abs(epochs.get_data())

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

hacking a private attribute is problematic :(

@agramfort

agramfort Mar 31, 2017

Member

hacking a private attribute is problematic :(

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
for iteration in range(n_bootstraps):
bs_indices = rng.choice(indices, replace=True, size=len(indices))
gfps_bs[iteration] = np.sum(average.data[bs_indices] ** 2, 0) / rank
gfps_bs = np.array(gfps_bs)

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

gfps_bs is already an array

@agramfort

agramfort Mar 31, 2017

Member

gfps_bs is already an array

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
def get_gfp_ci(average, rank=rank, n_bootstraps=2000):
"""get confidence intervals from non-parametric bootstrap"""
indices = np.arange(len(average.ch_names), dtype=int)
gfps_bs = np.zeros((n_bootstraps, len(average.times)))

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

np.empty

@agramfort

agramfort Mar 31, 2017

Member

np.empty

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
# Now we can track the emergence of spatial patterns compared to baseline
# for each frequency band

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

too many blank lines

@agramfort

agramfort Mar 31, 2017

Member

too many blank lines

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
axes.ravel()[-1].set_xlabel('Time [ms]')
# We see dominant responses in the Alpha and Beta bands.

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

I would put this in a header of the last cell

@agramfort

agramfort Mar 31, 2017

Member

I would put this in a header of the last cell

dengemann added some commits Mar 31, 2017

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member

I think all comments are addressed or clarified.

Member

dengemann commented Mar 31, 2017

I think all comments are addressed or clarified.

@larsoner

This comment has been minimized.

Show comment
Hide comment
@larsoner

larsoner Mar 31, 2017

Member

Several of my comments are unaddressed

Member

larsoner commented Mar 31, 2017

Several of my comments are unaddressed

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
# Now we can compute the Global Field Power
# We first estimate the rank as this data is rank-reduced as SSS was applied.
# Therefore the degrees of freedom are less then the number of sensors.

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

there is no rank anymore

@agramfort

agramfort Mar 31, 2017

Member

there is no rank anymore

single trial. Finally, we rectify the signals prior to averaging across trials
by taking the magniude of the Hilbert.
Then the GFP is computed as described in [2]_, using the sum of the squares
but without normalization by the rank.

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

no rank anymore

@agramfort

agramfort Mar 31, 2017

Member

no rank anymore

This comment has been minimized.

@dengemann

dengemann Mar 31, 2017

Member

yes this is what I write here :) see "without"

@dengemann

dengemann Mar 31, 2017

Member

yes this is what I write here :) see "without"

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
for ((freq_name, fmin, fmax), average), color, ax in zip(
frequency_map, colors, reversed(axes.ravel())):
times = average.times * 1e3
gfp = np.sum(average.data ** 2, 0)

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

gfp = np.sum(average.data ** 2, axis=0)

@agramfort

agramfort Mar 31, 2017

Member

gfp = np.sum(average.data ** 2, axis=0)

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
fig, axes = plt.subplots(4, 1, figsize=(10, 7), sharex=True, sharey=True)
colors = plt.cm.viridis((0.1, 0.35, 0.75, 0.95))
for ((freq_name, fmin, fmax), average), color, ax in zip(
frequency_map, colors, reversed(axes.ravel())):

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

axes.ravel()[::-1]

would keep it an array

@agramfort

agramfort Mar 31, 2017

Member

axes.ravel()[::-1]

would keep it an array

Show outdated Hide outdated examples/time_frequency/plot_time_frequency_global_field_power.py
###############################################################################
# Now we can compute the Global Field Power

This comment has been minimized.

@agramfort

agramfort Mar 31, 2017

Member

I would make it a full block of text and don't have the inline comments that won't pop out

@agramfort

agramfort Mar 31, 2017

Member

I would make it a full block of text and don't have the inline comments that won't pop out

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

Since the gfp_bs = np.array(gfp_bs) line is gone, I'm happy :)

Would prefer to have a title that advertises this to ERD/ERS people though.

Contributor

jona-sassenhagen commented Mar 31, 2017

Since the gfp_bs = np.array(gfp_bs) line is gone, I'm happy :)

Would prefer to have a title that advertises this to ERD/ERS people though.

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member
Member

dengemann commented Mar 31, 2017

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

Well what'd be most user-friendly? How do you actually reach people?

I think ERD/ERS is pretty common, and that's what people who need this code are probably looking for. (Not specifically these words - but something that's more descriptive of what's actually going on wrt. analysing experimental data, e.g. 'Event-Related Spectral Perturbations' as Makeig et al. would say.

Contributor

jona-sassenhagen commented Mar 31, 2017

Well what'd be most user-friendly? How do you actually reach people?

I think ERD/ERS is pretty common, and that's what people who need this code are probably looking for. (Not specifically these words - but something that's more descriptive of what's actually going on wrt. analysing experimental data, e.g. 'Event-Related Spectral Perturbations' as Makeig et al. would say.

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

I mean, if you want to reach different people, that's a different thing.

Contributor

jona-sassenhagen commented Mar 31, 2017

I mean, if you want to reach different people, that's a different thing.

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member

I mean, if you want to reach different people, that's a different thing.

I would veto this. ERD/ERS is too local. Only certain people call it like this and this goes to far.

Member

dengemann commented Mar 31, 2017

I mean, if you want to reach different people, that's a different thing.

I would veto this. ERD/ERS is too local. Only certain people call it like this and this goes to far.

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

I don't mean those specific words - just something that provides the required information. Explore oscillatory activity in sensor space doesn't really say much about what's specific about this - that it's 1. a smart way of getting specific bands, and 2. about their event-related dynamics.

Contributor

jona-sassenhagen commented Mar 31, 2017

I don't mean those specific words - just something that provides the required information. Explore oscillatory activity in sensor space doesn't really say much about what's specific about this - that it's 1. a smart way of getting specific bands, and 2. about their event-related dynamics.

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member

@jona-sassenhagen propse a name that is informative but does not explicitly buy into certain theoretical camps

Member

dengemann commented Mar 31, 2017

@jona-sassenhagen propse a name that is informative but does not explicitly buy into certain theoretical camps

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

How about "Explore event-related dynamics for specific frequency bands [in sensor space]"?
Or "Unbiased isolation of frequency band specific time courses" ... something like this.

I mean, the current title isn't bad, I just think it could sell the thing to more people :)

Contributor

jona-sassenhagen commented Mar 31, 2017

How about "Explore event-related dynamics for specific frequency bands [in sensor space]"?
Or "Unbiased isolation of frequency band specific time courses" ... something like this.

I mean, the current title isn't bad, I just think it could sell the thing to more people :)

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Mar 31, 2017

Member

I mean, the current title isn't bad, I just think it could sell the thing to more people :)

ok I'm totally for this :)

"Explore event-related dynamics for specific frequency bands ..." sounds good.

Member

dengemann commented Mar 31, 2017

I mean, the current title isn't bad, I just think it could sell the thing to more people :)

ok I'm totally for this :)

"Explore event-related dynamics for specific frequency bands ..." sounds good.

@jona-sassenhagen

This comment has been minimized.

Show comment
Hide comment
@jona-sassenhagen

jona-sassenhagen Mar 31, 2017

Contributor

I actually really prefer this approach to the currently much more popular broad-band ERSP approach. As Lopes Da Silva and Pfurtscheller write:

"the term ERD is only meaningful if the baseline measured some seconds before the event represents a rhythmicity seen as a clear peak in the power spectrum. Similarly, the term ERS only has a meaning if the event results in the appearance of a rhythmic component and therewith in a spectral peak that was initially not detect- able (Lopes da Silva and Pfurtscheller, 1999)."

I guess people just love 2D arrays in Jet too much though.

Contributor

jona-sassenhagen commented Mar 31, 2017

I actually really prefer this approach to the currently much more popular broad-band ERSP approach. As Lopes Da Silva and Pfurtscheller write:

"the term ERD is only meaningful if the baseline measured some seconds before the event represents a rhythmicity seen as a clear peak in the power spectrum. Similarly, the term ERS only has a meaning if the event results in the appearance of a rhythmic component and therewith in a spectral peak that was initially not detect- able (Lopes da Silva and Pfurtscheller, 1999)."

I guess people just love 2D arrays in Jet too much though.

@dengemann

This comment has been minimized.

Show comment
Hide comment
@dengemann

dengemann Apr 1, 2017

Member

Does anyone wants to have another look before merging? It seems comments by 3 reviewers are addressed.

Member

dengemann commented Apr 1, 2017

Does anyone wants to have another look before merging? It seems comments by 3 reviewers are addressed.

@agramfort agramfort merged commit b6bb7bf into mne-tools:master Apr 1, 2017

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@larsoner larsoner added mne sprint and removed mne sprint labels Apr 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment