Skip to content
Snippets Groups Projects

Online-capable hitfinder for TRD and TRD2D

Merged Dominik Smith requested to merge d.smith/cbmroot:OnlineTrdHitfinder into master
  • Added online capable TRD hitfinders (1D and 2D) in cbm::algo.
  • Added ROOT-free implementations of CbmTrdCluster and CbmTrdHit.
  • Added class CbmTaskTrdHitFinder as wrapper for new hitfinders.
  • Added class CbmTaskTrdHitFinderParWrite to produce YAML input files.
  • Added example macro trd_hitfinder_run.C in beamtime/mcbm2022 folder to run new hitfinder.
  • Added a required getter function to CbmTrdParModGeo.h.
  • Updated CMakeLists and LinkDef files to enable compilation.

The current state of the code is ready to be integrated into the online reconstruction chain. The hotfix from !1752 (merged) is currently not included and will be implemented separately. The code is also currently not parallelized, which will be a separate MR.

Performance-wise there are drastic improvements compared to the original classes. A benchmark on a single core of a Core i9 13900K CPU shows that a high-multiplicity timeslice with 15 million TRD digis (1D and 2D combined) is processed in less than 10 seconds.

There are surely possibilities for further improvements, both in terms of performance and code structure. In particular, I would like to further clean up the HitFinder2D class. The current state is fit for purpose however.

The 1D system is missing a hit-merge step, as it was concluded in discussions that the original implementation does not work as intended. This can also be added at a later time.

The code is organized as follows: There are now separate classes for the building of clusters and the subsequent creation of hit objects from the clusters (both steps were done in by one class in the original codes). These each exist separately for the 1D and 2D systems. The base classes were dropped entirely, so each algorithm is a stand-alone class. There is a steering class trd/Hitfind which wraps all of the algorithms. Everything is configurable by YAML files (one for 1D and one for 2D).

ROOT-free versions of CbmTrdCluster (separately for 1D and 2D) and CbmTrdHit (combined for 1D and 2D) are included, which do away with the dependency on CBM base classes (such as CbmPixelHit) etc. The later (hit) class maintains the interface of the original CbmTrdHit class, so it includes some functions which are not needed by this implementation of the hitfinder. There is also a ROOT-free version of CbmRecoDigi which is needed by the 2D hitfinder.

There are some preprocessor macros SETBIT, CLRBIT etc. which are originally from ROOT and have been re-defined in header files as needed. This currently produces some compiler warnings. We should ultimately put them somewhere in the algo folder (to be decided by @fweig).

