[DEE] Add drift times from depth calibration already in `MSubModuleChargeTransport` by fhagemann · Pull Request #165 · cositools/nuclearizer

fhagemann · 2026-06-15T21:24:14Z

This PR addresses issue #111, and is a first step to add realistic nearest-neighbor timing to the DEE.

TLDR: I'm moving applying the depth splines + stretch + offset from MSubModuleDepthReadout to MSubModuleChargeTransport, while keeping the smearing of timing values in MSubModuleDepthReadout,
while adding an extra field to MDEEStripHit to keep track of the expected time at which the fast-shaper signal peaks.

Some background information:

Scenario 1: an event where all charge is collected on one strip, the main strip.
- The fast-shaper signal of the main strip is unipolar, with a very pronounced peak. We use that for timing and depth calibration.
- The nearest-neighbor (spectator) strips typically feature a bipolar fast-shaper. The fast-shaper signal for these strips features a peak-dip structure that peaks ~50ns earlier than the main strip (for most of the detector depth, ignoring events VERY close to the HV and LV sides)
Scenario 2: an event where the charge is shared between two neighboring strips.
- Here, the fast-shaper of both collecting strips is a superposition of the above-mentioned unipolar (direct) signal, and the bipolar (transient) signal, resulting both of them to peak earlier compared the direct signal in scenario 1.

To first order, the relation between when the fast-shaper signal peaks and how much energy it records is pretty linear, i.e. t_drift = t_unipolar - 50ns * (1 - E_recorded / E_event)

The drift times for the main strips can be determined from the simulated electron and hole drift times, by applying stretch and offset from the depth calibration. For main strips with no charge sharing, the "time until fast shaper peaks" should be similar to those drift times.

In order to include simulated drift times in the DEE, we need to know which strips are main strips / NN strips and if there was charge sharing present or not, because for these the "time until fast shaper peaks" is not identical to the charge drift times, and we need additional information (basically the depth, Z and if/how much energy was charge-shared), all of which is only available in MSubModuleChargeTransport. Therefore, the determination of charge drift times + stretch + offset is moved to MSubModuleChargeTransport, and for now we apply nearest-neighbor timing by using the simplistic linear relationship t_drift = t_unipolar - 50ns * (1 - E_recorded / E_event).

