Skip to content
Snippets Groups Projects

Much qa reco

Merged Dominik Smith requested to merge d.smith/cbmroot:muchQAreco into master
All threads resolved!

QA for MUCH cluster and hit finder.

Edited by Dominik Smith

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Author Maintainer

    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.

  • Author Maintainer

    Ok. I will wait for input from Sergey.

  • Dominik Smith added 1 commit

    added 1 commit

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

    Compare with previous version

  • Florian Uhlig resolved all threads

    resolved all threads

    • Resolved by Sergey Gorbunov

      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) ^~~~~~~~~~~~~~~~~~

  • Dominik Smith added 1 commit

    added 1 commit

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

    Compare with previous version

  • Author Maintainer

    Fixed it

  • Sergey Gorbunov resolved all threads

    resolved all threads

    • Resolved by Dominik Smith

      @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.

    • Resolved by Sergey Gorbunov

      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.

    • Resolved by Sergey Gorbunov

      @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.

    • Resolved by Dominik Smith
      //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

    • Resolved by Dominik Smith

      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.

      Edited by Sergey Gorbunov
  • Dominik Smith added 1 commit

    added 1 commit

    • c2e291c7 - CbmMuchQA: Cleaned up initializations.

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

    • 6110ed9d - CbmMuchHitFinderQa: Updated includes according to iwyu.

    Compare with previous version

  • Author Maintainer

    @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;

    Edited by Dominik Smith
  • Dominik Smith added 1 commit

    added 1 commit

    • 056b378d - CbmMuchHitFinderQa: Implemented DeInit() method.

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

    • 7da13518 - CbmMuchHitFinderQa: Implemented DeInit() method.

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

    • bea80841 - Expanded deconstructors of CbmMuchDigitizerQa and CbmMuchTransportQa.

    Compare with previous version

  • Dominik Smith added 9 commits

    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.

    • Resolved by Dominik Smith

      @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.

    • Resolved by Dominik Smith

      @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.

  • Dominik Smith added 11 commits

    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

  • Dominik Smith added 1 commit

    added 1 commit

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

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

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

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

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

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

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

    Compare with previous version

  • Dominik Smith added 26 commits

    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

  • Dominik Smith added 42 commits

    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.

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

    • c6655f47 - CbmMuchQa: Cleaned up screen output.

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

    • 4f9e57f8 - CbmMuchHitFinderQa: Added some safeguards.

    Compare with previous version

  • Dominik Smith resolved all threads

    resolved all threads

  • Author Maintainer

    @se.gorbunov all threads are resolved.

  • Dominik Smith added 1 commit

    added 1 commit

    • 15ce0d06 - CbmMuchHitFinderQa: Added some safeguards.

    Compare with previous version

  • Please register or sign in to reply
    Loading