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

TGSpeedo mean value calculation #7164

Closed
ferdymercury opened this issue Feb 9, 2021 · 2 comments · Fixed by #7170
Closed

TGSpeedo mean value calculation #7164

ferdymercury opened this issue Feb 9, 2021 · 2 comments · Fixed by #7170
Assignees

Comments

@ferdymercury
Copy link
Collaborator

Explain what you would like to see improved

TGSpeedo class is quite helpful, but it lacks the ability of auto-calculating the mean value (a mean mark is there, but is up to the user to put a senseful value there on each update).

Optional: share how it could be improved

I would propose to let the user define a circular buffer size, so that the TGSpeedo class autocalculates the mean value of the last N entries. The default value would be N=0 to keep old behaviour and save memory, but the user could call for example SetMeanBufferSize(10) to do an average of the last 10 entries.

Could be implemented for example using https://root.cern/doc/master/circular_8C.html
or just with this simple header class: https://github.com/jnk0le/Ring-Buffer/blob/master/ringbuffer.hpp

And then the average just as std::sum(begin(...), end(...), 0)/N

To Reproduce

  1. EnableMeanMark in TGSpeedo
  2. The Mean value is not calculated automatically

Setup

  1. ROOT 6.23/01
  2. Linux
  3. Self-built
@github-actions github-actions bot added this to Needs triage in Triage Feb 9, 2021
@eguiraud eguiraud removed this from Needs triage in Triage Feb 10, 2021
bellenot added a commit to bellenot/root that referenced this issue Feb 10, 2021
…ct#7164)

Add a very simple circular buffer allowing to automatically compute and display the mean of last N values when enabling the mean mark with `EnableMeanMark()`. The size of the buffer can be changed with `SetBufferSize()`. Thanks to @ferdymercury for the suggestion
@bellenot
Copy link
Member

bellenot commented Feb 10, 2021

@ferdymercury FYI, a Pull Request has been created

ferdymercury added a commit to ferdymercury/root that referenced this issue Feb 10, 2021
bellenot added a commit that referenced this issue Feb 15, 2021
)

* Add a circular buffer to compute the mean value (fix issue #7164)

Add a very simple circular buffer allowing to automatically compute and display the mean of last N values when enabling the mean mark with `EnableMeanMark()`. The size of the buffer can be changed with `SetBufferSize()`. Thanks to @ferdymercury for the suggestion

* Fix typo (thanks @ferdymercury)

* Preserve backward compatibility (thanks @ferdymercury)

* Better (more precise) mean calculation

* properly prevent int overflows

* Fix warning: comparison of integer expressions of different signedness

* Apply changes from @ferdymercury

* Update gui/gui/src/TGSpeedo.cxx

Co-authored-by: Enrico Guiraud <enrico.guiraud@cern.ch>

* Address some of the @eguiraud comments

- Add some explanation about how to use the circular buffer and how it works
- Protect against negative argument given for the buffer size
- Remove an unecessary cast to float

Co-authored-by: Enrico Guiraud <enrico.guiraud@cern.ch>
@bellenot
Copy link
Member

Fixed by #7170

@bellenot bellenot added this to Issues in Fixed in 6.24/00 via automation Feb 15, 2021
nicknagi pushed a commit to nicknagi/root that referenced this issue Mar 30, 2021
…ct#7164) (root-project#7170)

* Add a circular buffer to compute the mean value (fix issue root-project#7164)

Add a very simple circular buffer allowing to automatically compute and display the mean of last N values when enabling the mean mark with `EnableMeanMark()`. The size of the buffer can be changed with `SetBufferSize()`. Thanks to @ferdymercury for the suggestion

* Fix typo (thanks @ferdymercury)

* Preserve backward compatibility (thanks @ferdymercury)

* Better (more precise) mean calculation

* properly prevent int overflows

* Fix warning: comparison of integer expressions of different signedness

* Apply changes from @ferdymercury

* Update gui/gui/src/TGSpeedo.cxx

Co-authored-by: Enrico Guiraud <enrico.guiraud@cern.ch>

* Address some of the @eguiraud comments

- Add some explanation about how to use the circular buffer and how it works
- Protect against negative argument given for the buffer size
- Remove an unecessary cast to float

Co-authored-by: Enrico Guiraud <enrico.guiraud@cern.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants