Extracting a fit form a class function

Newbie
1

Dear experts,

I am writing an analysis code spread across several different classes with global inheritance form a base class object. The base class object is equipped with several helper functions (recursive directory creation, file counting, derivations, statistics functions, smoothing and iterative fitting).
I am passing a vector of doubles to the iterative fitting function and request the result of a Gaussian fit. The iterative fitting will try several binning and constraint combinations and return the best result based on a chi2/NDF test.
This works fine as long as I only want to get numerical values, (which are passed as references to the function) but I encounter a crash when i try to extract the TH1D object to save it to my ntuple.

The declaration of the function is the following:

int BaseCalss::IterativeFit(std::vector<double> *w, std::pair<double, double> &gmean, std::pair<double, double> &gsigma, TH1D* &FitHist, double &minchi2, std::string methode, std::pair<int, int> points)

Internally the function will create a vector of TH1D* with all the possible trials:
std::vector<TH1D* > Fits;

At some point at the end of the function, when I have decided which is the best fit, i do:
FitHist = (TH1D*)(Fits.at(indx))->Clone();

There exactly root will crash completely. No compiler errors, no warning messages other that:

===========================================================
#5  0x00007f4a32194865 in LGADBase::IterativeFit(std::vector<double, std::allocator<double> >*, std::pair<double, double>&, std::pair<double, double>&, TH1D*&, double&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<int, int>) () from /afs/cern.ch/user/e/egkougko/private/LGADUtils/Root/LGADFits_cxx.so
#6  0x00007f4a31b5ff98 in WaveForm::Calculate() () from /afs/cern.ch/user/e/egkougko/private/LGADUtils/Root/WaveForm_cxx.so
#7  0x00007f4a3193ed0f in DUTChannel::AppendWaveform(WaveForm*, bool) () from /afs/cern.ch/user/e/egkougko/private/LGADUtils/Root/DUTChannel_cxx.so
#8  0x00007f4a3193fd4d in DUTChannel::AppendEvent(std::vector<double, std::allocator<double> >*, std::vector<double, std::allocator<double> >*, bool) () from /afs/cern.ch/user/e/egkougko/private/LGADUtils/Root/DUTChannel_cxx.so
#9  0x00007f4a3171b0b3 in LGADRun::Process(long long) () from /afs/cern.ch/user/e/egkougko/private/LGADUtils/Root/LGADRun_cxx.so
#10 0x00007f4a3391f360 in TTreePlayer::Process(TSelector*, char const*, long long, long long) () from /cvmfs/sft.cern.ch/lcg/releases/ROOT/6.14.04-0d8dc/x86_64-slc6-gcc62-opt/lib/libTreePlayer.so
#11 0x00007f4a3150cf47 in LGADUtils::Analyse(long long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) () from /afs/cern.ch/user/e/egkougko/private/LGADUtils/Root/LGADUtils_cxx.so
#12 0x00007f4a469550aa in ?? ()
#13 0x0000000000000016 in ?? ()
#14 0x0000000000000000 in ?? ()
===========================================================

The code has been tried in windows (root 5.34/36) and linux (root 6.14/0.4) wih the same exact results.

What is the correct way to get the fit out of the function? I also tried instead of passing a reference to a pointer TH1D* to pass a reference to a TH1D and at the end assignment I set it using the de-referencing operator ( FitHist = *Fits.at(indx); ), but the results are the same.

Please read tips for efficient and successful posting and posting code

ROOT Version: Not Provided
Platform: Not Provided
Compiler: Not Provided

2

Most likely the pointer in
std::vector<TH1D* > Fits;
are invalid at the time you use them. The ‘question’ is why.

On step toward resolution would be to run the failing example with valgrind (for example valgrind --suppressions=$ROOTSYS/etc/valgrind-root.supp root.exe -b -l -q ...) and with some luck this will point out the problem.

Cheers,
Philippe.

3

Thanks a lot for the answer Philippe. I think the problem was that I was creating a dummy object during the re-fitting:
TH1D* Mag
which I was then using to perform all the mathematical operations and at the end of each iteration I was trying to save it to the std::vector<TH1D*> Fits with a simple pus_back() operation instead of a Clone(). Although I am still not quite sure why the push_back wouldn’t work, is it because there is no equal operator for the TH1 objects?

Anyway, I used directly the vector to perform all the iterations and that seemed to work, but hen i had to deal with all the usual root issues about not deleting the objects when one does Fits.at(i)->Delete(), so I had to search the gDirectory and delete the objects by name in each event.
I would expect the Delete() to remove the object form stack but i guess because I have an output file already open, root would save everything in it even if the object pointer is no longer there right?

4

The ‘Delete’ function is used to remove the ‘content’ on an object (for example mylist->Delete(); calls operator delete on each of the element of the list). So to remove the object itself you do need to call delete Fits.at(i);.

