Unpredictable errors in TH3D constructor with arrays

ROOT
1

Hi folks,
I made up the following snippet of code to illustrate the problem I’m facing.

template<typename T>
const std::shared_ptr<T> MakeBinsArray(const int nbins, T start, T end){
  const std::shared_ptr<T> array( new T[nbins+1], std::default_delete<T[]>() );

  array.get()[0] = start;
  array.get()[nbins] = end;

  for(int i=1; i<nbins; i++){ array.get()[i] = start + i*(array.get()[nbins] - array.get()[0])/nbins; }

  return array;
};

const int nkbins = 30;
const double kbins[nkbins+1] = {
  0.020,0.025,0.032,0.040,0.050,0.063,0.080,0.100,0.126,0.159,
  0.200,0.252,0.317,0.399,0.502,0.632,0.796,1.002,1.262,1.589,
  2.000,2.518,3.170,3.991,5.024,6.325,7.962,10.024,12.619,15.887,
  20.000
};

void test_bug(){

  cout << "1" << endl;
  const int nselbins = 10;
  auto selbins = MakeBinsArray(nselbins, (Double_t) 0, (Double_t) nselbins).get();

  cout << "2" << endl;
  const int nmassbins = 500;
  auto massbins = MakeBinsArray(nmassbins, (Double_t) -5, (Double_t) 5).get();

  //Mass histos
  cout << "3" << endl;
  TH3D* h1 = new TH3D("hMass_Tof", ";M (GeV);Counts", nmassbins, massbins, nkbins, kbins, nselbins, selbins);
  cout << "4" << endl;
  TH3D* h2 = new TH3D("hMass_NaF", ";M (GeV);Counts", nmassbins, massbins, nkbins, kbins, nselbins, selbins);
  cout << "5" << endl;
  TH3D* h3 = new TH3D("hMass_Agl", ";M (GeV);Counts", nmassbins, massbins, nkbins, kbins, nselbins, selbins);
  cout << "6" << endl;

}

If I run it more than once I see every time a different number of errors coming from the TH3D constructors

Run1:

Processing test_bug.C...
1
2
3
4
5
6

Run2:

Processing test_bug.C...
1
2
3
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
4
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
5
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
Error in <TAxis::TAxis::Set>: bins must be in increasing order
6

Run3:

1
2
3
Error in <TAxis::TAxis::Set>: bins must be in increasing order
4
Error in <TAxis::TAxis::Set>: bins must be in increasing order
5
Error in <TAxis::TAxis::Set>: bins must be in increasing order
6

and the number of errors seems pretty random.

This is true wether I run the snippet in the interpreter, or the full compiled code where the code was actually taken from.

I’m running root v 6.11.02 on a macbook pro with xcode 9.1 but could reproduce the problem also on SLC6 with root5 and gcc 4.9.3.

I suspect this is due to the usage of shared_prt, I wanted all the arrays to get deleted after they are not needed anymore.

Cheers,
Valerio

2

Your code has undefined behaviour.

Here:

  auto selbins = MakeBinsArray(nselbins, (Double_t) 0, (Double_t) nselbins).get();

You deref the temporary smart pointer, i.e. you store the raw pointer in selbins. After this is completed, the temporary shared_ptr object is destroyed. I.e. you are not allowed to deref selbins!

The real question here is: what is wrong with std::vector? Why shared_ptr? You don’t need sharing at all! You could use unique_ptr. But why?! Just use vector and be happy.

3

Ok thanks, I thought something like this, but wasn’t realising where it happened.

The original problem is that TH3 have no “mixed” constructor (i.e. where you pass the bin array for one axis, and number-of-bins + boundaries for the others) so I have to build arrays on the fly and then I won’t need them anymore. I wrote a function for this, and I wanted to make sure to get rid of them once the histograms are created. “Auto-deletion” triggered in my mind the idea of using smart pointers.
But then I run into the problem that I can’t pass smart pointers to the TH3 constructor, they actually want the raw pointer (one could argue that since ROOT went full c++11 we could also have constructors that allow the use of other pointer types other than raw pointers, but I realise that it’s a first world problem).

As far as I know, I can’t pass them a vector either, correct if I’m wrong.

4

Look up http://www.cplusplus.com/reference/vector/vector/data/ if that is what is missing.

That would NOT be a good idea. You take raw pointers as function arguments if you don’t deal with ownership. And the histogram constructor does not do anything to the lifetime of your input data.

5

Yep, that was it, I didn’t know about that.

Understood, thanks

6

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.