Much qa reco

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

There are some QA parameters which are only printed out on the screen:

printf("===================================\n");

printf("StatisticsQa:\n");

printf("Total number of points: %i\n", fPointsTotal);

printf("Points overcounted: %i\n", fPointsOverCounted);

printf("Points undercounted: %i\n", fPointsUnderCounted);

printf("Signal points: %i\n", fSignalPoints);

printf("Signal hits: %i\n", fSignalHits);

I think they should be put to histograms.

added 1 commit

c2e291c7 - CbmMuchQA: Cleaned up initializations.

Compare with previous version

added 1 commit

6110ed9d - CbmMuchHitFinderQa: Updated includes according to iwyu.

Compare with previous version

@se.gorbunov should I change these to std::vector<TH1I*> as in the other Qa classes. It has the advantage that I don't need to keep track of fNstations.

TH1I** fhPointsInCluster = nullptr;

TH1I** fhDigisInCluster = nullptr;

TH1I** fhHitsInCluster = nullptr;

added 1 commit

056b378d - CbmMuchHitFinderQa: Implemented DeInit() method.

Compare with previous version

added 1 commit

7da13518 - CbmMuchHitFinderQa: Implemented DeInit() method.

Compare with previous version

added 1 commit

bea80841 - Expanded deconstructors of CbmMuchDigitizerQa and CbmMuchTransportQa.

Compare with previous version

added 9 commits

967283d1 - 1 commit from branch computing:master
fe26c609 - MuchQA: Added new files for class for reconstruction QA. Added this class to task list in run_qa.C.
1b9c6e96 - Cleanup of CbmMuchRecoQa.
8ab9c9c9 - CbmMuchRecoQa: Added canvases to store histograms. Added safeguards for absent data.
c2af5e8f - Renamed CbmMuchRecoQa.cxx to CbmMuchHitFinderQa.cxx. Moved it to...
7bf48719 - CbmMuchQA: Cleaned up initializations.
cd3a7fa1 - CbmMuchHitFinderQa: Updated includes according to iwyu.
3c8e3130 - CbmMuchHitFinderQa: Implemented DeInit() method.
532e5c14 - Expanded deconstructors of CbmMuchDigitizerQa and CbmMuchTransportQa.

Compare with previous version

@d.smith, I think, it makes sense.

@d.smith,

I forgot to tell you: we need to store the number of processed events as a TParameter in the hist folder. Look how it is done in the transport QA:

TParameter fhNevents; /// number of processed events

We should do it the digi QA as well.

@d.smith there is too much INFO output:

[INFO] Event: 96 start Stat QA [INFO] Event: 96 [INFO] Event: 97 [INFO] Event: 97 start Stat QA [INFO] Event: 97

I think it is not needed. It is more like DEBUG information. For my taste, INFO makes sense only at the end as a summary, or at the initialization, but not at every event.

added 11 commits

532e5c14...ce91d7fa - 2 commits from branch computing:master
31c42c74 - MuchQA: Added new files for class for reconstruction QA. Added this class to task list in run_qa.C.
7ec7cbc5 - Cleanup of CbmMuchRecoQa.
fa8bd3e2 - CbmMuchRecoQa: Added canvases to store histograms. Added safeguards for absent data.
4b8a2421 - Renamed CbmMuchRecoQa.cxx to CbmMuchHitFinderQa.cxx. Moved it to...
b49d115b - CbmMuchQA: Cleaned up initializations.
91e0cddc - CbmMuchHitFinderQa: Updated includes according to iwyu.
66fc331a - CbmMuchHitFinderQa: Implemented DeInit() method.
605d4670 - Expanded deconstructors of CbmMuchDigitizerQa and CbmMuchTransportQa.
764c8625 - CbmMuchHitFinderQa: Changed some histogram names and labels. Added statistics...

Compare with previous version

added 1 commit

7a678cc6 - CbmMuchHitFinderQa: Changed some histogram names and labels. Added statistics...

Compare with previous version

added 1 commit

898382e8 - CbmMuchHitFinderQa: Replaced several Int_t counters by TParameter<int>...

Compare with previous version

added 1 commit

b310d191 - CbmMuchHitFinderQa: Replaced several Int_t counters by TParameter<int>...

Compare with previous version

added 1 commit

599a8eff - CbmMuchHitFinderQa: Replaced several Int_t counters by TParameter<int>...

Compare with previous version

added 26 commits

599a8eff...b95ae2e9 - 16 commits from branch computing:master
e614e21d - MuchQA: Added new files for class for reconstruction QA. Added this class to task list in run_qa.C.
9f9d7da0 - Cleanup of CbmMuchRecoQa.
51bb6c34 - CbmMuchRecoQa: Added canvases to store histograms. Added safeguards for absent data.
fd733406 - Renamed CbmMuchRecoQa.cxx to CbmMuchHitFinderQa.cxx. Moved it to...
6b38e1f6 - CbmMuchQA: Cleaned up initializations.
d197f765 - CbmMuchHitFinderQa: Updated includes according to iwyu.
0904d0df - CbmMuchHitFinderQa: Implemented DeInit() method.
077081a3 - Expanded deconstructors of CbmMuchDigitizerQa and CbmMuchTransportQa.
fb187459 - CbmMuchHitFinderQa: Changed some histogram names and labels. Added statistics...
ca76681f - CbmMuchHitFinderQa: Replaced several Int_t counters by TParameter<int>...

Compare with previous version

added 42 commits

ca76681f...98a15643 - 31 commits from branch computing:master
84115493 - MuchQA: Added new files for class for reconstruction QA. Added this class to task list in run_qa.C.
decfc250 - Cleanup of CbmMuchRecoQa.
591b0362 - CbmMuchRecoQa: Added canvases to store histograms. Added safeguards for absent data.
cac88d12 - Renamed CbmMuchRecoQa.cxx to CbmMuchHitFinderQa.cxx. Moved it to...
bcc6e5a4 - CbmMuchQA: Cleaned up initializations.
a60a9d56 - CbmMuchHitFinderQa: Updated includes according to iwyu.
005aa30b - CbmMuchHitFinderQa: Implemented DeInit() method.
53db33ed - Expanded deconstructors of CbmMuchDigitizerQa and CbmMuchTransportQa.
ea79207d - CbmMuchHitFinderQa: Changed some histogram names and labels. Added statistics...
12051109 - CbmMuchHitFinderQa: Replaced several Int_t counters by TParameter<int>...
be167051 - CbmMuchHitFinderQa: Added statistics to pull and residual distributions.