Skip to content
Snippets Groups Projects

det(must): Add first version of the MUST detector

Closed Radoslaw Karabowicz requested to merge r.karabowicz/cbmroot:must_firstversion into master
9 unresolved threads

First version of the MUST detector. Includes simulation and simplified digitization and hit finding.

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
  • 112 /** This variable contains the radial distance to the wire **/
    113 Double_t fIsochrone{0.};
    114 ;
    115 /** This variable contains the error on the radial distance to the wire **/
    116 Double_t fIsochroneError{0.};
    117
    118 /** time pulse **/
    119 Double_t fPulse{0.};
    120
    121 /** deposit charge (arbitrary units) **/
    122 Double_t fDepCharge{0.};
    123
    124 /** tube id **/
    125 Int_t fDetSysId{0}; // CHECK added
    126 Int_t fDetectorId{0};
    127 Int_t fSkewed{0};
    • Comment on lines +110 to +127

      Please, avoid the ROOT types (e.g., fDirection can be a std::array<double, 3>).

      Can the MUST provide a mesurement of a track point in space (to a known precision)? If yes, then please consider inheriting the interface of the CbmPixelHit class (see the CbmTrdHit class as a reference).

    • Please register or sign in to reply
  • 112 /** This variable contains the radial distance to the wire **/
    113 Double_t fIsochrone{0.};
    114 ;
    115 /** This variable contains the error on the radial distance to the wire **/
    116 Double_t fIsochroneError{0.};
    117
    118 /** time pulse **/
    119 Double_t fPulse{0.};
    120
    121 /** deposit charge (arbitrary units) **/
    122 Double_t fDepCharge{0.};
    123
    124 /** tube id **/
    125 Int_t fDetSysId{0}; // CHECK added
    126 Int_t fDetectorId{0};
    127 Int_t fSkewed{0};
  • 95 protected:
    96 // exit coordinates in straw frame
    97 Double_t fX_out_local, fY_out_local, fZ_out_local;
    98 // entry coordinates in straw frame
    99 Double_t fX_in_local, fY_in_local, fZ_in_local;
    100
    101 Double_t fPx_out, fPy_out, fPz_out;
    102 Double_t fPx_in, fPy_in, fPz_in;
    103 // wire direction
    104 // Double_t fX_wire_dir, fY_wire_dir, fZ_wire_dir;
    105
    106 // particle mass
    107 Double_t fMass;
    108 Int_t fLayerID;
    109 Int_t fModuleID;
    110 Int_t fStrawID;
  • 92 /** Output to screen **/
    93 virtual void Print(const Option_t* opt) const;
    94
    95 protected:
    96 // exit coordinates in straw frame
    97 Double_t fX_out_local, fY_out_local, fZ_out_local;
    98 // entry coordinates in straw frame
    99 Double_t fX_in_local, fY_in_local, fZ_in_local;
    100
    101 Double_t fPx_out, fPy_out, fPz_out;
    102 Double_t fPx_in, fPy_in, fPz_in;
    103 // wire direction
    104 // Double_t fX_wire_dir, fY_wire_dir, fZ_wire_dir;
    105
    106 // particle mass
    107 Double_t fMass;
    • A MC-point is fully related to the MC-track, which leaves it in the detector. The MC track has a PDG code field, which carries already the whole information about the track. So I would remove the fMass field

    • Please register or sign in to reply
  • 38 CbmMustFindHits::CbmMustFindHits() : FairTask("MustFindHits", 1) {}
    39
    40 // ----- Destructor ----------------------------------------------------
    41 CbmMustFindHits::~CbmMustFindHits() {}
    42 // -------------------------------------------------------------------------
    43
    44 // ----- Task execution ------------------------------------------------
    45 void CbmMustFindHits::Exec(Option_t* /*opt*/)
    46 {
    47 assert(fDigiArray);
    48 fHitArray->Delete();
    49
    50 // LOG(info) << "got " << fDigiArray->GetEntriesFast() << " points";
    51 for (Int_t iDigi = 0; iDigi < fDigiArray->GetEntriesFast(); iDigi++) {
    52 const CbmMustDigi* digi = (const CbmMustDigi*) fDigiArray->At(iDigi);
    53 if (!digi) LOG(info) << "no digi";
    • This should be a LOG(debug) instead. But in general it should be checked somewhere only once, that the fDigiArray is filled. For STS, TOF and many other detectors the digis are stored in an std::vector, so I would propose to follow this format from the very beginning

    • Please register or sign in to reply
  • 37 // -------------------------------------------------------------------------
    38 CbmMustFindHits::CbmMustFindHits() : FairTask("MustFindHits", 1) {}
    39
    40 // ----- Destructor ----------------------------------------------------
    41 CbmMustFindHits::~CbmMustFindHits() {}
    42 // -------------------------------------------------------------------------
    43
    44 // ----- Task execution ------------------------------------------------
    45 void CbmMustFindHits::Exec(Option_t* /*opt*/)
    46 {
    47 assert(fDigiArray);
    48 fHitArray->Delete();
    49
    50 // LOG(info) << "got " << fDigiArray->GetEntriesFast() << " points";
    51 for (Int_t iDigi = 0; iDigi < fDigiArray->GetEntriesFast(); iDigi++) {
    52 const CbmMustDigi* digi = (const CbmMustDigi*) fDigiArray->At(iDigi);
    • Please, use a C++-style casting formats. Here you know, that fDigiArray contains exactly a CbmMustDigi object, so you could just use a static_cast:

      const auto* digi = static_cast<const CbmMustDigi*>(fDigiArray->At(iDigi));

      A remark to my comment below: in such case you would not need a check for the nullptr anymore

      Edited by Sergei Zharko
    • Please register or sign in to reply
  • 74
    75 // ----- Initialisation -----------------------------------------------
    76 InitStatus CbmMustFindHits::Init()
    77 {
    78 FairRootManager* ioman = FairRootManager::Instance();
    79 if (!ioman) {
    80 std::cout << "-E- CbmMustFindHits::Init: " /// todo replace with logger!
    81 << "RootManager not instantiated!" << std::endl;
    82 return kFATAL;
    83 }
    84
    85 fDigiArray = static_cast<TClonesArray*>(ioman->GetObject("MUSTDigi"));
    86 if (!fDigiArray) {
    87 std::cout << "-W- CbmMustFindHits::Init: "
    88 << "No Digi array!" << std::endl;
    89 return kERROR;
    • I would replace kERROR with kFATAL: if you require the FairRunAna to find the MUST hits, but you don't have the corresponding digis, there is no sense of starting the reconstruction

    • Please register or sign in to reply
  • @r.karabowicz, the MR request looks impressive. To integrate it into the CbmRoot you should consider the data formats, used for other detectors. I would suggest taking a look at the corresponding classes for STS (CbmStsPoint, CbmStsDigi, CbmStsHit) and for TRD (CbmTrdPoint, CbmTrdDigi, CbmTrdHit).

    Also, if MUST is supposed to be a part of tracking, please provide a tracking interface class (CbmMustTrackingInterface) and add it here: https://git.cbm.gsi.de/computing/cbmroot/-/blob/master/reco/L1/CbmTrackingDetectorInterfaceInit.h?ref_type=heads. The reference classes are CbmStsTrackingInterface, CbmTofTrackingInterface and CbmTrdTrackingInteface

    Edited by Sergei Zharko
  • added 2 commits

    • 85482326 - chore(must): Apply format
    • 783fa029 - del(must): Remove obsolete classes MapCreator and SingleStraw

    Compare with previous version

  • Dear @f.uhlig, @v.friese, @p.-a.loizeau,

    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.

  • Radoslaw Karabowicz mentioned in merge request !2040

    mentioned in merge request !2040

  • I have decided to break this MR into smaller chunks. The simulation part is under !2040 I have implemented the requested changes.

  • Please register or sign in to reply
    Loading