-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
MRG: "GFP" trace in STC time viewer is actually RMS #8965
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
Conversation
b8e3513 to
ab2afcf
Compare
ab2afcf to
c56048e
Compare
I've also rearranged the formula such that it becomes immediately obvious that we're dealing with RMS values. See mne-tools#8775 for our lengthy discussion on this for sensor-level data.
c56048e to
300bc18
Compare
agramfort
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides lgtm
|
Well maybe not but it’s bad practice I would say. It advertised not optimal code for no reason
|
|
@agramfort Ok I've addressed this, also in |
agramfort
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx @hoechenberger 👍
|
FYI this had CI failures beyond just pip and pip pre, fixed by #8971 |
|
Thanks for taking care of this, @larsoner |
I've also rearranged the respective formula such that it becomes immediately obvious that we're dealing with RMS values.
See #8775 for our lengthy discussion on GFP vs RMS naming, albeit for sensor-level data.