Skip to content
Snippets Groups Projects

CbmBuildEventsQA: Added histograms to display percentage of correct digis per...

Merged Dominik Smith requested to merge d.smith/cbmroot:EventBuilderQaHistos into master

@se.gorbunov @f.uhlig @v.friese

Added two histograms to the CbmBuildEventsQA class to visualize the event-wise distribution of the percentage of correct digis and percentage of found digis. In order to follow the conventions of the other QA classes, a QA folder was created in the output tree and a histogram sub-folder within it, which contains the histograms.

Right now only the distributions for the full system are shown. Additional histograms for each subsystem can easily be added. Also, canvases can be added outside of the histogram folder.

Following the conventions of the other QA classes, a DeInit() function was created, which is called inside Init(). This was requested by @se.gorbunov.

The histograms span the range 0 to 100.1 instead of 0 to 100 because root seems to place edge values in the upper bin. Here this is unfortunate because the value "100 percent" occurs frequently and would not be displayed if 100 was the upper limit. I did not manage to find out how to change this behavior in root. Perhaps someone knows.

(side remark @v.friese: I was working on the requested unpacking/eventbuilding macro but lustre went offline and has not been back up, so I could not continue and did this instead)

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
  • Hi @d.smith, DeInit() is not a requirement. It is just useful because you can avoid code duplication in Init() and in the destructor.

    I think my idea was to have GetQa(), which prepares the output folder:

    /// Prepare Qa output and return it as a reference to an internal folder.
      /// The reference is non-const, because FairSink can not write const objects
      TFolder& GetQa();

    And call it from the Finish():

    void CbmMuchDigitizerQa::Finish() {
    
      if (!FairRootManager::Instance() || !FairRootManager::Instance()->GetSink()) {
        LOG(error) << "No sink found";
        return;
      }
      FairSink* sink = FairRootManager::Instance()->GetSink();
      sink->WriteObject(&GetQa(), nullptr);
    }

    My idea was that for the online processing, QA tasks would probably need to produce their output regularly, not only at the end of the data processing. I don't know if it makes sense :)

    Maybe it is time to create a base class CbmQaTask where all the requirements will be written in C++ language:)

  • Sergey Gorbunov approved this merge request

    approved this merge request

  • Dominik Smith added 1 commit

    added 1 commit

    • 3d6b6ee0 - CbmBuildEventsQA: Histograms with eventbuilder performance are now produced...

    Compare with previous version

  • Author Maintainer

    In this updated version, histograms are produced individually for each subsystem (it is automatically detected which subsystems are present and a std::map container with histograms is produced). I think this could be very useful for the tuning of the different time windows. A quick look at some example runs shows that the distributions can vary quite a bit between subsystems.

    @se.gorbunov I haven't implemented a separate GetQa() here because only a single thing is done, but I agree that it makes sense for larger QA classes which do lots of different things. I also agree that we might need a base class at some point.

  • @d.smith, concerning the histograms, I don't know any other trick. Maybe it will look nicer when the upper border is set to 101. The 100.1 value might produce non-integer ticks on the axis.

    Do you want comments from the others, or should I merge the request?

  • Author Maintainer

    The ticks seem to be ok. You can merge in my opinion.

  • Hi @d.smith I can not merge for some reason, I guess you need to press a rebase button first.

  • Dominik Smith added 3 commits

    added 3 commits

    • 00116898 - 1 commit from branch computing:master
    • 89c72128 - CbmBuildEventsQA: Added histograms to display percentage of correct digis per...
    • 11788bff - CbmBuildEventsQA: Histograms with eventbuilder performance are now produced...

    Compare with previous version

  • Author Maintainer

    @se.gorbunov I just pressed it. Maybe it will work now.

  • Dear @d.smith,

    you have been identified as code owner of at least one file which was changed with this merge request.

    Please check the changes and approve them or request changes.

  • Dominik Smith approved this merge request

    approved this merge request

Please register or sign in to reply
Loading