…duleChargeTransport`

fhagemann · 2026-06-15T21:26:01Z

+  //! The drift time in ns since creation of event
+  double m_DriftTime;


Maybe the name m_DriftTime might not be correct.
It is essentially the time between the creation of the electron-hole pairs until the fast shaper on a non-charge-sharing main strip would peak, minus some offset accounting for the delay in peaking...
For non-charge-sharing main strips, this value should correspond to the drift time, but it's not the case for charge-sharing strips or nearest neighbors. Suggestions welcome ^^

How about m_TimetoFastShaperPeak to make it very explicit? I agree m_DriftTime might be a little misleading for the non main strips.

Or m_FastPeakTime?

I like m_FastPeakTime, because it keeps the name reasonably short while still capturing what it describes. Thanks for this suggestion!

fhagemann · 2026-06-15T21:28:34Z

    while (IterLV2 != LVHits.end()) {
      if (IterLV1->m_ROE == IterLV2->m_ROE) {
        IterLV1->m_Energy += IterLV2->m_Energy;
+        IterLV1->m_DriftTime = IterLV1->m_Energy > IterLV2->m_Energy ? IterLV1->m_DriftTime : IterLV2->m_DriftTime;


The way we combine timing values when merging strip hits might not be too accurate. This works well for hits that were initially close together, but might be wrong for events that are spatially more separated.
If the fast-shaper direct signals would feature two separated peaks (usually more than 50ns apart), then it would depend on if the first peak would be above threshold etc. ...
Maybe this merging can only happen during MSubModuleDepthReadout, when we know what the fast threshold is?

Good question as to where these gets combined. Looking at our current flow, I'm wondering if it then makes sense to bring MSubModuleDepthReadout immediate after MSubModuleChargeTransport? That allows you to apply the fast threshold and properly combine the m_DriftTimes without keeping the striphit drift times separate throughout all of the energy-focused modules in between? There may be an issues with having the Depth Readout come before the StripTrigger module, but it's not immediately jumping out at me...

This can also be kept as a TODO for now just to get something working for the majority of hits.

Maybe we can even combine MSubModuleStripReadout and MSubModuleDepthReadout, and they are doing the same thing for energy and timing (smearing, inverse ecal/taccal, applying thresholds)?

Not a bad idea. Let's keep that as something to explore when you address the proper combination, but leave this as it is for the current PR.

fhagemann · 2026-06-15T21:30:06Z

    // calculate σ and η, assuming t = z / v = z / (µ * E)
-    double Sigma = std::sqrt(2.0 * kB * Temperature * ΔZ / (ElementaryCharge * MeanElectricField)); // in cm
-    double Eta   = std::cbrt(std::pow(InitialChargeCloudSize, 3) + 3.0 * N * ElementaryCharge * ΔZ / (4.0 * TMath::Pi() * Epsilon0 * EpsilonR * MeanElectricField)); // in cm
+    double Sigma = std::sqrt(2.0 * kB * Temperature * DeltaZ / (ElementaryCharge * MeanElectricField)); // in cm
+    double Eta   = std::cbrt(std::pow(InitialChargeCloudSize, 3) + 3.0 * N * ElementaryCharge * DeltaZ / (4.0 * TMath::Pi() * Epsilon0 * EpsilonR * MeanElectricField)); // in cm


Here, we could also think about updating t = z / v = z / (µ * E) to using the simulated drift times. Two caveats though: the files have those simulated drift times already convolved with electronics, and this might be quite sensitive to how you apply the depth-calibration offset (e.g. adding it to the hole drift times or subtracting it from the electron hole drift times)

Interesting idea. It's nice to think about everything being self-consistent. But I wondering if there are any hints that t = z / v = z / (µ * E) is not accurate enough? Can the simulations be run without electronics and still be self-consistent with the drift time sims you're using above? Maybe this is a nice-to-have improvement for the future...

There are some hints that t = z / v = z / (µ * E) might not be accurate enough:

E (electric field) not being homogeneous in the detector due to the non-zero impurity density. The electric field strength increases linearly towards the HV side, which makes this t non-linear in z

µ (charge carrier mobility) is now set to a constant value, but is actually temperature dependent, dependent on E (saturates at higher electric fields), and technically also on the crystal axis orientation.

So, in that case, the t ~ z relationship might not necessarily hold.

Okay, again, let's think of this as something to work towards, but leave it as is for now.

fhagemann · 2026-06-15T21:31:34Z

    MainSH.m_ROE.SetStripID(ID);
    MainSH.m_OppositeStripID = OppositeStripID;
    MainSH.m_Energy = MainStripEnergy;
+    MainSH.m_DriftTime = DriftTime - 50 * (1 - MainStripEnergy / SH.m_SimulatedEnergy);


This is how to account for the difference in timing with respect to having a non-charge-sharing main strip. I know that this implementation is very simplistic, but captures what we see well for most events that are not close to the HV/LV sides.

We have one of our undergrads (Isidro) looking into this is more detail.
I would push going for higher-order implementations in a future PR.

Yes, agreed. Leave this simple case for now and add a TODO.

fhagemann · 2026-06-15T21:35:09Z

+      if (g_Verbosity >= c_Warning) {
+        cout << "No depth calibration coefficients for pixel in DetID " << DetID << " HV " << (isLV ? OppositeStripID : ID) << " LV " << (isLV ? ID : OppositeStripID) << endl;
+      }
+      DriftTime = 1e10;


If we don't have stretch and offset available, I'm setting the drift times unreasonably high. Using a pixel-based inverse depth calibration, this can result in:

if there is a HV strip without fast timing, all events collected on that strip would also give unreasonable drift times on the LV strips.

If we were to use these charge drift times for diffusion/self-repulsion, this would screw the calculation up and result in gigantic amounts of charge sharing.

This would be avoided by transitioning from pixel-based to strip-based inverse depth calibration (see #158).

I'm also open to suggestions how to deal with events with no depth calibration.

Hmm, good question. If we're sold on moving towards a strip-based inverse depth, then I guess whatever you implement here is temporary. But what about taking the average of some neighboring pixels as a placeholder for the stretch and offset? It's not perfect, but it's an okay guess. Note that once this goes through the forward pipeline, this pixel still won't have a valid depth calibration so it'll result in an error anyways, right?

Yes, going towards strip-based inverse depth calibration (#158) was motivated exactly by this issue.
Taking an average for the offset might not be the best estimate (we've seen offsets between neighboring pixels jump, especially when the TAC cal was also jumping), but it's probably the best we can do for now + keep a TODO on revisiting this once we tackle #158.

Although the strip hits without depth calibration coefficients might get no valid depth calibration in the forward pipeline, this will also mess up the timing values for the neighboring strips, which have valid depth calibration. Let me think about this a bit more and how to tackle this.

fhagemann · 2026-06-15T21:37:30Z

-          if (m_ApplyTimingResolutionCalibration == true) {
+    if (SH.m_IsGuardRing == false) {
+      unsigned int StripID = SH.m_ROE.GetStripID();
+      if (SH.m_DriftTime > -200.0){


This is an arbitrary number right now.
This number CAN be negative (if the charge drift time is shorter than 50ns, and the neighbor did not charge share, we might get e.g. 40ns for the main strip and -10ns for its nearest neighbor).

Ah, this answers my question from above. Maybe just add a comment in the code as to why you choose 200 ns as a placeholder.

Why do we need this cut at all? Is this to filter out the unphysical large drift times when we don't have a valid depth calibrations (I thought those would be positive?)?

When I played around with other parameterizations for the nearest neighbor timing values, there were some cases in which I was getting very negative values. But you are right: we should also safe-guard against the large positive default value (1e10) for strips without depth calibration here.

fhagemann · 2026-06-23T00:32:32Z

+    // if (NNLeftStripEnergy > IonizationEnergy) {
      MDEEStripHit NNLeftSH = SH;
-      NNLeftSH.m_Energy = NNLeftStripEnergy;
+      NNLeftSH.m_Energy = std::max(NNLeftStripEnergy, 0.0);


Note to myself: Looks like this std::max to avoid tiny negative energies (e.g. -1e-12) and therefore errors in the inverse ecal (TF1 could not find root in interval [0,16383]) is not needed if the InitialChargeCloudSize is set to something positive instead of 0.0 right now (e.g. 0.01 for 10µm initial charge cloud size)

In anticipation of PR cositools#165

ckierans

Sorry this took so long to get through, @fhagemann! Nice rework here. I think things are mostly pretty clear, but I do have a few questions. Nothing major.

ckierans · 2026-07-28T02:12:42Z

+  //! The drift time in ns since creation of event
+  double m_DriftTime;


How about m_TimetoFastShaperPeak to make it very explicit? I agree m_DriftTime might be a little misleading for the non main strips.

ckierans · 2026-07-28T02:36:38Z

+  DepthCalibration.SetCoeffsFileName(m_DepthCoefficientsFileName);
+  if (DepthCalibration.LoadCoeffsFile(m_DepthCoefficientsFileName) == true) {
+    // Copy depth calibration coefficients
+    m_Coeffs = DepthCalibration.GetCoeffs();


Comparing with the previous MSubModuleDepthReadout.cxx, I see you've removed the m_Coeffs_Energy variable. I'm trying to track down what happened to that after PR #152. Are you still reliant on that variable? Is so, where are you calling it?

I am still reliant on that variable but not here in MSubModuleChargeTransport, but in MSubModuleDepthReadout.

The depth coefficients file carry pixel information on stretch, offset, and timing resolution. m_CoeffsEnergy is used to scale the timing resolution to the strip hit energy, but irrelevant for stretch & offset.

In this PR, I'm only moving the stretch & offset part to MSubModuleChargeTransport, while keeping the timing resolution (and m_CoeffsEnergy) in MSubModuleDepthReadout (specifically in line 87):
https://github.com/fhagemann/nuclearizer/blob/bbd325ac35b957c7f6873c845c1c916bf4bdaf60/src/MSubModuleDepthReadout.cxx#L82-L87

ckierans · 2026-07-28T02:40:10Z

+  //! The drift time in ns since creation of event
+  double m_DriftTime;


Or m_FastPeakTime?

ckierans · 2026-07-29T18:31:28Z

      if (ID > 0) {
        NNLeftSH.m_ROE.SetStripID(ID - 1);
        NNLeftSH.m_IsGuardRing = false;
-        // NNLeftSH.m_IsNearestNeighbor = true;


Will this flag then be added in the StripTrigger module instead?

This is the plan. We only know after combining strip hits if a strip exceeded the slow threshold or not, and becomes a triggered strip or a "nearest neighbor"

Plus, we don't know what the slow threshold is, which might only be applied later in MSubModuleStripReadout

ckierans · 2026-07-29T18:51:49Z

-          if (m_ApplyTimingResolutionCalibration == true) {
+    if (SH.m_IsGuardRing == false) {
+      unsigned int StripID = SH.m_ROE.GetStripID();
+      if (SH.m_DriftTime > -200.0 ) {


Where does the -200 come from?

This is an arbitrary threshold I chose based on the following:

m_DriftTime for the main strip can (in principle) be as tiny as 0ns (or even slightly negative of certain values of the offset determined in the depth calibration)

nearest-neighbor strips, in this current implementation, can have values that are 50ns smaller

With this -200, I want to keep all strip hits with "reasonable" timing (NN strips etc., offset values that we might expect), while getting rid of those with "unreasonable" timing

ckierans · 2026-07-29T18:56:52Z

+          cout<<"MSubModuleDepthReadout::AnalyzeEvent: Unphysical drift time."<<endl;
+        }
+        SH.m_TAC = 0;
+        SH.m_HasTriggered = false;


Shouldn't this be m_HasFastTiming == false instead of m_HasTriggered?

Good point, I will update this.

ckierans · 2026-07-29T18:59:33Z

-          if (m_ApplyTimingResolutionCalibration == true) {
+    if (SH.m_IsGuardRing == false) {
+      unsigned int StripID = SH.m_ROE.GetStripID();
+      if (SH.m_DriftTime > -200.0){


Ah, this answers my question from above. Maybe just add a comment in the code as to why you choose 200 ns as a placeholder.

Why do we need this cut at all? Is this to filter out the unphysical large drift times when we don't have a valid depth calibrations (I thought those would be positive?)?

ckierans · 2026-07-29T19:00:55Z

+          cout<<"MSubModuleDepthReadout::AnalyzeEvent: Unphysical drift time."<<endl;
        }
+        SH.m_TAC = 0;
+        SH.m_HasTriggered = false;


Same question as above about this flag. I'd need to go back to my notes/git convos, but I thought m_HasTriggered is about the slow circuit?

Turns out MDEEStripHit did not have the flag m_HasFastTiming, so I added that now in bb05fb2.

fhagemann · 2026-07-30T19:13:42Z

Ok, I have incorporated all changes and pushed those to this PR.

fhagemann · 2026-07-30T19:15:16Z

+      const vector<double>& Coeffs = it->second;
+      double Stretch = Coeffs[0];
+      double Offset = isLV ? Coeffs[1] : 0.0;
+      FastPeakTime = (DriftTimeSpline->Eval(Z) + Offset) * Stretch;


Small note: I did not rename DriftTimeSpline, because these really are drift times (plus electronics), but then changed everything else to FastPeakTime, where then timing effects of charge sharing or early peaking of nearest-neighbor strips will play an effect.

parshadkp

Hi @fhagemann, sorry this is super late but I was looking through the changes and found this one potential issue. Let me know what you think.

parshadkp · 2026-08-06T14:49:36Z

-        // NNRightSH.m_IsNearestNeighbor = true;
-      } else {
-        NNRightSH.m_ROE.SetStripID(NStrips);
-        NNRightSH.m_IsGuardRing = true;


Sorry, this is super late but does this assign a GR hit even if there was no energy transported to the GR? Is this okay for a GR veto?

Yes, it does, and it might trigger a GR veto in its current version.
But, by applying GR energy thresholds later in MSubModuleStripTrigger or MSubModuleStripReadout (I think this is not implemented right now), these events would then be filtered out.

That makes sense. Once we have the thresholds implemented then I can check if the number of GR vetoes goes down.

fhagemann added 3 commits June 15, 2026 12:51

Add m_DriftTime to MDEEStripHit

9407f5b

Move calculating drift times from MSubModuleDepthReadout to `MSubMo…

