Skip to content

Commit

Permalink
make EcalRingCalibrationTools thread safe
Browse files Browse the repository at this point in the history 
Implement a minimal set of changes to EcalRingCalibrationTools to make
its initialisation and usage thead-safe.
In particular, this should make HLTEcalPhiSymFilter thread-safe again.

Note that EcalRingCalibrationTools is still non-compliant with the CMS
coding rules, and should be rewritten either as an ESProducer, or as a
non-static data member of an EDProducer.
  • Loading branch information
@fwyzard
fwyzard committed Sep 17, 2015
1 parent 8f8c933 commit 4d6b4b1
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 56 deletions.
22 changes: 13 additions & 9 deletions Calibration/Tools/interface/EcalRingCalibrationTools.h
View file
Expand Up @@ -11,6 +11,8 @@
***************************************/

#include <vector>
#include <mutex>
#include <atomic>
#include "DataFormats/EcalDetId/interface/EBDetId.h"
#include "DataFormats/EcalDetId/interface/EEDetId.h"

Expand All @@ -33,21 +35,23 @@ class EcalRingCalibrationTools
static std::vector<DetId> getDetIdsInModule(short int);
static std::vector<DetId> getDetIdsInECAL();

static const short N_RING_TOTAL = 248;
static const short N_RING_BARREL = 170 ;
static const short N_RING_ENDCAP = 78;
static const short N_RING_TOTAL = 248;
static const short N_RING_BARREL = 170;
static const short N_RING_ENDCAP = 78;

static const short N_MODULES_BARREL = 144;

static void setCaloGeometry(const CaloGeometry* geometry) { caloGeometry_ = geometry; };
static void setCaloGeometry(const CaloGeometry* geometry);

private:

static void initializeFromGeometry(); //needed only for the endcap
static void initializeFromGeometry(CaloGeometry const* geometry); // needed only for the endcap

static bool isInitializedFromGeometry_;
static short endcapRingIndex_[EEDetId::IX_MAX][EEDetId::IY_MAX]; //array needed only for the endcaps
static const CaloGeometry* caloGeometry_;
static std::atomic<bool> isInitializedFromGeometry_;

[[cms::thread_guard("isInitializedFromGeometry_")]]
static short endcapRingIndex_[EEDetId::IX_MAX][EEDetId::IY_MAX]; // array needed only for the endcaps

static std::once_flag once_;

};
#endif
81 changes: 35 additions & 46 deletions Calibration/Tools/src/EcalRingCalibrationTools.cc
View file
Expand Up @@ -10,11 +10,10 @@

#include <iostream>


//by default is not initialized, gets initialized at first call
bool EcalRingCalibrationTools::isInitializedFromGeometry_ = false;
const CaloGeometry* EcalRingCalibrationTools::caloGeometry_ = 0;
std::atomic<bool> EcalRingCalibrationTools::isInitializedFromGeometry_(false);
short EcalRingCalibrationTools::endcapRingIndex_[EEDetId::IX_MAX][EEDetId::IY_MAX];
std::once_flag EcalRingCalibrationTools::once_;


