[MUONDigi] Adding a digitizer of IDEA Muon system. by mahmoudali2 · Pull Request #24 · key4hep/k4RecTracker

mahmoudali2 · 2024-07-25T15:56:56Z

BEGINRELEASENOTES

Defining a first draft for the muon system digitizer, which can do the following tasks:

Smearing the simHits in 2 Dimensions in the local coordinates of the chamber and applying the changes to the global coordinates of the Hits.
Emulating the efficiency of the chamber.
The generated root file will have branches to validate the smearing (Differences between DigiHits and SimHits in global coordinates).

ENDRELEASENOTES

Add missing GaudiAlgLib

… MUONDigi

BrieucF

Very nice Mahmoud! Here are already a few questions/comments

BrieucF · 2024-07-25T18:49:19Z

MUONdigi/dataFormatExtension/muonSystemHit.yaml

+
+datatypes:
+
+  extension::MCRecoMuonSystemDigiAssociation:


Why not using MCRecoTrackerAssociation https://github.com/key4hep/EDM4hep/blob/main/edm4hep.yaml#L634 ?

Can it be used for linking between digi and sim? I fount it relates between reco and sim..
On another hand I'd like to see if there is an association also to store the validation data (e.g: simDigipPositionDifference)

BrieucF · 2024-07-25T18:51:50Z

MUONdigi/include/MUONsimpleDigitizer.h

+  // Detector efficiency
+  FloatProperty m_efficiency{this, "efficiency", 0.95, "Detector efficiency"};
+
+  // Declaration of validation distribution


Can you add a flag, set to false by default, to produce these collections? See e.g. https://github.com/key4hep/k4RecTracker/blob/master/DCHdigi/include/DCHsimpleDigitizerExtendedEdm.h#L78 (the branch will unfortunately still be there I am afraid)

would a set of output histograms be enough?

BrieucF · 2024-07-25T18:52:27Z

MUONdigi/include/MUONsimpleDigitizer.h

+  DataHandle<podio::UserDataCollection<double>> m_simDigiDifferenceZ{"simDigiDifferenceZ", Gaudi::DataHandle::Writer, this}; // mm
+
+  // Random Number Service
+  IRndmGenSvc* m_randSvc;


@jmcarcell let us know if a consensus was reached on which random number generator we should use

I am not sure if this the best example, but at least it works. The generator must be defined as thread local and distributions as mutable [link].
Later, for each event, remember to reset the internal state of the pseudorandom generator(s) that you may use, in a reproducible manner using the UID service [link]

BrieucF · 2024-07-25T18:56:01Z

MUONdigi/include/MUONsimpleDigitizer.h

+  // y resolution in mm
+  FloatProperty m_y_resolution{this, "yResolution", 1.0, "Spatial resolution in the y direction [mm]"};
+
+  // z resolution in mm


I don't understand why we need 3 numbers here. Since you do the smearing in local coordinates (as it should), 2 numbers should be sufficient no? OR at least we should put ine by default to 0, otherwise we really smear in the 3 dimensions.

atolosadelgado

Hi @mahmoudali2

thank you for this PR. I made few comments, but we can talk offline further if you want :)

atolosadelgado · 2025-01-27T09:07:52Z

MUONdigi/include/MUONsimpleDigitizer.h

+// EDM4HEP
+#include "edm4hep/SimTrackerHitCollection.h"
+#include "edm4hep/TrackCollection.h"
+#if __has_include("edm4hep/TrackerHit3DCollection.h")


maybe a version check would be enough? @jmcarcell can you help here?

atolosadelgado · 2025-01-27T09:08:16Z

MUONdigi/include/MUONsimpleDigitizer.h

