-
Couldn't load subscription status.
- Fork 1.4k
[hist] Implement initial RHistAutoAxisFiller
#20180
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
base: master
Are you sure you want to change the base?
Conversation
I missed this example in commit 943ee82 ("Accept regular axis interval as std::pair").
RHistAutoAxisFiller
Test Results 22 files 22 suites 3d 19h 18m 39s ⏱️ Results for commit 8bb1c23. ♻️ This comment has been updated with latest results. |
| /// \see Flush() | ||
| RHist<BinContentType> &GetHist() | ||
| { | ||
| Flush(); |
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.
That means that we can't get an empty histogram from the auto filler. Is this the desired behavior?
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.
Yes correct. It's a good question what we want to happen in this case. If we want to allow an empty histogram, what would be the axis interval and what would we want to happen in case of further fills? Would we modify the constructed histogram (potentially) behind the users' back? If there are many fills, how would the user tell is that now is the time to flush the buffer, while they are already looking at the histogram?
I think my proposal for the moment would be to disallow and see if it poses problems that we find a good compromise how to address.
| /// | ||
| /// Throws an exception if the buffer is empty, the axis interval cannot be determined, or if it would be empty | ||
| /// because the minimum equals the maximum. | ||
| void Flush() |
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.
Do we want/need to expose this publicly?
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.
Another good question. A potential use case would be where the user designates a limited set of "sample" entries that are potentially not aligned with fMaxBufferSize, or multiple RHistAutoAxisFiller with different fill frequencies that the user wants to "synchronize".
To fill a regular one-dimensional histogram without specifying an axis interval during construction.
To fill a regular one-dimensional histogram without specifying an axis interval during construction.