short EcalRingCalibrationTools::getRingIndex(DetId id)
Expand All @@ -33,8 +32,9 @@ short EcalRingCalibrationTools::getRingIndex(DetId id)
if (id.subdetId() == EcalEndcap)
{
//needed only for the EE, it can be replaced at some point with something smarter
if (!isInitializedFromGeometry_)
initializeFromGeometry();
if (not isInitializedFromGeometry_)
throw std::logic_error("EcalRingCalibrationTools::initializeFromGeometry Ecal Endcap geometry is not initialized");

EEDetId eid(id);
short endcapRingIndex = endcapRingIndex_[eid.ix()-1][eid.iy()-1] + N_RING_BARREL;
if (eid.zside() == 1) endcapRingIndex += N_RING_ENDCAP/2;
Expand Down Expand Up @@ -71,7 +71,6 @@ short EcalRingCalibrationTools::getModuleIndex(DetId id)
std::vector<DetId> EcalRingCalibrationTools::getDetIdsInRing(short etaIndex)
{


std::vector<DetId> ringIds;
if (etaIndex < 0)
return ringIds;
Expand All @@ -94,8 +93,8 @@ std::vector<DetId> EcalRingCalibrationTools::getDetIdsInRing(short etaIndex)
else if (etaIndex < N_RING_TOTAL)
{
//needed only for the EE, it can be replaced at some point maybe with something smarter
if (!isInitializedFromGeometry_)
initializeFromGeometry();
if (not isInitializedFromGeometry_)
throw std::logic_error("EcalRingCalibrationTools::initializeFromGeometry Ecal Endcap geometry is not initialized");

int zside= (etaIndex < N_RING_BARREL + (N_RING_ENDCAP/2) ) ? -1 : 1;
short eeEtaIndex = (etaIndex - N_RING_BARREL)%(N_RING_ENDCAP/2);
Expand All @@ -112,7 +111,6 @@ std::vector<DetId> EcalRingCalibrationTools::getDetIdsInRing(short etaIndex)

std::vector<DetId> EcalRingCalibrationTools::getDetIdsInECAL()
{

std::vector<DetId> ringIds;

for(int ieta= - EBDetId::MAX_IETA; ieta<=EBDetId::MAX_IETA; ++ieta)
Expand All @@ -121,8 +119,8 @@ std::vector<DetId> EcalRingCalibrationTools::getDetIdsInECAL()
ringIds.push_back(EBDetId(ieta,iphi));

//needed only for the EE, it can be replaced at some point maybe with something smarter
if (!isInitializedFromGeometry_)
initializeFromGeometry();
if (not isInitializedFromGeometry_)
throw std::logic_error("EcalRingCalibrationTools::initializeFromGeometry Ecal Endcap geometry is not initialized");

for (int ix=0;ix<EEDetId::IX_MAX;++ix)
for (int iy=0;iy<EEDetId::IY_MAX;++iy)
Expand Down Expand Up @@ -200,52 +198,46 @@ std::vector<DetId> EcalRingCalibrationTools::getDetIdsInModule(short moduleIndex
return ringIds;
}

void EcalRingCalibrationTools::initializeFromGeometry()
{
void EcalRingCalibrationTools::setCaloGeometry(const CaloGeometry* geometry)
{
std::call_once(once_, EcalRingCalibrationTools::initializeFromGeometry, geometry);
}

if (!caloGeometry_)
{
edm::LogError("EcalRingCalibrationTools") << "BIG ERROR::Initializing without geometry handle" ;
return;
}
void EcalRingCalibrationTools::initializeFromGeometry(CaloGeometry const* geometry)
{
if (not geometry)
throw std::invalid_argument("EcalRingCalibrationTools::initializeFromGeometry called with a nullptr argument");

float m_cellPosEta[EEDetId::IX_MAX][EEDetId::IY_MAX];
float cellPosEta[EEDetId::IX_MAX][EEDetId::IY_MAX];
for (int ix=0; ix<EEDetId::IX_MAX; ++ix)
for (int iy=0; iy<EEDetId::IY_MAX; ++iy)
{
m_cellPosEta[ix][iy] = -1.;
endcapRingIndex_[ix][iy]=-9;
}


const CaloSubdetectorGeometry *endcapGeometry = caloGeometry_->getSubdetectorGeometry(DetId::Ecal, EcalEndcap);

if (!endcapGeometry)
for (int iy=0; iy<EEDetId::IY_MAX; ++iy)
{
edm::LogError("EcalRingCalibrationTools") << "BIG ERROR::Ecal Endcap geometry not found" ;
return;
cellPosEta[ix][iy] = -1.;
endcapRingIndex_[ix][iy]=-9;
}

CaloSubdetectorGeometry const* endcapGeometry = geometry->getSubdetectorGeometry(DetId::Ecal, EcalEndcap);
if (not endcapGeometry)
throw std::logic_error("EcalRingCalibrationTools::initializeFromGeometry Ecal Endcap geometry not found");

const std::vector<DetId>& m_endcapCells= caloGeometry_->getValidDetIds(DetId::Ecal, EcalEndcap);
std::vector<DetId> const& endcapCells = geometry->getValidDetIds(DetId::Ecal, EcalEndcap);

for (std::vector<DetId>::const_iterator endcapIt = m_endcapCells.begin();
endcapIt!=m_endcapCells.end();
for (std::vector<DetId>::const_iterator endcapIt = endcapCells.begin();
endcapIt!=endcapCells.end();
++endcapIt)
{
EEDetId ee(*endcapIt);
if (ee.zside() == -1) continue; //Just using +side to fill absEta x,y map
const CaloCellGeometry *cellGeometry = endcapGeometry->getGeometry(*endcapIt) ;
int ics=ee.ix() - 1 ;
int ips=ee.iy() - 1 ;
m_cellPosEta[ics][ips] = fabs(cellGeometry->getPosition().eta());

cellPosEta[ics][ips] = fabs(cellGeometry->getPosition().eta());
//std::cout<<"EE Xtal, |eta| is "<<fabs(cellGeometry->getPosition().eta())<<std::endl;

}

float eta_ring[N_RING_ENDCAP/2];
for (int ring=0; ring<N_RING_ENDCAP/2; ++ring)
eta_ring[ring]=m_cellPosEta[ring][50];
eta_ring[ring]=cellPosEta[ring][50];

double etaBoundary[N_RING_ENDCAP/2 + 1];
etaBoundary[0]=1.47;
Expand All @@ -254,29 +246,26 @@ void EcalRingCalibrationTools::initializeFromGeometry()
for (int ring=1; ring<N_RING_ENDCAP/2; ++ring)
etaBoundary[ring]=(eta_ring[ring]+eta_ring[ring-1])/2.;



for (int ring=0; ring<N_RING_ENDCAP/2; ring++){
for (int ring=0; ring<N_RING_ENDCAP/2; ++ring){
// std::cout<<"***********************EE ring: "<<ring<<" eta "<<(etaBoundary[ring] + etaBoundary[ring+1])/2.<<std::endl;
for (int ix=0; ix<EEDetId::IX_MAX; ix++)
for (int iy=0; iy<EEDetId::IY_MAX; iy++)
if (m_cellPosEta[ix][iy]>etaBoundary[ring] && m_cellPosEta[ix][iy]<etaBoundary[ring+1])
if (cellPosEta[ix][iy]>etaBoundary[ring] && cellPosEta[ix][iy]<etaBoundary[ring+1])
{
endcapRingIndex_[ix][iy]=ring;
//std::cout<<"endcapRing_["<<ix+1<<"]["<<iy+1<<"] = "<<ring<<";"<<std::endl;
}
}

const std::vector<DetId>& m_barrelCells= caloGeometry_->getValidDetIds(DetId::Ecal, EcalBarrel);
std::vector<DetId> const& barrelCells = geometry->getValidDetIds(DetId::Ecal, EcalBarrel);

for (std::vector<DetId>::const_iterator barrelIt = m_barrelCells.begin();
barrelIt!=m_barrelCells.end();
for (std::vector<DetId>::const_iterator barrelIt = barrelCells.begin();
barrelIt!=barrelCells.end();
++barrelIt)
{
EBDetId eb(*barrelIt);
}


//EB

isInitializedFromGeometry_ = true;
Expand Down
2 changes: 1 addition & 1 deletion HLTrigger/special/src/HLTEcalPhiSymFilter.cc
View file
Expand Up @@ -82,7 +82,7 @@ HLTEcalPhiSymFilter::filter(edm::StreamID, edm::Event & event, const edm::EventS
//Get iRing-geometry
edm::ESHandle<CaloGeometry> geoHandle;
setup.get<CaloGeometryRecord>().get(geoHandle);
EcalRingCalibrationTools::setCaloGeometry(&(*geoHandle));
EcalRingCalibrationTools::setCaloGeometry(geoHandle.product());
EcalRingCalibrationTools CalibRing;

static const short N_RING_BARREL = EcalRingCalibrationTools::N_RING_BARREL;
Expand Down

0 comments on commit 4d6b4b1

Please sign in to comment.