From 28c0ab2c445449187960f3a13ed3a846e921f5aa Mon Sep 17 00:00:00 2001 From: Sunanda Date: Thu, 2 Nov 2023 09:30:08 +0100 Subject: [PATCH 1/2] Make use of constexpr in the scintillator DeId for HGCal for faster processing --- .../interface/HGCScintillatorDetId.h | 155 ++++++++++++------ .../ForwardDetId/src/HGCScintillatorDetId.cc | 116 ++----------- 2 files changed, 122 insertions(+), 149 deletions(-) diff --git a/DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h b/DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h index 5ef34fb633f99..a65df713985b7 100644 --- a/DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h +++ b/DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h @@ -5,6 +5,7 @@ #include #include "DataFormats/DetId/interface/DetId.h" #include "DataFormats/ForwardDetId/interface/ForwardSubdetector.h" +#include "FWCore/Utilities/interface/Exception.h" /* \brief description of the bit assigment [0:8] iphi index wrt x-axis on +z side @@ -23,81 +24,143 @@ class HGCScintillatorDetId : public DetId { public: /** Create a null cellid*/ - HGCScintillatorDetId(); + constexpr HGCScintillatorDetId() : DetId() {} /** Create cellid from raw id (0=invalid tower id) */ - HGCScintillatorDetId(uint32_t rawid); + constexpr HGCScintillatorDetId(uint32_t rawid) : DetId(rawid) {} /** Constructor from subdetector, zplus, layer, module, cell numbers */ - HGCScintillatorDetId(int type, int layer, int ring, int iphi, bool trigger = false, int sipm = 0); + constexpr HGCScintillatorDetId(int type, int layer, int ring, int phi, bool trigger = false, int sipm = 0) : DetId(HGCalHSc, ForwardEmpty) { + int zside = (ring < 0) ? 1 : 0; + int itrig = trigger ? 1 : 0; + int ringAbs = std::abs(ring); + id_ |= (((type & kHGCalTypeMask) << kHGCalTypeOffset) | ((zside & kHGCalZsideMask) << kHGCalZsideOffset) | ((sipm & kHGCalSiPMMask) << kHGCalSiPMOffset) | ((itrig & kHGCalTriggerMask) << kHGCalTriggerOffset) | ((layer & kHGCalLayerMask) << kHGCalLayerOffset) | ((ringAbs & kHGCalRadiusMask) << kHGCalRadiusOffset) | ((phi & kHGCalPhiMask) << kHGCalPhiOffset)); + } + /** Constructor from a generic cell id */ - HGCScintillatorDetId(const DetId& id); + constexpr HGCScintillatorDetId(const DetId& gen) { + if (!gen.null()) { + if (gen.det() != HGCalHSc) { + throw cms::Exception("Invalid DetId") + << "Cannot initialize HGCScintillatorDetId from " << std::hex << gen.rawId() << std::dec; + } + } + id_ = gen.rawId(); + } + /** Assignment from a generic cell id */ - HGCScintillatorDetId& operator=(const DetId& id); + constexpr HGCScintillatorDetId& operator=(const DetId& gen) { + if (!gen.null()) { + if (gen.det() != HGCalHSc) { + throw cms::Exception("Invalid DetId") + << "Cannot assign HGCScintillatorDetId from " << std::hex << gen.rawId() << std::dec; + } + } + id_ = gen.rawId(); + return (*this); + } /** Converter for a geometry cell id */ - HGCScintillatorDetId geometryCell() const; + constexpr HGCScintillatorDetId geometryCell() const { + if (trigger()) { + return HGCScintillatorDetId(type(), layer(), iradiusTrigger(), iphiTrigger(), false); + } else { + return HGCScintillatorDetId(type(), layer(), iradius(), iphi(), false); + } + } /// get the subdetector - DetId::Detector subdet() const { return det(); } + constexpr DetId::Detector subdet() const { return det(); } /// get/set the type - int type() const { return (id_ >> kHGCalTypeOffset) & kHGCalTypeMask; } - void setType(int type); + constexpr int type() const { return (id_ >> kHGCalTypeOffset) & kHGCalTypeMask; } + constexpr void setType(int type) { + id_ &= kHGCalTypeMask0; + id_ |= ((type & kHGCalTypeMask) << kHGCalTypeOffset); + } /// get the z-side of the cell (1/-1) - int zside() const { return (((id_ >> kHGCalZsideOffset) & kHGCalZsideMask) ? -1 : 1); } + constexpr int zside() const { return (((id_ >> kHGCalZsideOffset) & kHGCalZsideMask) ? -1 : 1); } /// get the layer # - int layer() const { return (id_ >> kHGCalLayerOffset) & kHGCalLayerMask; } + constexpr int layer() const { return (id_ >> kHGCalLayerOffset) & kHGCalLayerMask; } /// get the eta index - int ring() const; - int iradiusAbs() const { return ring(); } - int iradius() const { return zside() * ring(); } - int ietaAbs() const { return ring(); } - int ieta() const { return zside() * ring(); } + constexpr int ring() const { + if (trigger()) + return (2 * ((id_ >> kHGCalRadiusOffset) & kHGCalRadiusMask)); + else + return ((id_ >> kHGCalRadiusOffset) & kHGCalRadiusMask); + } + constexpr int iradiusAbs() const { return ring(); } + constexpr int iradius() const { return zside() * ring(); } + constexpr int ietaAbs() const { return ring(); } + constexpr int ieta() const { return zside() * ring(); } /// get the phi index - int iphi() const; - std::pair ietaphi() const { return std::pair(ieta(), iphi()); } - std::pair ringphi() const { return std::pair(iradius(), iphi()); } + constexpr int iphi() const { + if (trigger()) + return (2 * ((id_ >> kHGCalPhiOffset) & kHGCalPhiMask)); + else + return ((id_ >> kHGCalPhiOffset) & kHGCalPhiMask); + } + constexpr std::pair ietaphi() const { return std::pair(ieta(), iphi()); } + constexpr std::pair ringphi() const { return std::pair(iradius(), iphi()); } /// get/set the sipm size - int sipm() const { return (id_ >> kHGCalSiPMOffset) & kHGCalSiPMMask; } - void setSiPM(int sipm); + constexpr int sipm() const { return (id_ >> kHGCalSiPMOffset) & kHGCalSiPMMask; } + constexpr void setSiPM(int sipm) { + id_ &= kHGCalSiPMMask0; + id_ |= ((sipm & kHGCalSiPMMask) << kHGCalSiPMOffset); + } /// trigger or detector cell std::vector detectorCells() const; - bool trigger() const { return (((id_ >> kHGCalTriggerOffset) & kHGCalTriggerMask) == 1); } - HGCScintillatorDetId triggerCell() const; + + constexpr bool trigger() const { return (((id_ >> kHGCalTriggerOffset) & kHGCalTriggerMask) == 1); } + constexpr HGCScintillatorDetId triggerCell() const { + if (trigger()) + return HGCScintillatorDetId(type(), layer(), iradius(), iphi(), true); + else + return HGCScintillatorDetId(type(), layer(), iradiusTrigger(), iphiTrigger(), true); + } /// consistency check : no bits left => no overhead - bool isEE() const { return false; } - bool isHE() const { return true; } - bool isForward() const { return true; } + constexpr bool isEE() const { return false; } + constexpr bool isHE() const { return true; } + constexpr bool isForward() const { return true; } static const HGCScintillatorDetId Undefined; public: - static const int kHGCalPhiOffset = 0; - static const int kHGCalPhiMask = 0x1FF; - static const int kHGCalRadiusOffset = 9; - static const int kHGCalRadiusMask = 0xFF; - static const int kHGCalLayerOffset = 17; - static const int kHGCalLayerMask = 0x1F; - static const int kHGCalTriggerOffset = 22; - static const int kHGCalTriggerMask = 0x1; - static const int kHGCalSiPMOffset = 23; - static const int kHGCalSiPMMask = 0x1; - static const int kHGCalSiPMMask0 = 0xFF7FFFFF; - static const int kHGCalZsideOffset = 25; - static const int kHGCalZsideMask = 0x1; - static const int kHGCalTypeOffset = 26; - static const int kHGCalTypeMask = 0x3; - static const int kHGCalTypeMask0 = 0xF3FFFFFF; - - int iradiusTriggerAbs() const; - int iradiusTrigger() const { return zside() * iradiusTriggerAbs(); } - int iphiTrigger() const; + static constexpr int kHGCalPhiOffset = 0; + static constexpr int kHGCalPhiMask = 0x1FF; + static constexpr int kHGCalRadiusOffset = 9; + static constexpr int kHGCalRadiusMask = 0xFF; + static constexpr int kHGCalLayerOffset = 17; + static constexpr int kHGCalLayerMask = 0x1F; + static constexpr int kHGCalTriggerOffset = 22; + static constexpr int kHGCalTriggerMask = 0x1; + static constexpr int kHGCalSiPMOffset = 23; + static constexpr int kHGCalSiPMMask = 0x1; + static constexpr int kHGCalSiPMMask0 = 0xFF7FFFFF; + static constexpr int kHGCalZsideOffset = 25; + static constexpr int kHGCalZsideMask = 0x1; + static constexpr int kHGCalTypeOffset = 26; + static constexpr int kHGCalTypeMask = 0x3; + static constexpr int kHGCalTypeMask0 = 0xF3FFFFFF; + + constexpr int iradiusTriggerAbs() const { + if (trigger()) + return ((ring() + 1) / 2); + else + return ring(); + } + constexpr int iradiusTrigger() const { return zside() * iradiusTriggerAbs(); } + constexpr int iphiTrigger() const { + if (trigger()) + return ((iphi() + 1) / 2); + else + return iphi(); + } }; std::ostream& operator<<(std::ostream&, const HGCScintillatorDetId& id); diff --git a/DataFormats/ForwardDetId/src/HGCScintillatorDetId.cc b/DataFormats/ForwardDetId/src/HGCScintillatorDetId.cc index 60afb5fd2a873..cd7cf0723a652 100644 --- a/DataFormats/ForwardDetId/src/HGCScintillatorDetId.cc +++ b/DataFormats/ForwardDetId/src/HGCScintillatorDetId.cc @@ -1,114 +1,24 @@ #include "DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h" -#include "FWCore/Utilities/interface/Exception.h" #include #include const HGCScintillatorDetId HGCScintillatorDetId::Undefined(0, 0, 0, 0, false); -HGCScintillatorDetId::HGCScintillatorDetId() : DetId() {} - -HGCScintillatorDetId::HGCScintillatorDetId(uint32_t rawid) : DetId(rawid) {} - -HGCScintillatorDetId::HGCScintillatorDetId(int type, int layer, int ring, int phi, bool trigger, int sipm) - : DetId(HGCalHSc, ForwardEmpty) { - int zside = (ring < 0) ? 1 : 0; - int itrig = trigger ? 1 : 0; - int ringAbs = std::abs(ring); - id_ |= (((type & kHGCalTypeMask) << kHGCalTypeOffset) | ((zside & kHGCalZsideMask) << kHGCalZsideOffset) | - ((sipm & kHGCalSiPMMask) << kHGCalSiPMOffset) | ((itrig & kHGCalTriggerMask) << kHGCalTriggerOffset) | - ((layer & kHGCalLayerMask) << kHGCalLayerOffset) | ((ringAbs & kHGCalRadiusMask) << kHGCalRadiusOffset) | - ((phi & kHGCalPhiMask) << kHGCalPhiOffset)); -} - -HGCScintillatorDetId::HGCScintillatorDetId(const DetId& gen) { - if (!gen.null()) { - if (gen.det() != HGCalHSc) { - throw cms::Exception("Invalid DetId") - << "Cannot initialize HGCScintillatorDetId from " << std::hex << gen.rawId() << std::dec; - } - } - id_ = gen.rawId(); -} - -HGCScintillatorDetId& HGCScintillatorDetId::operator=(const DetId& gen) { - if (!gen.null()) { - if (gen.det() != HGCalHSc) { - throw cms::Exception("Invalid DetId") - << "Cannot assign HGCScintillatorDetId from " << std::hex << gen.rawId() << std::dec; - } - } - id_ = gen.rawId(); - return (*this); -} - -int HGCScintillatorDetId::ring() const { - if (trigger()) - return (2 * ((id_ >> kHGCalRadiusOffset) & kHGCalRadiusMask)); - else - return ((id_ >> kHGCalRadiusOffset) & kHGCalRadiusMask); -} - -int HGCScintillatorDetId::iradiusTriggerAbs() const { - if (trigger()) - return ((ring() + 1) / 2); - else - return ring(); -} - -int HGCScintillatorDetId::iphi() const { - if (trigger()) - return (2 * ((id_ >> kHGCalPhiOffset) & kHGCalPhiMask)); - else - return ((id_ >> kHGCalPhiOffset) & kHGCalPhiMask); -} - -int HGCScintillatorDetId::iphiTrigger() const { - if (trigger()) - return ((iphi() + 1) / 2); - else - return iphi(); -} - -void HGCScintillatorDetId::setType(int type) { - id_ &= kHGCalTypeMask0; - id_ |= ((type & kHGCalTypeMask) << kHGCalTypeOffset); -} - -void HGCScintillatorDetId::setSiPM(int sipm) { - id_ &= kHGCalSiPMMask0; - id_ |= ((sipm & kHGCalSiPMMask) << kHGCalSiPMOffset); -} - std::vector HGCScintillatorDetId::detectorCells() const { - std::vector cells; - int irad = ring(); - int ifi = iphi(); - int iz = zside(); - if (trigger()) { - cells.emplace_back(HGCScintillatorDetId(type(), layer(), (2 * irad - 1) * iz, 2 * ifi - 1, false)); - cells.emplace_back(HGCScintillatorDetId(type(), layer(), 2 * irad * iz, 2 * ifi - 1, false)); - cells.emplace_back(HGCScintillatorDetId(type(), layer(), (2 * irad - 1) * iz, 2 * ifi, false)); - cells.emplace_back(HGCScintillatorDetId(type(), layer(), 2 * irad * iz, 2 * ifi, false)); - } else { - cells.emplace_back(HGCScintillatorDetId(type(), layer(), irad * iz, ifi, false)); - } - return cells; -} - -HGCScintillatorDetId HGCScintillatorDetId::geometryCell() const { - if (trigger()) { - return HGCScintillatorDetId(type(), layer(), iradiusTrigger(), iphiTrigger(), false); - } else { - return HGCScintillatorDetId(type(), layer(), iradius(), iphi(), false); + std::vector cells; + int irad = ring(); + int ifi = iphi(); + int iz = zside(); + if (trigger()) { + cells.emplace_back(HGCScintillatorDetId(type(), layer(), (2 * irad - 1) * iz, 2 * ifi - 1, false)); + cells.emplace_back(HGCScintillatorDetId(type(), layer(), 2 * irad * iz, 2 * ifi - 1, false)); + cells.emplace_back(HGCScintillatorDetId(type(), layer(), (2 * irad - 1) * iz, 2 * ifi, false)); + cells.emplace_back(HGCScintillatorDetId(type(), layer(), 2 * irad * iz, 2 * ifi, false)); + } else { + cells.emplace_back(HGCScintillatorDetId(type(), layer(), irad * iz, ifi, false)); + } + return cells; } -} - -HGCScintillatorDetId HGCScintillatorDetId::triggerCell() const { - if (trigger()) - return HGCScintillatorDetId(type(), layer(), iradius(), iphi(), true); - else - return HGCScintillatorDetId(type(), layer(), iradiusTrigger(), iphiTrigger(), true); -} std::ostream& operator<<(std::ostream& s, const HGCScintillatorDetId& id) { return s << " HGCScintillatorDetId::EE:HE= " << id.isEE() << ":" << id.isHE() << " trigger= " << id.trigger() From 95c7bc2481b1d9bb6b584720f15872ebc1784400 Mon Sep 17 00:00:00 2001 From: Sunanda Date: Thu, 2 Nov 2023 10:06:26 +0100 Subject: [PATCH 2/2] Code check --- .../interface/HGCScintillatorDetId.h | 18 ++++++++----- .../ForwardDetId/src/HGCScintillatorDetId.cc | 26 +++++++++---------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h b/DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h index a65df713985b7..f62ba55d3db28 100644 --- a/DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h +++ b/DataFormats/ForwardDetId/interface/HGCScintillatorDetId.h @@ -28,19 +28,23 @@ class HGCScintillatorDetId : public DetId { /** Create cellid from raw id (0=invalid tower id) */ constexpr HGCScintillatorDetId(uint32_t rawid) : DetId(rawid) {} /** Constructor from subdetector, zplus, layer, module, cell numbers */ - constexpr HGCScintillatorDetId(int type, int layer, int ring, int phi, bool trigger = false, int sipm = 0) : DetId(HGCalHSc, ForwardEmpty) { + constexpr HGCScintillatorDetId(int type, int layer, int ring, int phi, bool trigger = false, int sipm = 0) + : DetId(HGCalHSc, ForwardEmpty) { int zside = (ring < 0) ? 1 : 0; int itrig = trigger ? 1 : 0; int ringAbs = std::abs(ring); - id_ |= (((type & kHGCalTypeMask) << kHGCalTypeOffset) | ((zside & kHGCalZsideMask) << kHGCalZsideOffset) | ((sipm & kHGCalSiPMMask) << kHGCalSiPMOffset) | ((itrig & kHGCalTriggerMask) << kHGCalTriggerOffset) | ((layer & kHGCalLayerMask) << kHGCalLayerOffset) | ((ringAbs & kHGCalRadiusMask) << kHGCalRadiusOffset) | ((phi & kHGCalPhiMask) << kHGCalPhiOffset)); + id_ |= (((type & kHGCalTypeMask) << kHGCalTypeOffset) | ((zside & kHGCalZsideMask) << kHGCalZsideOffset) | + ((sipm & kHGCalSiPMMask) << kHGCalSiPMOffset) | ((itrig & kHGCalTriggerMask) << kHGCalTriggerOffset) | + ((layer & kHGCalLayerMask) << kHGCalLayerOffset) | ((ringAbs & kHGCalRadiusMask) << kHGCalRadiusOffset) | + ((phi & kHGCalPhiMask) << kHGCalPhiOffset)); } /** Constructor from a generic cell id */ constexpr HGCScintillatorDetId(const DetId& gen) { if (!gen.null()) { if (gen.det() != HGCalHSc) { - throw cms::Exception("Invalid DetId") - << "Cannot initialize HGCScintillatorDetId from " << std::hex << gen.rawId() << std::dec; + throw cms::Exception("Invalid DetId") + << "Cannot initialize HGCScintillatorDetId from " << std::hex << gen.rawId() << std::dec; } } id_ = gen.rawId(); @@ -50,8 +54,8 @@ class HGCScintillatorDetId : public DetId { constexpr HGCScintillatorDetId& operator=(const DetId& gen) { if (!gen.null()) { if (gen.det() != HGCalHSc) { - throw cms::Exception("Invalid DetId") - << "Cannot assign HGCScintillatorDetId from " << std::hex << gen.rawId() << std::dec; + throw cms::Exception("Invalid DetId") + << "Cannot assign HGCScintillatorDetId from " << std::hex << gen.rawId() << std::dec; } } id_ = gen.rawId(); @@ -107,7 +111,7 @@ class HGCScintillatorDetId : public DetId { /// get/set the sipm size constexpr int sipm() const { return (id_ >> kHGCalSiPMOffset) & kHGCalSiPMMask; } - constexpr void setSiPM(int sipm) { + constexpr void setSiPM(int sipm) { id_ &= kHGCalSiPMMask0; id_ |= ((sipm & kHGCalSiPMMask) << kHGCalSiPMOffset); } diff --git a/DataFormats/ForwardDetId/src/HGCScintillatorDetId.cc b/DataFormats/ForwardDetId/src/HGCScintillatorDetId.cc index cd7cf0723a652..05ba5896b8254 100644 --- a/DataFormats/ForwardDetId/src/HGCScintillatorDetId.cc +++ b/DataFormats/ForwardDetId/src/HGCScintillatorDetId.cc @@ -5,20 +5,20 @@ const HGCScintillatorDetId HGCScintillatorDetId::Undefined(0, 0, 0, 0, false); std::vector HGCScintillatorDetId::detectorCells() const { - std::vector cells; - int irad = ring(); - int ifi = iphi(); - int iz = zside(); - if (trigger()) { - cells.emplace_back(HGCScintillatorDetId(type(), layer(), (2 * irad - 1) * iz, 2 * ifi - 1, false)); - cells.emplace_back(HGCScintillatorDetId(type(), layer(), 2 * irad * iz, 2 * ifi - 1, false)); - cells.emplace_back(HGCScintillatorDetId(type(), layer(), (2 * irad - 1) * iz, 2 * ifi, false)); - cells.emplace_back(HGCScintillatorDetId(type(), layer(), 2 * irad * iz, 2 * ifi, false)); - } else { - cells.emplace_back(HGCScintillatorDetId(type(), layer(), irad * iz, ifi, false)); - } - return cells; + std::vector cells; + int irad = ring(); + int ifi = iphi(); + int iz = zside(); + if (trigger()) { + cells.emplace_back(HGCScintillatorDetId(type(), layer(), (2 * irad - 1) * iz, 2 * ifi - 1, false)); + cells.emplace_back(HGCScintillatorDetId(type(), layer(), 2 * irad * iz, 2 * ifi - 1, false)); + cells.emplace_back(HGCScintillatorDetId(type(), layer(), (2 * irad - 1) * iz, 2 * ifi, false)); + cells.emplace_back(HGCScintillatorDetId(type(), layer(), 2 * irad * iz, 2 * ifi, false)); + } else { + cells.emplace_back(HGCScintillatorDetId(type(), layer(), irad * iz, ifi, false)); } + return cells; +} std::ostream& operator<<(std::ostream& s, const HGCScintillatorDetId& id) { return s << " HGCScintillatorDetId::EE:HE= " << id.isEE() << ":" << id.isHE() << " trigger= " << id.trigger()