fc5903b

…duleChargeTransport`

Always record NN in the DEE (to keep information about NN timing)

bbd325a

fhagemann added the DEE Development related to the detector effects engine label Jun 15, 2026

fhagemann commented Jun 15, 2026

View reviewed changes

fhagemann marked this pull request as ready for review June 15, 2026 21:37

fhagemann commented Jun 23, 2026

View reviewed changes

fhagemann mentioned this pull request Jun 23, 2026

[DEE] Update inverse depth calibration from pixel-based to strip-based #158

Draft

parshadkp added a commit to parshadkp/nuclearizer_parshad_fork that referenced this pull request Jun 29, 2026

Remove nearest neighbor hack

af177fc

In anticipation of PR cositools#165

parshadkp mentioned this pull request Jun 29, 2026

Remove nearest neighbor hack #173

Open

ckierans reviewed Jul 29, 2026

View reviewed changes

fhagemann added 5 commits July 30, 2026 12:02

Rename m_DriftTime to m_FastPeakTime

06e6919

Add TODOs

075605c

Add comment on m_FastPeakTime cut values in MSubModuleDepthReadout

2e6fa67

Add m_HasFastTiming to MDEEStripHit

bb05fb2

Clean up

9471069

fhagemann requested a review from ckierans July 30, 2026 19:12

fhagemann commented Jul 30, 2026

View reviewed changes

ckierans merged commit d5ab14d into cositools:develop/em Aug 4, 2026
1 check passed

fhagemann deleted the feature/NN branch August 4, 2026 00:12

fhagemann mentioned this pull request Aug 4, 2026

ADD: Unit tests for MDEEStripHit #185

Merged

parshadkp reviewed Aug 6, 2026

View reviewed changes

fhagemann mentioned this pull request Aug 18, 2026

Combining strip hits in the DEE #111

Closed

		//! The drift time in ns since creation of event
		double m_DriftTime;

Uh oh!

Conversation

fhagemann commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fhagemann Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fhagemann Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

fhagemann Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ckierans 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!

Uh oh!

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!

fhagemann commented Jun 15, 2026 •

edited

Loading

fhagemann Jul 29, 2026 •

edited

Loading

fhagemann Jun 15, 2026 •

edited

Loading

fhagemann Jun 23, 2026 •

edited

Loading