Much qa reco

QA for MUCH cluster and hit finder.

changed title from Much q areco to Much qa reco

added 1 commit

• 61dae671 - CbmMuchRecoQa: Added canvases to store histograms. Added safeguards for absent data.

Compare with previous version

added 1 commit

• 0979afb7 - CbmMuchRecoQa: Added canvases to store histograms. Added safeguards for absent data.

Compare with previous version

assigned to @se.gorbunov

added 7 commits

• 0979afb7...6516ec1e - 4 commits from branch computing:master
• 330a48e9 - MuchQA: Added new files for class for reconstruction QA. Added this class to task list in run_qa.C.
• 74601bb7 - Cleanup of CbmMuchRecoQa.
• 17801b4d - CbmMuchRecoQa: Added canvases to store histograms. Added safeguards for absent data.

Compare with previous version

Ok. But in the folder "~/cbmroot/reco/detectors/much" there is still the file "CbmMuchHitFinderQa.cxx", which is the original source of these Qa tasks. What should we do with it? And also: Should we make a "qa" subfolder?

If the original class isn't needed any longer it should be removed. In the end Sergey and you have to decide on that after checking if the class is used in some of our macros. Concerning the qa directory I have the tendency to create one.

Ok. I will wait for input from Sergey.

added 1 commit

• 7226b6eb - Renamed CbmMuchRecoQa.cxx to CbmMuchHitFinderQa.cxx. Moved it to...

Compare with previous version

resolved all threads

Hi @d.smith, I got compiler warnings:

In file included from /home/cbmdock/cbmroot/reco/detectors/much/qa/CbmMuchHitFinderQa.cxx:8: /home/cbmdock/cbmroot/reco/detectors/much/qa/CbmMuchHitFinderQa.h: In constructor 'CbmMuchHitFinderQa::CbmMuchHitFinderQa(const char*, Int_t)': /home/cbmdock/cbmroot/reco/detectors/much/qa/CbmMuchHitFinderQa.h:72:9: warning: 'CbmMuchHitFinderQa::fNstations' will be initialized after [-Wreorder] Int_t fNstations; ^~~~~~~~~~ /home/cbmdock/cbmroot/reco/detectors/much/qa/CbmMuchHitFinderQa.h:65:11: warning: 'TFolder CbmMuchHitFinderQa::fOutFolder' [-Wreorder] TFolder fOutFolder; /// output folder with histos and canvases ^~~~~~~~~~ /home/cbmdock/cbmroot/reco/detectors/much/qa/CbmMuchHitFinderQa.cxx:56:1: warning: when initialized here [-Wreorder] CbmMuchHitFinderQa::CbmMuchHitFinderQa(const char* name, Int_t verbose) ^~~~~~~~~~~~~~~~~~

added 1 commit

• a3dd4722 - Renamed CbmMuchRecoQa.cxx to CbmMuchHitFinderQa.cxx. Moved it to...

Compare with previous version

Fixed it

resolved all threads

@d.smith,

Looks nice, I have some wishes for the histograms:

"Points in cluster" -> I'd rename it to "MC points in cluster", to make it clear. These are MC points, right?

"Hits in cluster" -> I think, "in" is misleading here. If I'm not mistaken, a hit is composed out of two clusters. So, clusters are "in" hits, not vise versa. We better name it "Hits per cluster" or so.

Pulls distribution: these histograms need statistics, including N outliers and a result of a gaussian fit, better in large font that the numbers are readable. It can be set via gStyle->SetOptStat(..). I don't remember the right parameter value. Something like 111111.

Pull distribution T: set range to +-5, like in the other pulls. Don't know why it is set asymmetric. Perhaps somebody was debuggíng something some time ago..

Residuals: Again, statistics are missing and the T range is asymmetric.

Hi @d.smith

These includes can be replaced by forward declarations. "TH2.h" is not used at all, I think.

#include "CbmMuchDigi.h"

#include "TClonesArray.h"

#include "TH1.h"

#include "TH2.h"

FYI: In c++11 you can initialize class members in the header. This way you don't need to repeat all of them in all the constructors in the right order. It makes the code cleaner, I think. But it is a matter of taste.

@d.smith, there is no destructor.

Also, in Init() you need to delete everything before you call new. Potentially, this Init() can be called several times. It makes sense to write some private DeInit() which deletes everything and then call it from Init() and from the destructor.

You can use SafeDelete(p) macro. It deletes a pointer and then sets it to nullptr.

//Access Match from CbmDigi only
  CbmMatch* match =
    (CbmMatch*) fDigiManager->GetMatch(ECbmModuleId::kMuch, index);

One should ensure that match is not 0 here.

There is a flag fDigiManager->IsMatchPresent(ECbmModuleId::kMuch) If matches are present and a match is 0, we should produce an error. When matches are not present, we should just not go to this loop