Address reviewer comments

Author: ChunzhengLab

Detectors/ITSMFT/common/simulation/include/ITSMFTSimulation/AlpideSimResponse.h

@@ -69,7 +69,7 @@ class AlpideRespSimMat
  private:
   std::array<float, MatSize> data;
 
-  ClassDefNV(AlpideRespSimMat, 1);
+  ClassDef(AlpideRespSimMat, 2);
 };
 
 /*

Detectors/Upgrades/ITS3/base/include/ITS3Base/SegmentationMosaix.h

@@ -17,8 +17,6 @@
 #ifndef ALICEO2_ITS3_SEGMENTATIONMOSAIX_H_
 #define ALICEO2_ITS3_SEGMENTATIONMOSAIX_H_
 
-#include <type_traits>
-
 #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 <typename T>
-  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 <typename T>
-  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<float>(row) + 0.5f) * PitchRow + WidthH;
-    zCol = (static_cast<float>(col) + 0.5f) * PitchCol - LengthH;
+    xRow = -(row + 0.5f) * PitchRow + WidthH;
+    zCol = (col + 0.5f) * PitchCol - LengthH;
   }
 
-  template <typename T>
-  constexpr bool detectorToLocal(T const row, T const col, math_utils::Point3D<float>& loc) const noexcept
+  bool detectorToLocal(float const row, float const col, math_utils::Point3D<float>& 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 <typename T>
-  constexpr void detectorToLocalUnchecked(T const row, T const col, math_utils::Point3D<float>& loc) const noexcept
+  void detectorToLocalUnchecked(float const row, float const col, math_utils::Point3D<float>& 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:

Detectors/Upgrades/ITS3/base/include/ITS3Base/SpecsV2.h

@@ -134,7 +134,6 @@ constexpr std::array<double, nLayers> radii{19.0006 * mm, 25.228 * mm, 31.4554 *
 constexpr std::array<double, nLayers> radiiInner{radii[0] - silicon::thicknessIn, radii[1] - silicon::thicknessIn, radii[2] - silicon::thicknessIn};                 // inner silicon radius
 constexpr std::array<double, nLayers> radiiOuter{radii[0] + silicon::thicknessOut, radii[1] + silicon::thicknessOut, radii[2] + silicon::thicknessOut};              // outer silicon radius
 constexpr std::array<double, nLayers> radiiMiddle{(radiiInner[0] + radiiOuter[0]) / 2., (radiiInner[1] + radiiOuter[1]) / 2., (radiiInner[2] + radiiOuter[2]) / 2.}; // middle silicon radius
-constexpr double nominalYShift{-metalstack::thickness / 2.};                                                                                                         // shift to position in silicion volume to the chip volume (silicon+metalstack)
 
 // extra information of pixels and their response functions
 namespace pixelarray::pixels

Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ChipDigitsContainer.h

@@ -19,15 +19,9 @@
 #include "ITS3Simulation/DigiParams.h"            // ITS3-specific DigiParams interface
 #include <TRandom.h>
 
-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 +32,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 +54,6 @@ class ChipDigitsContainer : public o2::itsmft::ChipDigitsContainer
   ClassDefNV(ChipDigitsContainer, 1);
 };
 
-} // namespace its3
-} // namespace o2
+} // namespace o2::its3
 
 #endif // ALICEO2_ITS3_CHIPDIGITSCONTAINER_

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

Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/DigiParams.h

@@ -48,18 +48,18 @@ 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; }
 
   void print() const final;
 
  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

Detectors/Upgrades/ITS3/simulation/include/ITS3Simulation/ITS3DPLDigitizerParam.h

@@ -15,9 +15,7 @@
 #include "CommonUtils/ConfigurableParam.h"
 #include "CommonUtils/ConfigurableParamHelper.h"
 
-namespace o2
-{
-namespace its3
+namespace o2::its3
 {
 
 struct ITS3DPLDigitizerParam : public o2::conf::ConfigurableParamHelper<ITS3DPLDigitizerParam> {
@@ -29,7 +27,6 @@ struct ITS3DPLDigitizerParam : public o2::conf::ConfigurableParamHelper<ITS3DPLD
   O2ParamDef(ITS3DPLDigitizerParam, "ITS3DPLDigitizerParam");
 };
 
-} // namespace its3
-} // namespace o2
+} // namespace o2::its3
 
 #endif

Detectors/Upgrades/ITS3/simulation/src/DigiParams.cxx

@@ -14,6 +14,7 @@
 
 #include "Framework/Logger.h"
 #include "ITS3Simulation/DigiParams.h"
+#include <cstdio>
 
 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<const o2::its3::ChipSimResponse*>(response);
-  if (chipResp) {
-    mIBSimResponse = chipResp;
-    if (mIBSimResponse->getRespCentreDep() == 0.f) {
-      const_cast<o2::its3::ChipSimResponse*>(mIBSimResponse)->computeCentreFromData();
-    }
-  } else {
-    LOG(error) << "Provided response is not of type ChipSimResponse. Cannot proceed.";
-    return;
+  mIBSimResponse = response;
+  if (mIBSimResponse) {
+    mIBSimResponse->computeCentreFromData();
   }
 }
 

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)