[hist] Implement initial RHistAutoAxisFiller
#20180
Conversation
I missed this example in commit 943ee82 ("Accept regular axis interval as std::pair").
To fill a regular one-dimensional histogram without specifying an axis interval during construction.
hahnjo
changed the title
RHistAutoAxisFiller
Test Results 20 files 20 suites 3d 17h 31m 28s ⏱️ Results for commit 588e8fb. |
| if (x < fMinimum) { | ||
| fMinimum = x; | ||
| } | ||
| if (x > fMaximum) { | ||
| fMaximum = x; | ||
| } |
There was a problem hiding this comment.
| if (x < fMinimum) { | |
| fMinimum = x; | |
| } | |
| if (x > fMaximum) { | |
| fMaximum = x; | |
| } | |
| fMinimum = std::min(x, fMinimum); | |
| fMaximum = std::max(x, fMaximum); |
| /// \see Flush() | ||
| RHist<BinContentType> &GetHist() | ||
| { | ||
| Flush(); |
There was a problem hiding this comment.
That means that we can't get an empty histogram from the auto filler. Is this the desired behavior?
| /// | ||
| /// 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.
Do we want/need to expose this publicly?
| EXPECT_EQ(hist.GetBinContent(RBinIndex::Underflow()), 0); | ||
| EXPECT_EQ(hist.GetBinContent(RBinIndex::Overflow()), 0); | ||
| } | ||
|
|
There was a problem hiding this comment.
Thanks!
For completeness, I would propose copy-pasting the CTest "TTreeDraw_AutoBinning" from https://github.com/root-project/root/pull/19605/files into this class
Ie filling -999 and 0 and checking that auto-axis does not put them in the overflow or underflow.
There was a problem hiding this comment.
I briefly looked at the test case, which additional issue does it cover? In other words, what is special about -999? Note that the test for Fill above also already checks that all values landed in the normal bins. I'm against adding more tests that don't increase coverage just for completeness.
There was a problem hiding this comment.
In other words, what is special about
-999?
In a case where the user filled with 0 and -999, the automatic range calculation was sometimes missing the 0. (putting it in the overflow rather than in the last bin), due to some floating precision effects if I recall correctly.
https://root-forum.cern.ch/t/bug-or-feature-in-ttree-draw/62862
(Checking 0 and -1 might not trigger the issue since the relative error is smaller)
3 participants
To fill a regular one-dimensional histogram without specifying an axis interval during construction.