Digitization: Threshold discrimination applied too early

[ybedfer]

In SiliconTrackerDigi.cc

1. A threshold on EDep is applied first to input SimTrackerHits.

     if (sim_hit.getEDep() < m_cfg.threshold) {
         debug("  edep is below threshold of {:.2f} [keV]", m_cfg.threshold / dd4hep::keV);
         continue;
     }
   

2. EDep is then accumulated to produce the total EDep per cell.

I interpret this as a simulation of the discrimination threshold of the FE electronics. If I am correct, then steps 1 and 2 should be swapped.

I put below a patch to implement the fix above. Should I make a PR of it? (and of similar patch to MPGDTrackerDigi.cc).

diff --git a/src/algorithms/digi/SiliconTrackerDigi.cc b/src/algorithms/digi/SiliconTrackerDigi.cc
index b5f21216..26ec3565 100644
--- a/src/algorithms/digi/SiliconTrackerDigi.cc
+++ b/src/algorithms/digi/SiliconTrackerDigi.cc
@@ -60,8 +60,2 @@ void SiliconTrackerDigi::process(
 
-
-        if (sim_hit.getEDep() < m_cfg.threshold) {
-            debug("  edep is below threshold of {:.2f} [keV]", m_cfg.threshold / dd4hep::keV);
-            continue;
-        }
-
         if (cell_hit_map.count(sim_hit.getCellID()) == 0) {
@@ -89,3 +83,8 @@ void SiliconTrackerDigi::process(
     for (auto item : cell_hit_map) {
-        raw_hits->push_back(item.second);
+        auto &raw_hit = item.second;
+        if (raw_hit.getCharge() < m_cfg.threshold * 1e6) {
+            debug("  charge is below threshold of {:.2f} [keV]", m_cfg.threshold / dd4hep::keV);
+            continue;
+        }
+        raw_hits->push_back(raw_hit);
 
@@ -96,3 +95,3 @@ void SiliconTrackerDigi::process(
             hitassoc.setWeight(1.0);
-            hitassoc.setRawHit(item.second);
+            hitassoc.setRawHit(raw_hit);
 #if EDM4EIC_VERSION_MAJOR >= 6

[veprbl]

Please, submit a PR.

[simonge]

Unless there is some specific example that this fixes, I don't think this change is worth making to the simple digitization. Its hard to tell if it will result in a more or less realistic reconstruction, particularly based on how the time of the digitized hit is currently being set to the earliest simulation hit

It is very rare cells in our simulation with event based MC samples will ever see more than a single hit. When expanding to time frame samples, the separation of hits over time is important rather than squashing them into a single hit, at which point we need to move to a more detailed description of the FE electronics anyway.

[veprbl]

I assume, the different steps will be summed by DD4hep itself. Multiple hits will appear, for example, when a conversion happens in the cell itself.

[ybedfer]

I do not insist. Yet...
...It could be seen as a precaution, so that it is not forgotten to enforce the correct order in the eventual sophisticated digitization.
...What about synchrotron radiation? Could it be that that produces numerous low EDep's, which accumulate.

[simonge]

The problem is that both the current implementation and the proposed change are both "wrong". The current method stops spurious negligible hits from setting an early time for the hit while allowing the example @veprbl gave to include energy from both contributions of a conversion (assuming the threshold isn't so low their individual contributions are expected to be lost, but then the threshold is probably too high). The compromise may be to sum over all of the hits but swap the time to the time of the maximum charge hit, but again that can be wrong for other reasons.

If we're frequently getting multiple synchrotron radiation hits in the same cell which sum up to a signal we have bigger problems and these need to be studied with a proper realistic digitization scheme.

Basically my opinion is that this algorithm has served its purpose and should be phased out, not patched. Others with more clout might hold other opinions.

[veprbl]

I would agree that we should not put excessive effort into a small correction. If we have a PR, it will at least run benchmarks and we can see how significant it is.

@simonge Do you have a technical requirements or a TODO list in mind for the proper digitization scheme?

[ybedfer]

The compromise may be to sum over all of the hits but swap the time to the time of the maximum charge hit, but again that can be wrong for other reasons.

I can implement this. And then submit a PR, for the purpose of running benchmarks.

[simonge]

@simonge Do you have a technical requirements or a TODO list in mind for the proper digitization scheme?

I do have a step by step idea of how it could be implemented which I'd be happy to try and prepare some slides on for a Reconstruction meeting

[kkauder]

FYI, I'm closely following this PR. Once the tech details are hashed out, I plan to revive the logic of the non-merged #1359

[ShujieL]

I agree with Simon that the silicon digi algorithm served the purpose for now, at least for the silicon trackers. That's been said, I am not against this change if there is an immediate use case. @kkauder @ybedfer opinions?

[ybedfer]

Hello again,

I do have a step by step idea of how it could be implemented which I'd be happy to try and prepare some slides on for a Reconstruction meeting

Is the this presentation already scheduled? If indeed, I would like to be informed...

And in any case, I wait until it's done and discussed before taking any action.