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

[MRG][DOC] Add GFP to the glossary #5804

Merged
merged 3 commits into from
May 29, 2019
Merged

Conversation

massich
Copy link
Contributor

@massich massich commented Dec 18, 2018

This PR continues #5796 by adding an entry to the glossary and pointing at it.

@jona-sassenhagen can you (or anybody else) edit the glossary entry with
meaningful info? Thx. I think it would be good also to point to back to whatever
tuto/example explains better GFP.
(maybe examples/time_frequency/plot_time_frequency_global_field_power.py?)

@codecov
Copy link

codecov bot commented May 27, 2019

Codecov Report

Merging #5804 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5804      +/-   ##
==========================================
+ Coverage   89.32%   89.33%   +<.01%     
==========================================
  Files         408      408              
  Lines       74303    74303              
  Branches    12293    12293              
==========================================
+ Hits        66374    66376       +2     
+ Misses       5072     5070       -2     
  Partials     2857     2857

@massich
Copy link
Contributor Author

massich commented May 27, 2019

@drammock can you take over this one? thx

cross refere: #6321

@drammock drammock mentioned this pull request May 27, 2019
11 tasks
@cbrnr cbrnr changed the title [WIP][DOC] Add GDF to the glossary [WIP][DOC] Add GFP to the glossary May 27, 2019
@drammock
Copy link
Member

drammock commented May 29, 2019

In order to be write an honest glossary entry for GFP, we would need to do it the same way everywhere. Sadly that is not happening:

(The first two methods are equivalent, but the last one is not)

@drammock
Copy link
Member

rendered glossary

@agramfort agramfort changed the title [WIP][DOC] Add GFP to the glossary [MRG][DOC] Add GFP to the glossary May 29, 2019
@larsoner
Copy link
Member

Perhaps some of the confusion arises because for EEG data with a common average reference the RMS power at each time point across sensors is the same as the standard deviation (but without CAR it's not due to the mean subtraction step). I'm actually not sure which we should use everywhere, probably the std. But in any case, this looks good, thanks @drammock !

@larsoner larsoner merged commit 1020921 into mne-tools:master May 29, 2019
@drammock
Copy link
Member

@larsoner it strikes me as low priority, but to improve consistency (and ensure future additions remain consistent) WDYT about adding a GFP function that triages based on instance type (and maybe incorporates some of the baselining functionality that @dengemann is using in that time-freq-GFP example?)

@larsoner
Copy link
Member

Sure, a private function would be nice. It might not even need to triage based on type, it's entirely possible the sum should be mean in the TF code. But someone would need to check (and also check that you should subtract the mean at each time point, maybe you don't and just rely on the baselining or high-passing instead...)

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.

4 participants