FIX: Assign correct DetIDs to strip detectors when loading detector dimensions

[fhagemann]

Currently, with the massmodel-cosi-payload (and probably every other massmodel using more than just 1 detector), when loading the detector dimensions for the MModuleDepthCalibration and MSubModuleChargeTransport, we assign DetIDs to all detectors in the DetectorList.
However, this DetectorList also includes the guard ring detectors.
Thus, the germanium strip detectors are only assigned even DetIDs in increments of 2:

Found detector GeWafer_Q0_L0 corresponding to DetID=0.
Detector thickness: 1.583
Number of X strips: 64
Number of Y strips: 64
X strip pitch: 0.1162
Y strip pitch: 0.1162
Found detector GeWafer_Q0_L1 corresponding to DetID=2.
Detector thickness: 1.55
Number of X strips: 64
Number of Y strips: 64
X strip pitch: 0.1162
Y strip pitch: 0.1162
Found detector GeWafer_Q0_L2 corresponding to DetID=4.
Detector thickness: 1.583
Number of X strips: 64
Number of Y strips: 64
X strip pitch: 0.1162
Y strip pitch: 0.1162
Found detector GeWafer_Q0_L3 corresponding to DetID=6.
Detector thickness: 1.568
Number of X strips: 64
Number of Y strips: 64
X strip pitch: 0.1162
Y strip pitch: 0.1162
...

In this PR, we only increment the DetID once we actually have found a "Strip3D" detector, to avoid incrementing the DetID also for the Simple guard ring detectors.

Found detector GeWafer_Q0_L0 corresponding to DetID=0.
Detector thickness: 1.583
Number of X strips: 64
Number of Y strips: 64
X strip pitch: 0.1162
Y strip pitch: 0.1162
Found detector GeWafer_Q0_L1 corresponding to DetID=1.
Detector thickness: 1.55
Number of X strips: 64
Number of Y strips: 64
X strip pitch: 0.1162
Y strip pitch: 0.1162
Found detector GeWafer_Q0_L2 corresponding to DetID=2.
Detector thickness: 1.583
Number of X strips: 64
Number of Y strips: 64
X strip pitch: 0.1162
Y strip pitch: 0.1162
Found detector GeWafer_Q0_L3 corresponding to DetID=3.
Detector thickness: 1.568
Number of X strips: 64
Number of Y strips: 64
X strip pitch: 0.1162
Y strip pitch: 0.1162
...

I'm also keeping the m_UCSDOverwrite to assign a DetID of 11

[fhagemann]

Note: this does not affect if events in the guard ring detector get read into the DEE.
In the DEE intake, we determine the DetID from the NAME of the sensitive detector, that should either be GeD_X or GuardRingDetector_GeD_X where X is the nuclearizer detector ID.

nuclearizer/src/MSubModuleDEEIntake.cxx

Lines 144 to 147 in 1a1b3ee

	if (DetectorName.BeginsWith("GeD") == true || DetectorName.BeginsWith("GuardRing") == true) {
	DetectorName.RemoveAllInPlace("GuardRingDetector_GeD_"); // Remove prefix GuardRing if existent
	DetectorName.RemoveAllInPlace("GeD_"); // The number after GeD is the COSI detector ID
	int DetectorID = DetectorName.ToInt();

[Copilot]

Pull request overview

This PR fixes DetID assignment for germanium strip (Strip3D) detectors when loading detector dimensions in geometries where the detector list also contains non-strip detectors (e.g., guard rings), ensuring strip detectors receive contiguous DetIDs.

Changes:

• Update MSubModuleChargeTransport to increment DetID only when encountering a Strip3D detector.
• Update MModuleDepthCalibration to increment DetID only when encountering a Strip3D detector, while preserving the UCSD override behavior (DetID = 11).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/MSubModuleChargeTransport.cxx	Adjusts DetID assignment so guard-ring detectors don’t consume DetID values when loading strip detector dimensions.
src/MModuleDepthCalibration.cxx	Adjusts DetID assignment similarly for depth calibration dimension loading, with UCSD override preserved.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ckierans]

Note: this does not affect if events in the guard ring detector get read into the DEE. In the DEE intake, we determine the DetID from the NAME of the sensitive detector, that should either be GeD_X or GuardRingDetector_GeD_X where X is the nuclearizer detector ID.

nuclearizer/src/MSubModuleDEEIntake.cxx

Lines 144 to 147 in 1a1b3ee

	if (DetectorName.BeginsWith("GeD") == true || DetectorName.BeginsWith("GuardRing") == true) {
	DetectorName.RemoveAllInPlace("GuardRingDetector_GeD_"); // Remove prefix GuardRing if existent
	DetectorName.RemoveAllInPlace("GeD_"); // The number after GeD is the COSI detector ID
	int DetectorID = DetectorName.ToInt();

Is there a reason we can't use the same logic to assign the DetID here?

[ckierans]

It looks like there is no way that DetID corresponds to the same number as X in GeD_X, but maybe I'm reading the code wrong. Can we either read the detector number from the mass model name, or do some sanity check to make sure the values are the same?

[fhagemann]

DetID is determined by iterating through all detectors (and skipping the GR detectors in this PR), so if the GeD_X detectors are defined in order (0 through 15), the current code should work.

We can read this from the detector names (right now, we are only checking the names of the SENSITIVE VOLUMES here, which is why we get Found detector GeWafer_Q0_L0 corresponding to DetID=0. instead of Found detector GeD_0 corresponding to DetID=0., but that could be added as sanity check).

Advantage: reading it from the name yields the same mapping as the DEE intake -> hits will always be attributed to the correct detector volume, no matter their "occurence" in the mass model.
Disadvantage: requires detectors in the mass model to strictly follow the naming GeD_X, also outside of the DEE (this code is also used in the depth calibration for example, and might also be relevant for charge trapping, see #184).

[fhagemann]

For example, for the UCSD cage, the detector would HAVE to be called GeD_11 if its detector ID is 11 in the data.

[ckierans]

Sorry, my original comment should have read "It looks like there is no confirmation that DetID corresponds to the same number as X in GeD_X". I agree that if the detectors are listed in order then all should be okay, but we should add a sanity check to be sure.

I'd like to enforce the GeD_X naming for all mass models going forward, but that might make backward compatibility (i.e. applying the DEE to an older mass model) more challenging so if you want to keep this as is with the numerical iterations, let's at least confirm the GeD_X corresponds to DetID X.

[fhagemann]

But then only throw a warning if the GeD_X names do not agree with the DetID, without exiting the computation? Because throwing an error and exiting poses the same issue with the code then not being backward compatible.

[ckierans]

Sorry for the delay in responding. I worry a warning messages might get lost in the long command line output. But if the detector IDs don't match, then that should be a showstopper for most analysis, unless someone knows what they're doing.

What about checking the name of the Geometry itself before enforcing the error? If it's COSI-SMEX-Payload and the numbers don't match then you can exit with an error, if not, then just give a warning? The MGeometry class has a GetName() function, and each geometry has a Name at the top of the .geo.setup file.

[fhagemann]

Like this? 😅

[ckierans]

Yes :)

Looks good!