that I was creating a dummy object during the re-fitting:
TH1D* Mag

Since that is a pointer rather than object, the object is not temporary. However histograms ‘attach’ themselves to the current ROOT directory (gDirectory) at the time of their creation and when that directory is deleted, it deletes anything that was attached to it. When you do the Clone then the new copy get attached to a different directory and that might be the difference. (For example, you could see if calling ``Mag->SetDirectory(nullptr);``` makes a difference (you will then also need to explicitly write the histograms to the output file).

Cheers,
Philippe.

5

Thank you very much, this was very helpful. I had the misconception that Delete() would also take care of removing the object, which is not the case. I manged to fix the issues by cloning and correctly deleting all objects.
I might have a couple of questions on the same topic (should I move to another thread?):

  1. What is root’s behavior when two or more different object pointers have the same object name? Would all objects be attached to the gDirectory as different versions or will they be truncated even if logically the pointers are actually different? (eg. Fits.at(1) = new TH1D("hist", "hist") and Fits.at(2) = new TH1D("hist", "hist"))

  2. While deleting, would for instance: delete Fits.at(i); be equivalent as doing Fits.at(i)->Delete(); Fits.at(i)=NULL; ? I assume not because although the pointer is null, the object empty and the pointer is no longer associated with anything, the object still exists right?

  3. When I close a file, root will crash unless i have already emtired the tree and objects associated with that file. So for instance if I do m_treee->Write(); and then m_file->Close();, he does not like that very much. If on the other hand I do m_treee->Write();, m_tree->Delete(); and then m_file->Close(); he seems to be happy. What is the correct way? One should delete/empty all associated objects to the file before closing the file?

Thank you very much again for the help!!

Cheers,

Vagelis

6
  1. In a ROOT directory the answer depends on the type. The default behavior is that they both ends up in the list, only one can be retrieve by name but both can be accessed by iterating. However for histogram you will get the explicit/accurate message:
Warning in <TROOT::Append>: Replacing existing TH1: histname (Potential memory leak).

i.e. the older histogram with the same name is remove from the list but not delete.

  1. “, the object still exists right?” → yes, exactly.

  2. m_treee->Write(); and then m_file->Close();, should be sufficience (albeit I would recommend m_file->Write(); delete m_file();).

One should delete/empty all associated objects to the file before closing the file?

no, by ‘definition’, the TFile will delete the object that it co-own (i.e. the object associated to the file).

he does not like that very much.

Where does it crash?

7

Thanks again for all the help and the clarifications, this is really helpful!!

Just to come back to the last issue, I will give a bit of insight on what i do:

  • I open a bunch of raw files (in various non root formats) and convert them to a root file with a tree (lets call it m_ofile, m_tree and m_trigDt which is a TH1F object I would like to include). At the end of the conversion process I actually want the file and the tree to continue their existence since I intend to access them with a TSelector inherited analysis object. The only thing I managed to do so far that work is:
  m_ofile->Write();
  delete m_trigDt;
  delete m_tree;
  m_ofile->Close();

I cannot delete the file object because then root will crash (not a class crash, since for the moment I actually reassign the file object before passing it to the analysis). Anyway, this is not the main problem here.

During the analysis, I re-assign the m_ofile and m_tree objects by doing:

m_ofile = TFile::Open(file); -> file is a const char and is the filemname
m_tree = (there is a function here that looks through the m_file and finds the object named "tree" and sets it to the pointer)
m_ofile->ReOpen("UPDATE");
m_ofile->SetCompressionLevel(6)

Then since I want to write the updated tree on that file i do:

m_ofile->cd();
m_tree->Write("", TObject::kOverwrite);

The cd command is needed here, since I have during the analysis opened different files to recover info. Then, one would expect that the updated tree is already saved in the file (without the need to close the file right?), so if i try to re-access the new branches that should be possible. This seems not to be the case since the new branches are not saved. I think I am still missing a concept here, but i don’t really know what it is…

8

All of this ‘should’ work from your description, so we would need a standalone reproducer of the failure to be able to debug this further.

As a side note, one thing I would do different is to use

m_ofile = TFile::Open(file,"UPDATE");
m_tree = (there is a function here that looks through the m_file and finds the object named "tree" and sets it to the pointer)
m_ofile->SetCompressionLevel(6)

Actually one more thing:

I cannot delete the file object because then root will crash (not a class crash, since for the moment I actually reassign the file object before passing it to the analysis).

is very suspicious. In your description there is no reasons why delete the TFile object would lead to a crash, so there are additional use/information how that pointer is used that might explain what’s going on. (Note you may want to consider running the failing example with valgrind to get more information about that crash).

9

Thanks again for the very helpful comments and insight.

I managed actually to pinpoint the issue to the fact that there are additional objects attached to the TDirectory that also get deleted when the file is closed.

During the analysis, in the Notify() function of the TSelector, I create the output file and some analysis objects. You might be wondering why I do that there and not in the Begin() function. The reason for that is that I need the TSelector to have already initialized the input file, since I have automatic variable detection and the amount of initialized objects depends on the available variables in the input file. I have a bollean switch in order not to re-call that part every time I open a new file, just on the first one.

The issue is that the analysis objects are also attached to the opened file, so they have to be deleted before the file closes, or they just don’t exist afterwords (even though these are not root objects). By deleting the analysis objects before closing the file I manged to correct the issue.

I noticed though that for the windows version, root does not run the Terminate function of the TSelector at all and crashes once reaching that state (even if the function is empty). At lxplus there is no such issue.

Thanks again for all the help!!

10

The issue is that the analysis objects are also attached to the opened file … (even though these are not root objects)

I am a little confused, if they are not inheriting from TObject (directly or indirectly), how are they attached to the directory?

11

They all inherit all from a TSelector…

12

Alright, so they inherit indirectly from TObject. How are they attached to the directory?

13

Not sure I understand the question (or that I can answer it…)
I would suppose that because I call the object constructor within the Notify() function just after having opened the file in “UPDATE” mode, root would attach them to the open file right?

The objects are declared in the header file as:

std::vector<DUTChannel*> m_DUTCh;

and is created via:

for (unsigned int a = 0; a < m_nchan; a++) m_DUTCh.at(a) = new DUTChannel(m_channels.at(a));

If I want to be rigorous, I would need to delete these objects at the end of my analysis (especially since each one of them creates daughter objects and fits). In their respective destructors, I have implemented the deletion of all daug5ter object they create and I assume (maybe wrongly?) tat by doing:

    for (unsigned int a = 0; a < m_nchan; a++) delete m_DUTCh.at(a);

I am executing the code I implemented in the destructor of the DUTChannel Class.

Now the real question is if I create these objects before I change the input root file mode to “UPDATE”, where will root attach them? If I do that, do I get to close the file and keep the objects in memory until I decide to delete them (although technically when i create them I have already opened the input file, just not in an update mode)?

14

Since you are calling std::vector::at and not std::vector::push_back, did you also call std::vector::resize?

where will root attach them?

ROOT does not (as far as I remember/know) automatically add TSelector objects to a directory. So if they are attached it ought to be because it is done explicitly in your code (or code you call).

If I do that, do I get to close the file and keep the objects in memory until I decide to delete them

Whether or not the TFile is open in update mode, if you attach an object to it, it will delete the object when it closed (and thus when it is deleted).

However the only/main reason to attach an object is a TFile is simplify storing the object in the file. It is unusual for a TSelector to be stored in a TFile (except for its output list).

Cheers,
Philippe.

15

Since you are calling std::vector::at and not std::vector::push_back, did you also call std::vector::resize?

Yes, this is part of the initialization before the object creation. I do not explicitly attach the object to the active directory and I think I do not call any code that does this, although the object itself will create several fits. These will be attached to the directory, but not sure if that means that the parent object is also attached.

The main problem I have now is that in a windows environment root will not go through the terminate function in the selector (in any code, not just this one) …

16

What type of objects do you use for those fits?

The parent object is not attached but if it isn’t entered (directly or indirectly) in the ListOfCleanups and does not implement RecursiveRemove [correctly] then upon closing of the file, if the parent object is still ‘alive’ it will contain dangling pointers.

will not go through the terminate function in the selector (in any code, not just this one) …

Strange. How do you use the selector?

17

OK, I think I get it now.

The analysis objects may inherit from the TSelector but are not attached to the open file. The fits (based on TH1D), nevertheless that these objects create are attached to the file (although I am not saving them because I create a list on which i attach the fits I want to save and then explicitly write the list to the file). This means that when the file closes, the analysis objects loose their pointers to the TH1Ds that they had, causing the code to crash. On the other hand, if I delete the analysis objects, since their destructors take care of also removing whatever they have created, this is no longer an issue. When closing the file afterwords there are no dangling pointers to cause any problems. Sorry for taking so long to get here, the behavour makes sens now to the way the code works.

On the last matter of the Terminate():

I actually use a steering macro that compiles all the classes and then creates a general analysis object (the LGADRun). This is a mirror copy of the TSelector where all the functions are implemented, since I have changed the function declarations to the actual selector to virtual.
So basically this LGADRun object acts now as the Telector and is equipped with the Begin, SlaveBegin, Notify, Process, SlaveTerminate and Terminate methods.

18

You can avoid this by detaching the histograms:

hist->SetDirectory(nullptr);
19

Is the Terminate issue resolved (if not I think we will need a reproducer).?

20

Sorry for being late to reply on this, yes i resolved the terminate issue, there was a problem in the declaration. Thanks !!!