+#include "edm4hep/TrackerHitCollection.h"
+
+namespace edm4hep {
+  using TrackerHit3DCollection = edm4hep::TrackerHitCollection;


version check as well?

atolosadelgado · 2025-01-27T09:09:21Z

MUONdigi/include/MUONsimpleDigitizer.h

+/** @class MUONsimpleDigitizer
+ *
+ *  Algorithm for creating digitized Muon system hits (still based on edm4hep::TrackerHit3D) from edm4hep::SimTrackerHit.
+ *  You have to specify the expected resolution in z and in xy.


does it perform only position smearing or also detection efficiency with a russian roulette method?

atolosadelgado · 2025-01-27T09:10:48Z

MUONdigi/include/MUONsimpleDigitizer.h

+ *
+ */
+
+class MUONsimpleDigitizer : public GaudiAlgorithm {


I guess the GaudiAlgorithm class will be maintained for some time, @jmcarcell do you think it is worth to move this algorithm to a Gaudi functional?

atolosadelgado · 2025-01-27T09:12:30Z

MUONdigi/include/MUONsimpleDigitizer.h

+  // Detector efficiency
+  FloatProperty m_efficiency{this, "efficiency", 0.95, "Detector efficiency"};
+
+  // Declaration of validation distribution


would a set of output histograms be enough?

atolosadelgado · 2025-01-27T09:16:19Z

MUONdigi/include/MUONsimpleDigitizer.h

+  DataHandle<podio::UserDataCollection<double>> m_simDigiDifferenceZ{"simDigiDifferenceZ", Gaudi::DataHandle::Writer, this}; // mm
+
+  // Random Number Service
+  IRndmGenSvc* m_randSvc;


I am not sure if this the best example, but at least it works. The generator must be defined as thread local and distributions as mutable [link].
Later, for each event, remember to reset the internal state of the pseudorandom generator(s) that you may use, in a reproducible manner using the UID service [link]

atolosadelgado · 2025-01-27T09:17:48Z

MUONdigi/src/MUONsimpleDigitizer.cpp

+  auto simDigiDifferenceZ = m_simDigiDifferenceZ.createAndPut();
+
+  for (const auto& input_sim_hit : *input_sim_hits) {
+    // Apply efficiency


please add this efficiency feature in the doxigen description in the class

atolosadelgado · 2025-01-27T10:02:26Z

MUONdigi/src/MUONsimpleDigitizer.cpp

+    debug() << "Digitisation of " << m_readoutName << ", cellID: " << cellID << endmsg;
+    //  auto                           cellDetElement = m_volman.lookupDetElement(cellID);
+
+    const auto& stripsTransformMatrix = m_volman.lookupVolumePlacement(cellID).matrix();


maybe using the segmentation position and cellID functions [link] give the same functionality without the overhead of the volume manager?

mahmoudali2 added 12 commits February 20, 2024 18:04

Adding the first draft for the Muon system digitizer

a53ed41

Merge branch 'master' of github.com:mahmoudali2/k4RecTracker

b1b7691

Merge branch 'key4hep:master' into master

e1030a0

Fix muon digitizer

01eab53

Merge branch 'key4hep:master' into master

f2b548b

Changing Muon Hits collection into cartisian

36fc063

Merge branch 'master' of https://github.com/mahmoudali2/k4RecTracker

6028615

Add missing GaudiAlgLib

Merge branch 'key4hep:master' into master

783a359

Updating muon-system readout

d013b40

Merge branch 'master' with the local code

09a55d8

Defining detector efficiency and adding Association extension for the…

a5a4ace

… MUONDigi

Adding validation for the MUON digitizer

7bdd29f

BrieucF reviewed Jul 25, 2024

View reviewed changes

mahmoudali2 added 2 commits October 3, 2024 15:28

Merge branch 'key4hep:master' into master

d1e094d

Merge branch 'key4hep:master' into master

2ca04ca

atolosadelgado reviewed Jan 27, 2025

View reviewed changes

jmcarcell mentioned this pull request Feb 3, 2025

k4rectracker: add v0.4.0 and a dependency on pandorasdk key4hep/key4hep-spack#705

Merged


		datatypes:

		extension::MCRecoMuonSystemDigiAssociation:

Conversation

mahmoudali2 commented Jul 25, 2024

Uh oh!

BrieucF left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atolosadelgado left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants