From c1e4ed529a7846710184637f9c80f749b120e106 Mon Sep 17 00:00:00 2001 From: ChunzhengLab Date: Mon, 21 Apr 2025 21:14:53 +0200 Subject: [PATCH 1/2] Address reviewer comments --- .../ITSMFTSimulation/AlpideSimResponse.h | 2 +- .../include/ITS3Base/SegmentationMosaix.h | 27 ++++------- .../ITS3Simulation/ChipDigitsContainer.h | 15 ++---- .../include/ITS3Simulation/ChipSimResponse.h | 2 +- .../include/ITS3Simulation/DigiParams.h | 8 ++-- .../ITS3Simulation/ITS3DPLDigitizerParam.h | 8 +--- .../ITS3/simulation/src/DigiParams.cxx | 46 ++++++++----------- .../simulation/src/ITS3DPLDigitizerParam.cxx | 7 --- 8 files changed, 43 insertions(+), 72 deletions(-) diff --git a/Detectors/ITSMFT/common/simulation/include/ITSMFTSimulation/AlpideSimResponse.h b/Detectors/ITSMFT/common/simulation/include/ITSMFTSimulation/AlpideSimResponse.h index 25cad6d27b9a5..674202fd1a0d9 100644 --- a/Detectors/ITSMFT/common/simulation/include/ITSMFTSimulation/AlpideSimResponse.h +++ b/Detectors/ITSMFT/common/simulation/include/ITSMFTSimulation/AlpideSimResponse.h @@ -69,7 +69,7 @@ class AlpideRespSimMat private: std::array data; - ClassDefNV(AlpideRespSimMat, 1); + ClassDef(AlpideRespSimMat, 2); }; /* diff --git a/Detectors/Upgrades/ITS3/base/include/ITS3Base/SegmentationMosaix.h b/Detectors/Upgrades/ITS3/base/include/ITS3Base/SegmentationMosaix.h index 782bdfdbba038..fbf9a59e6da4b 100644 --- a/Detectors/Upgrades/ITS3/base/include/ITS3Base/SegmentationMosaix.h +++ b/Detectors/Upgrades/ITS3/base/include/ITS3Base/SegmentationMosaix.h @@ -17,8 +17,6 @@ #ifndef ALICEO2_ITS3_SEGMENTATIONMOSAIX_H_ #define ALICEO2_ITS3_SEGMENTATIONMOSAIX_H_ -#include - #include "MathUtils/Cartesian.h" #include "ITS3Base/SpecsV2.h" @@ -78,7 +76,6 @@ class SegmentationMosaix static constexpr float PitchCol{constants::pixelarray::pixels::mosaix::pitchZ}; static constexpr float PitchRow{constants::pixelarray::pixels::mosaix::pitchX}; static constexpr float SensorLayerThickness{constants::totalThickness}; - static constexpr float NominalYShift{0.0f}; /// Transformation from the curved surface to a flat surface. /// Additionally a shift in the flat coordinates must be applied because @@ -104,7 +101,7 @@ class SegmentationMosaix // the y position is in the silicon volume however we need the chip volume (silicon+metalstack) // this is accounted by a y shift xFlat = WidthH - mRadius * phi; - yFlat = dist - mRadius + NominalYShift; + yFlat = dist - mRadius; } /// Transformation from the flat surface to a curved surface @@ -121,7 +118,7 @@ class SegmentationMosaix { // MUST align the flat surface with the curved surface with the original pixel array is on and account for metal // stack - float dist = yFlat - NominalYShift + mRadius; + float dist = yFlat + mRadius; float phi = (WidthH - xFlat) / mRadius; // the y position is in the chip volume however we need the silicon volume // this is accounted by a -y shift @@ -170,8 +167,7 @@ class SegmentationMosaix /// center of the sensitive volume. /// If iRow and or iCol is outside of the segmentation range a value of -0.5*Dx() /// or -0.5*Dz() is returned. - template - constexpr bool detectorToLocal(T const row, T const col, float& xRow, float& zCol) const noexcept + bool detectorToLocal(float const row, float const col, float& xRow, float& zCol) const noexcept { if (!isValidDet(row, col)) { return false; @@ -182,30 +178,27 @@ class SegmentationMosaix // Same as detectorToLocal w.o. checks. // We position ourself in the middle of the pixel. - template - constexpr void detectorToLocalUnchecked(T const row, T const col, float& xRow, float& zCol) const noexcept + void detectorToLocalUnchecked(float const row, float const col, float& xRow, float& zCol) const noexcept { - xRow = -(static_cast(row) + 0.5f) * PitchRow + WidthH; - zCol = (static_cast(col) + 0.5f) * PitchCol - LengthH; + xRow = -(row + 0.5f) * PitchRow + WidthH; + zCol = (col + 0.5f) * PitchCol - LengthH; } - template - constexpr bool detectorToLocal(T const row, T const col, math_utils::Point3D& loc) const noexcept + bool detectorToLocal(float const row, float const col, math_utils::Point3D& loc) const noexcept { float xRow{0.}, zCol{0.}; if (!detectorToLocal(row, col, xRow, zCol)) { return false; } - loc.SetCoordinates(xRow, NominalYShift, zCol); + loc.SetCoordinates(xRow, 0.0f, zCol); return true; } - template - constexpr void detectorToLocalUnchecked(T const row, T const col, math_utils::Point3D& loc) const noexcept + void detectorToLocalUnchecked(float const row, float const col, math_utils::Point3D& loc) const noexcept { float xRow{0.}, zCol{0.}; detectorToLocalUnchecked(row, col, xRow, zCol); - loc.SetCoordinates(xRow, NominalYShift, zCol); + loc.SetCoordinates(xRow, 0.0f, zCol); } private: diff --git a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h index ae2fd487ece0e..6364a245469b4 100644 --- a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h +++ b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h @@ -19,15 +19,8 @@ #include "ITS3Simulation/DigiParams.h" // ITS3-specific DigiParams interface #include -namespace o2 -{ -namespace its3 -{ +namespace o2::its3 { -// IB uses the Alpide segmentation, -// OB uses the Mosaix segmentation. -using SegmentationIB = SegmentationMosaix; -using SegmentationOB = o2::itsmft::SegmentationAlpide; class ChipDigitsContainer : public o2::itsmft::ChipDigitsContainer { private: @@ -38,6 +31,9 @@ class ChipDigitsContainer : public o2::itsmft::ChipDigitsContainer public: explicit ChipDigitsContainer(UShort_t idx = 0); + using SegmentationIB = SegmentationMosaix; + using SegmentationOB = o2::itsmft::SegmentationAlpide; + /// Returns whether the chip is in the inner barrel (IB) void setChipIndex(UShort_t idx) { @@ -57,7 +53,6 @@ class ChipDigitsContainer : public o2::itsmft::ChipDigitsContainer ClassDefNV(ChipDigitsContainer, 1); }; -} // namespace its3 -} // namespace o2 +} // namespace o2::its3 #endif // ALICEO2_ITS3_CHIPDIGITSCONTAINER_ \ No newline at end of file diff --git a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipSimResponse.h b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipSimResponse.h index d528806920377..f96fde9fb0d55 100644 --- a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipSimResponse.h +++ b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipSimResponse.h @@ -32,7 +32,7 @@ class ChipSimResponse : public o2::itsmft::AlpideSimResponse private: float mRespCentreDep = 0.f; - ClassDefNV(ChipSimResponse, 1); + ClassDef(ChipSimResponse, 1); }; } // namespace its3 diff --git a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h index 751bb84ad036b..74017ddc5ba24 100644 --- a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h +++ b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h @@ -48,8 +48,8 @@ class DigiParams final : public o2::itsmft::DigiParams const o2::itsmft::AlpideSimResponse* getOBSimResponse() const { return mOBSimResponse; } void setOBSimResponse(const o2::itsmft::AlpideSimResponse* response) { mOBSimResponse = response; } - const o2::itsmft::AlpideSimResponse* getIBSimResponse() const { return mIBSimResponse; } - void setIBSimResponse(const o2::itsmft::AlpideSimResponse* response); + o2::its3::ChipSimResponse* getIBSimResponse() const { return mIBSimResponse; } + void setIBSimResponse(o2::its3::ChipSimResponse* response); bool hasResponseFunctions() const { return mIBSimResponse != nullptr && mOBSimResponse != nullptr; } @@ -57,9 +57,9 @@ class DigiParams final : public o2::itsmft::DigiParams private: const o2::itsmft::AlpideSimResponse* mOBSimResponse = nullptr; //!< pointer to external response - const o2::its3::ChipSimResponse* mIBSimResponse = nullptr; //!< pointer to external response + o2::its3::ChipSimResponse* mIBSimResponse = nullptr; //!< pointer to external response - ClassDefNV(DigiParams, 1); + ClassDef(DigiParams, 1); }; } // namespace o2::its3 diff --git a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h index 1134af288f85b..0508b9dea21e1 100644 --- a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h +++ b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h @@ -15,10 +15,7 @@ #include "CommonUtils/ConfigurableParam.h" #include "CommonUtils/ConfigurableParamHelper.h" -namespace o2 -{ -namespace its3 -{ +namespace o2::its3 { struct ITS3DPLDigitizerParam : public o2::conf::ConfigurableParamHelper { float IBNoisePerPixel = 1.e-8; ///< MOSAIX Noise per channel @@ -29,7 +26,6 @@ struct ITS3DPLDigitizerParam : public o2::conf::ConfigurableParamHelper ClassImp(o2::its3::DigiParams); @@ -50,36 +51,29 @@ void DigiParams::setIBChargeThreshold(int v, float frac2Account) void DigiParams::print() const { // print settings - LOGF(info, "ITS3 DigiParams settings:"); - LOGF(info, "Continuous readout : %s", isContinuous() ? "ON" : "OFF"); - LOGF(info, "Readout Frame Length(ns) : %f", getROFrameLength()); - LOGF(info, "Strobe delay (ns) : %f", getStrobeDelay()); - LOGF(info, "Strobe length (ns) : %f", getStrobeLength()); - LOGF(info, "IB Threshold (N electrons) : %d", getIBChargeThreshold()); - LOGF(info, "OB Threshold (N electrons) : %d", getChargeThreshold()); - LOGF(info, "Min N electrons to account for IB : %d", getIBMinChargeToAccount()); - LOGF(info, "Min N electrons to account for OB : %d", getMinChargeToAccount()); - LOGF(info, "Number of charge sharing steps of IB : %d", getIBNSimSteps()); - LOGF(info, "Number of charge sharing steps of OB : %d", getNSimSteps()); - LOGF(info, "ELoss to N electrons factor : %e", getEnergyToNElectrons()); - LOGF(info, "Noise level per pixel of IB : %e", getIBNoisePerPixel()); - LOGF(info, "Noise level per pixel of OB : %e", getNoisePerPixel()); - LOGF(info, "Charge time-response:\n"); + printf("ITS3 DigiParams settings:\n"); + printf("Continuous readout : %s\n", isContinuous() ? "ON" : "OFF"); + printf("Readout Frame Length(ns) : %f\n", getROFrameLength()); + printf("Strobe delay (ns) : %f\n", getStrobeDelay()); + printf("Strobe length (ns) : %f\n", getStrobeLength()); + printf("IB Threshold (N electrons) : %d\n", getIBChargeThreshold()); + printf("OB Threshold (N electrons) : %d\n", getChargeThreshold()); + printf("Min N electrons to account for IB : %d\n", getIBMinChargeToAccount()); + printf("Min N electrons to account for OB : %d\n", getMinChargeToAccount()); + printf("Number of charge sharing steps of IB : %d\n", getIBNSimSteps()); + printf("Number of charge sharing steps of OB : %d\n", getNSimSteps()); + printf("ELoss to N electrons factor : %e\n", getEnergyToNElectrons()); + printf("Noise level per pixel of IB : %e\n", getIBNoisePerPixel()); + printf("Noise level per pixel of OB : %e\n", getNoisePerPixel()); + printf("Charge time-response:\n"); getSignalShape().print(); } -void DigiParams::setIBSimResponse(const o2::itsmft::AlpideSimResponse* response) +void DigiParams::setIBSimResponse(o2::its3::ChipSimResponse* response) { - // This method is compatible with ChipSimResponse and will automatically handle center computation. - auto chipResp = static_cast(response); - if (chipResp) { - mIBSimResponse = chipResp; - if (mIBSimResponse->getRespCentreDep() == 0.f) { - const_cast(mIBSimResponse)->computeCentreFromData(); - } - } else { - LOG(error) << "Provided response is not of type ChipSimResponse. Cannot proceed."; - return; + mIBSimResponse = response; + if (mIBSimResponse) { + mIBSimResponse->computeCentreFromData(); } } diff --git a/Detectors/Upgrades/ITS3/simulation/src/ITS3DPLDigitizerParam.cxx b/Detectors/Upgrades/ITS3/simulation/src/ITS3DPLDigitizerParam.cxx index 003372759644c..69314b8a0be9b 100644 --- a/Detectors/Upgrades/ITS3/simulation/src/ITS3DPLDigitizerParam.cxx +++ b/Detectors/Upgrades/ITS3/simulation/src/ITS3DPLDigitizerParam.cxx @@ -11,11 +11,4 @@ #include "ITS3Simulation/ITS3DPLDigitizerParam.h" -namespace o2 -{ -namespace its3 -{ -static auto& sDigitizerParamITS3 = o2::its3::ITS3DPLDigitizerParam::Instance(); -} // namespace its3 -} // namespace o2 O2ParamImpl(o2::its3::ITS3DPLDigitizerParam) \ No newline at end of file From 7db64281287aebc5f764c43d7ba06b8516c007e3 Mon Sep 17 00:00:00 2001 From: ALICE Action Bot Date: Mon, 21 Apr 2025 19:16:39 +0000 Subject: [PATCH 2/2] Please consider the following formatting changes --- .../simulation/include/ITS3Simulation/ChipDigitsContainer.h | 3 ++- .../ITS3/simulation/include/ITS3Simulation/DigiParams.h | 2 +- .../simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h index 6364a245469b4..0c9627fe412c3 100644 --- a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h +++ b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h @@ -19,7 +19,8 @@ #include "ITS3Simulation/DigiParams.h" // ITS3-specific DigiParams interface #include -namespace o2::its3 { +namespace o2::its3 +{ class ChipDigitsContainer : public o2::itsmft::ChipDigitsContainer { diff --git a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h index 74017ddc5ba24..5764dfbd7d593 100644 --- a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h +++ b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h @@ -57,7 +57,7 @@ class DigiParams final : public o2::itsmft::DigiParams private: const o2::itsmft::AlpideSimResponse* mOBSimResponse = nullptr; //!< pointer to external response - o2::its3::ChipSimResponse* mIBSimResponse = nullptr; //!< pointer to external response + o2::its3::ChipSimResponse* mIBSimResponse = nullptr; //!< pointer to external response ClassDef(DigiParams, 1); }; diff --git a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h index 0508b9dea21e1..3192f73fb8f79 100644 --- a/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h +++ b/Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h @@ -15,7 +15,8 @@ #include "CommonUtils/ConfigurableParam.h" #include "CommonUtils/ConfigurableParamHelper.h" -namespace o2::its3 { +namespace o2::its3 +{ struct ITS3DPLDigitizerParam : public o2::conf::ConfigurableParamHelper { float IBNoisePerPixel = 1.e-8; ///< MOSAIX Noise per channel