Likewise, there is a function TMath:Nint() which rounds floats to the nearest even integer (Gaussian or Banker's rounding). I found no std function which reproduces this behavior, so I simply copied the ROOT version to the class which needs it.

@fweig in principle this is ready to be added to CbmReco. The interface is essentially the same as for TOF.

For lack of better ideas, I added @v.friese as reviewer. In principle this is relevant at least for @fweig @a.bercuci @p.kaehler @apuntke @dschledt.

Also maybe of interest to @p.-a.loizeau @f.uhlig

Update: Included the hotfix from !1752 (merged) and added OpenMP parallelization.

Update2: Added unit test supplied by @apuntke

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
  • added Online label

  • Dominik Smith requested review from @v.friese

    requested review from @v.friese

  • assigned to @d.smith

  • Dominik Smith mentioned in merge request !1747 (closed)

    mentioned in merge request !1747 (closed)

  • Dominik Smith added 1 commit

    added 1 commit

    • 62dc283b - - Added online capable TRD hitfinders (1D and 2D) in cbm::algo.

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

    • 19ee577d - - Added online capable TRD hitfinders (1D and 2D) in cbm::algo.

    Compare with previous version

  • Dominik Smith mentioned in merge request !1605 (closed)

    mentioned in merge request !1605 (closed)

  • Dominik Smith mentioned in merge request !1761 (closed)

    mentioned in merge request !1761 (closed)

  • Dominik Smith added 4 commits

    added 4 commits

    • 19ee577d...ec494a7b - 3 commits from branch computing:master
    • 23590151 - - Added online capable TRD hitfinders (1D and 2D) in cbm::algo.

    Compare with previous version

  • There are some preprocessor macros SETBIT, CLRBIT etc. which are originally from ROOT and have been re-defined in header files as needed. This currently produces some compiler warnings. We should ultimately put them somewhere in the algo folder (to be decided by @fweig).

    In principle a header in base/compat/ sounds like the place to put this.

  • Dear @d.smith, @fweig, @a.bercuci, @p.kaehler, @f.uhlig, @p.-a.loizeau, @v.friese,

    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 added 9 commits

    added 9 commits

    • 23590151...f8a30891 - 6 commits from branch computing:master
    • 5855f931 - - Added online capable TRD hitfinders (1D and 2D) in cbm::algo.
    • f901e691 - Applied hotfix from MR 1752 to online TRD code.
    • 90ad0914 - Applied OpenMP parallelization to trd::Hitfind.

    Compare with previous version

  • Dominik Smith changed the description

    changed the description

  • Author Maintainer

    @fweig OpenMP parallelization has been added and tested. Ready for merge in my opinion.

    Additional threads don't seem to help all that much. Not surprising, because the bottleneck is the hit-merging in the 2D modules, of which there is only one in the run 2457 data.

  • Dominik Smith added 1 commit

    added 1 commit

    • 32cf7693 - Applied OpenMP parallelization to trd::Hitfind.

    Compare with previous version

    • Resolved by Felix Weiglhofer

      I have created some tests to check the behavior of the new clusterizer after the implementation of the hofix (to be added in algo/test)._GTestTrdClusterizer.cxx

      The tests succeed and therefore I think the fix works.

      You can include this tests also in this MR if you like.

      Log output:

      7/7 Testing: _GTestTrdClusterizer
      7/7 Test: _GTestTrdClusterizer
      Command: "/lustre/cbm/users/apuntke/beamtime/d.smith-cbmroot-TrdHotFix/build/bin/_GTestTrdClusterizer"
      Directory: /u/apuntke/dev/d.smith-cbmroot-TrdHotFix/build/algo/test
      "_GTestTrdClusterizer" start time: Apr 19 15:53 CEST
      Output:
      ----------------------------------------------------------
      Running main() from /lustre/cbm/users/apuntke/beamtime/d.smith-cbmroot-TrdHotFix/src/external/googletest/googletest/src/gtest_main.cc
      [==========] Running 14 tests from 1 test suite.
      [----------] Global test environment set-up.
      [----------] 14 tests from _GTestTrdClusterizer
      [ RUN      ] _GTestTrdClusterizer.Check3PadCluster
      [       OK ] _GTestTrdClusterizer.Check3PadCluster (0 ms)
      [ RUN      ] _GTestTrdClusterizer.Check1PadSTCluster
      [       OK ] _GTestTrdClusterizer.Check1PadSTCluster (1 ms)
      [ RUN      ] _GTestTrdClusterizer.Check1PadNTCluster
      [       OK ] _GTestTrdClusterizer.Check1PadNTCluster (0 ms)
      [ RUN      ] _GTestTrdClusterizer.Check2PadLeftNTCluster
      [       OK ] _GTestTrdClusterizer.Check2PadLeftNTCluster (0 ms)
      [ RUN      ] _GTestTrdClusterizer.Check2PadRightNTCluster
      [       OK ] _GTestTrdClusterizer.Check2PadRightNTCluster (0 ms)
      [ RUN      ] _GTestTrdClusterizer.Check2PadWithSTOnNextRowCluster
      [       OK ] _GTestTrdClusterizer.Check2PadWithSTOnNextRowCluster (0 ms)
      [ RUN      ] _GTestTrdClusterizer.Check2PadWithNTOnNextRowCluster
      [       OK ] _GTestTrdClusterizer.Check2PadWithNTOnNextRowCluster (1 ms)
      [ RUN      ] _GTestTrdClusterizer.Check2PadWithSTOnPrevRowCluster
      [       OK ] _GTestTrdClusterizer.Check2PadWithSTOnPrevRowCluster (0 ms)
      [ RUN      ] _GTestTrdClusterizer.Check2PadWithNTOnPrevRowCluster
      [       OK ] _GTestTrdClusterizer.Check2PadWithNTOnPrevRowCluster (0 ms)
      [ RUN      ] _GTestTrdClusterizer.CheckAdjacent3PadClusters
      [       OK ] _GTestTrdClusterizer.CheckAdjacent3PadClusters (4 ms)
      [ RUN      ] _GTestTrdClusterizer.Check3PadWithSingleNTCluster
      [       OK ] _GTestTrdClusterizer.Check3PadWithSingleNTCluster (0 ms)
      [ RUN      ] _GTestTrdClusterizer.CheckLeftAndRightBordersFullyTriggered
      [       OK ] _GTestTrdClusterizer.CheckLeftAndRightBordersFullyTriggered (1 ms)
      [ RUN      ] _GTestTrdClusterizer.CheckTimeDistanceSmallEnough
      [       OK ] _GTestTrdClusterizer.CheckTimeDistanceSmallEnough (0 ms)
      [ RUN      ] _GTestTrdClusterizer.CheckTimeDistanceTooHigh
      [       OK ] _GTestTrdClusterizer.CheckTimeDistanceTooHigh (0 ms)
      [----------] 14 tests from _GTestTrdClusterizer (7 ms total)
      
      [----------] Global test environment tear-down
      [==========] 14 tests from 1 test suite ran. (7 ms total)
      [  PASSED  ] 14 tests.
      <end of output>
      Test time =   0.06 sec
      ----------------------------------------------------------
      Test Passed.
      "_GTestTrdClusterizer" end time: Apr 19 15:53 CEST
      "_GTestTrdClusterizer" time elapsed: 00:00:00
      ----------------------------------------------------------
  • Dominik Smith added 1 commit

    added 1 commit

    • 6746f1a1 - Added unit test provided by A.Puntke.

    Compare with previous version

  • Dominik Smith added 1 commit

    added 1 commit

    • b7bc07e7 - Added unit test provided by A.Puntke.

    Compare with previous version

  • Dominik Smith changed the description

    changed the description

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading