Skip to content

Commit f8fd1b4

Browse files
committed
[hist] Improve precision of TAxis::FindFixBin / FindBin.
Due to the floating-point subtraction x - xMin, the bin index occasionally flows over to the next bin. This is particularly annoying when it goes into the overflow, although the coordinate is strictly smaller than the max of the axis. By adding a correction term that can detect this case, overflows such as the one discussed in https://root-forum.cern.ch/t/bug-or-feature-in-ttree-draw/62862 can be avoided. An error in the other direction is possible, too, and fixed in this commit: https://root-forum.cern.ch/t/floating-point-rounding-error-when-filling-the-histogram/35189 Microbenchmarking the changed lines showed them to be the same speed in gcc, and 40% slower in clang. Both changes are by far outweighted by the virtual call overhead, though. This allowed for removing the cautionary note on rounding errors, added in 1703c54, which is fixed now. Fix #14091.
1 parent b4e8cce commit f8fd1b4

File tree

2 files changed

+7
-6
lines changed

2 files changed

+7
-6
lines changed

hist/hist/src/TAxis.cxx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,6 @@ void TAxis::ExecuteEvent(Int_t event, Int_t px, Int_t py)
289289
/// whereas the overflow bin (`nbins+1`) is for any `x` greater or equal than `fXmax`,
290290
/// as well as for `NaN`. The first regular bin (`1`) is for any `x`
291291
/// greater or equal than `fXmin` and strictly smaller than `fXmin + binwidth`, and so on.
292-
/// @note The bin assignment equation uses doubles, thus rounding errors are
293-
/// expected to appear at the edges. For example: `TAxis(1, -1., 0.).FindBin(-1e-17)`
294-
/// returns the overflow bin (`2`) rather than the theoretically correct bin (`1`).
295292

296293
Int_t TAxis::FindBin(Double_t x)
297294
{
@@ -314,7 +311,9 @@ Int_t TAxis::FindBin(Double_t x)
314311
return FindFixBin(x);
315312
} else {
316313
if (!fXbins.fN) { //*-* fix bins
317-
bin = 1 + int (fNbins*(x-fXmin)/(fXmax-fXmin) );
314+
const double width = (fXmax-fXmin)/fNbins;
315+
const int approxBin = (x-fXmin) / width; // Might be one unit too small/large due to limited precision of subtraction
316+
bin = 1 + approxBin - (x < fXmin + width*approxBin) + (fXmin + width*(approxBin+1) <= x);
318317
} else { //*-* variable bin sizes
319318
//for (bin =1; x >= fXbins.fArray[bin]; bin++);
320319
bin = 1 + TMath::BinarySearch(fXbins.fN,fXbins.fArray,x);
@@ -429,7 +428,9 @@ Int_t TAxis::FindFixBin(Double_t x) const
429428
bin = fNbins+1;
430429
} else {
431430
if (!fXbins.fN) { //*-* fix bins
432-
bin = 1 + int (fNbins*(x-fXmin)/(fXmax-fXmin) );
431+
const double width = (fXmax-fXmin)/fNbins;
432+
const int approxBin = (x-fXmin) / width; // Might be one unit too small/large due to limited precision of subtraction
433+
bin = 1 + approxBin - (x < fXmin + width*approxBin) + (fXmin + width*(approxBin+1) <= x);
433434
} else { //*-* variable bin sizes
434435
bin = 1 + TMath::BinarySearch(fXbins.fN,fXbins.fArray,x);
435436
}

hist/hist/test/test_TH1.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
#include "TH1F.h"
77
#include "THLimitsFinder.h"
88

9-
9+
#include <random>
1010
#include <vector>
1111

1212
// StatOverflows TH1

0 commit comments

Comments
 (0)