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

LFPViewer - fix mean+std calculation #616

Merged
merged 6 commits into from
Jul 19, 2024

Conversation

florian6973
Copy link

Regarding this issue #615, here is a small fix of the mean and std calculation, before a bigger refactoring of the LFP viewer.

@anjaldoshi
Copy link
Member

Hello @florian6973,

We appreciate your contribution. Upon reviewing your changes, I found that the Mean and Std Dev values are now accurately calculated using the data from the display buffer instead of the screen buffer. However, we noticed an issue when the “subtract offset” option is enabled. The signal plotted seems a bit off. This is due to the LfpDisplaySplitter::getMean function being used not only to calculate and display the channel-specific mean and std values, but also to adjust the signal’s Y coordinate’s min and max values for offset correction (you can refer to the code here). As the offset correction is performed when plotting the signal using the screen buffer, the mean value used there is incorrect because it’s derived from the display buffer with your modifications.

To rectify this, I suggest updating your current PR. The optimal solution would be to have two distinct methods for calculating the mean: one for drawing/plotting that utilizes the screen buffer, and another for calculating the standard deviation that uses the display buffer and will be displayed in the single channel view. This way, we can ensure both the mean and std values are accurately calculated and the offset correction functions properly.

Could you implement these changes and update the PR? If you’re unable to do so, please let us know and we’ll handle it. Thanks!

@anjaldoshi anjaldoshi self-requested a review July 11, 2024 00:34
@florian6973
Copy link
Author

Hi!

Thank you very much for your detailed review! I did not check indeed "subtract offset"... I updated the PR accordingly.

Please let me know if there are any other fixes to add :)

Copy link
Member

@anjaldoshi anjaldoshi left a comment

Choose a reason for hiding this comment

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

Thanks for making the requested changes. Looks great! We'll let you know when a new release is out with these changes.

@anjaldoshi anjaldoshi merged commit f1577fa into open-ephys:development Jul 19, 2024
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.

2 participants