Bug in TAxis::FindBin?

[DanielCussol]

Hello all,
It seems there is a bug in the TAxis::FindBin (Double_t x) method. If one runs the following script,

#include "TAxis.h"
#include "Riostream.h"
#include "TROOT.h"

Int_t GetRightBin (TAxis *ax, Double_t x)
{
//
//  Get correct bin number from x value for TAxis ax
//   
Int_t retVal = 1+Int_t(ax->GetNbins()*(x-ax->GetXmin())/(ax->GetXmax()-ax->GetXmin()) + 0.5);
return retVal;
}

void TAxisBug (Int_t nBins = 30, Double_t xMin=3.0, Double_t xMax=6.0)
{
//
// Test TAxis bug 
// 
TAxis *ax=new TAxis (nBins, xMin, xMax);
cout << "Root version : " << gROOT->GetVersion() << endl; 
for (Int_t i=1;i<=ax->GetNbins();i++)
 {
 cout << i << " => " << ax->FindBin (ax->GetBinLowEdge (i)) 
       << " : " << 
       GetRightBin (ax, ax->GetBinLowEdge (i)); 
 if (i != ax->FindBin (ax->GetBinLowEdge (i)))
  {
  cout << " <= Bug!!! ";
  }
 cout << endl;
 }
cout << endl;
}

one can see that the bin number returned from ax->FindBin (ax->GetBinLowEdge (i)) is not always i as expected.

When looking at the code of TAxis::FindBin (ROOT: hist/hist/src/TAxis.cxx Source File line 314), the formula used to compute the bin number is subject to rounding errors. It should be corrected as propose in the joint script.

Daniel CUSSOL

[couet]

I modified a bit your macro:

int GetRightBin(TAxis *ax, Double_t x) {
   int retVal = 1+int(ax->GetNbins()*(x-ax->GetXmin())/(ax->GetXmax()-ax->GetXmin()) + 0.5);
   return retVal;
}

void TAxisBug (int nBins = 30, Double_t xMin=3.0, Double_t xMax=6.0)
{
   //
   // Test TAxis bug
   //
   TAxis *ax = new TAxis (nBins, xMin, xMax);
   cout << "Root version : " << gROOT->GetVersion() << endl;
   cout << "+----+------+--------+----++------+--------+----+" << endl;
   cout << "| Id | Low  | Center | Up || Low  | Center | Up |" << endl;
   cout << "+----+------+--------+----++------+--------+----+" << endl;
   cout << "|    |      FindBin       ||   GetRightBin      |" << endl;
   cout << "+----+------+--------+----++------+--------+----+" << endl;
   for (int i=1;i<=8;i++) {
      cout << "| " << i;
      if (i == ax->FindBin (ax->GetBinLowEdge (i))) cout << "  |  " << ax->FindBin (ax->GetBinLowEdge (i));
      else                                          cout << "  |  \033[1;31m" << ax->FindBin (ax->GetBinLowEdge (i)) << "\033[0m";
      if (i == ax->FindBin (ax->GetBinCenter (i)))  cout << "   |   " << ax->FindBin (ax->GetBinCenter (i));
      else                                          cout << "   |   \033[1;31m" << ax->FindBin (ax->GetBinCenter (i)) << "\033[0m";
      if (i == ax->FindBin (ax->GetBinUpEdge (i)))  cout << "    | " << ax->FindBin (ax->GetBinUpEdge (i));
      else                                          cout << "    | \033[1;31m" << ax->FindBin (ax->GetBinUpEdge (i)) << "\033[0m";

      if (i == GetRightBin(ax,ax->GetBinLowEdge (i))) cout << "  ||  " << GetRightBin(ax,ax->GetBinLowEdge (i));
      else                                            cout << "  ||  \033[1;31m" << GetRightBin(ax,ax->GetBinLowEdge (i)) << "\033[0m";
      if (i == GetRightBin(ax,ax->GetBinCenter (i)))  cout << "   |   " << GetRightBin(ax,ax->GetBinCenter (i));
      else                                            cout << "   |   \033[1;31m" << GetRightBin(ax,ax->GetBinCenter (i)) << "\033[0m";
      if (i == GetRightBin(ax,ax->GetBinUpEdge (i)))  cout << "    | " << GetRightBin(ax,ax->GetBinUpEdge (i));
      else                                            cout << "    | \033[1;31m" << GetRightBin(ax,ax->GetBinUpEdge (i)) << "\033[0m";
      cout << "  |" << endl;
    }
   cout << "+----+------+--------+----++------+--------+----+" << endl;
}

it gives:

As you can see your version (GetRightBin) has problem with the bin center.

[moneta]

Hi,

This is not really a bug but a limitation due to numerical error when calling FindBin with the bin edge values. Your code will fixes this case but it will return a wrong result when calling FindBin( ax->GetBinLowEdge (i)) - eps) where eps is a value smaller than half of the bin width.
We could maybe provide a better handling of the bin edges cases to avoid this numerical error, but this will slow down the FindBin function which sometimes is used in some critical code. In general , whenever possible, it is always recommended to avoid filling an histogram with bin edge values

Lorenzo

[Wile_E_Coyote]

I recall two similar bug reports:

1. TGraphSmooth::Approx bug in ROOT 5 and 6

2. TAxis::SetRangeUser versus TH1::SetAxisRange

[DanielCussol]

Thanks a lot for your answer.

Daniel

[moneta]

I have added a PR, improving the bin edge cases in Taxis::FindBin, see

[hist] Improve calculation of `TAxis::FindBin(x)` when x is at the bin edges

Now axis.FindBin(axis.GetBinLowEdge(i) should return always i and axis.FindBin(axis.GetBinUpEdge(i)) returns i+1.

It is clear that if you compute the bin edges with a different function the return bin value will be sometimes different due to numerical error

Lorenzo

[DanielCussol]

Thanks a lot!