From 33097ef6060486d181994c5776b16f7d91fb5ade Mon Sep 17 00:00:00 2001 From: "W. David Dagenhart" Date: Thu, 21 Nov 2013 08:56:47 -0600 Subject: [PATCH] Threaded Random Number Interface for OscarProducer Modify OscarProducer to use the new interface for the random number service that requires a StreamID argument to the getEngine function. Modify the module to be a "One" type module with shared resources declared for GEANT and the CLHEPRandomEngine. It is also "watching" runs. Note that with this change it will now seg fault if any random numbers were generated in the constructor or beginRun. SimProducer is fixed to deal with the changed module type. We are presuming the static code checking would reveal any other threading problems related to making this a "One" type module. This is for GEANT 4.9.X. We will need to revisit this when we move to 4.10.X, which will be the multithreaded GEANT. The only changes in Validation are the addition of a missing forward declaration revealed by the other changes. --- .../interface/SharedResourceNames.h | 28 ++++++++++ FWCore/Concurrency/src/SharedResourceNames.cc | 4 ++ .../Application/interface/OscarProducer.h | 19 +++---- SimG4Core/Application/plugins/BuildFile.xml | 1 + .../Application/plugins/OscarProducer.cc | 53 ++++++++----------- .../HelpfulWatchers/src/BeginOfTrackCounter.h | 4 ++ SimG4Core/Watcher/interface/SimProducer.h | 14 +++-- .../interface/EcalSimHitsValidProducer.h | 4 ++ .../Geometry/interface/MaterialBudgetAction.h | 4 ++ .../HcalHits/interface/SimG4HcalValidation.h | 4 ++ 10 files changed, 88 insertions(+), 47 deletions(-) create mode 100644 FWCore/Concurrency/interface/SharedResourceNames.h create mode 100644 FWCore/Concurrency/src/SharedResourceNames.cc diff --git a/FWCore/Concurrency/interface/SharedResourceNames.h b/FWCore/Concurrency/interface/SharedResourceNames.h new file mode 100644 index 0000000000000..2899a65b65ce4 --- /dev/null +++ b/FWCore/Concurrency/interface/SharedResourceNames.h @@ -0,0 +1,28 @@ +#ifndef FWCore_Concurrency_SharedResourceNames_h +#define FWCore_Concurrency_SharedResourceNames_h +// +// Package: Concurrency +// Class : ShareResourceNames +// +/**\class edm::SharedResourceNames + +Description: Contains the names of external shared resources. + +*/ +// +// Original Author: W. David Dagenhart +// Created: 19 November 2013 +// + +#include + +namespace edm { + class SharedResourceNames { + public: + // GEANT 4.9.X needs to be declared a shared resource + // In the future, 4.10.X and later might not need to be + static const std::string kGEANT; + static const std::string kCLHEPRandomEngine; + }; +} +#endif diff --git a/FWCore/Concurrency/src/SharedResourceNames.cc b/FWCore/Concurrency/src/SharedResourceNames.cc new file mode 100644 index 0000000000000..43d9a43259914 --- /dev/null +++ b/FWCore/Concurrency/src/SharedResourceNames.cc @@ -0,0 +1,4 @@ +#include "FWCore/Concurrency/interface/SharedResourceNames.h" + +const std::string edm::SharedResourceNames::kGEANT = "GEANT"; +const std::string edm::SharedResourceNames::kCLHEPRandomEngine = "CLHEPRandomEngine"; diff --git a/SimG4Core/Application/interface/OscarProducer.h b/SimG4Core/Application/interface/OscarProducer.h index df548699ed26d..3b67723dba2a0 100644 --- a/SimG4Core/Application/interface/OscarProducer.h +++ b/SimG4Core/Application/interface/OscarProducer.h @@ -1,9 +1,8 @@ #ifndef SimG4Core_OscarProducer_H #define SimG4Core_OscarProducer_H -#include "FWCore/Framework/interface/EDProducer.h" +#include "FWCore/Framework/interface/one/EDProducer.h" #include "FWCore/Framework/interface/Event.h" -// #include "DataFormats/Common/interface/Handle.h" #include "DataFormats/Common/interface/Handle.h" #include "FWCore/Framework/interface/MakerMacros.h" #include "FWCore/Framework/interface/EventSetup.h" @@ -13,11 +12,9 @@ #include "SimG4Core/Application/interface/CustomUIsession.h" -namespace CLHEP { - class HepRandomEngine; -} +#include -class OscarProducer : public edm::EDProducer +class OscarProducer : public edm::one::EDProducer { public: typedef std::vector > Producers; @@ -25,16 +22,12 @@ class OscarProducer : public edm::EDProducer explicit OscarProducer(edm::ParameterSet const & p); virtual ~OscarProducer(); virtual void beginRun(const edm::Run & r,const edm::EventSetup& c) override; - virtual void beginJob(); - virtual void endJob(); + virtual void endRun(const edm::Run & r,const edm::EventSetup& c) override { } virtual void produce(edm::Event & e, const edm::EventSetup& c) override; protected: - RunManager* m_runManager; + std::unique_ptr m_runManager; Producers m_producers; - CustomUIsession* m_UIsession; - -private: - CLHEP::HepRandomEngine* m_engine; + std::unique_ptr m_UIsession; }; #endif diff --git a/SimG4Core/Application/plugins/BuildFile.xml b/SimG4Core/Application/plugins/BuildFile.xml index aa56efe7d3a7c..5bd84e3b240a3 100644 --- a/SimG4Core/Application/plugins/BuildFile.xml +++ b/SimG4Core/Application/plugins/BuildFile.xml @@ -1,3 +1,4 @@ + diff --git a/SimG4Core/Application/plugins/OscarProducer.cc b/SimG4Core/Application/plugins/OscarProducer.cc index 2fdc83441e4bb..073a5a9a56946 100644 --- a/SimG4Core/Application/plugins/OscarProducer.cc +++ b/SimG4Core/Application/plugins/OscarProducer.cc @@ -20,10 +20,15 @@ #include "FWCore/Utilities/interface/RandomNumberGenerator.h" #include "CLHEP/Random/Random.h" +#include "FWCore/Concurrency/interface/SharedResourceNames.h" #include "FWCore/MessageLogger/interface/MessageLogger.h" #include +namespace edm { + class StreamID; +} + namespace { // // this machinery allows to set CLHEP static engine @@ -37,7 +42,7 @@ namespace { // class StaticRandomEngineSetUnset { public: - StaticRandomEngineSetUnset(); + StaticRandomEngineSetUnset(edm::StreamID const&); explicit StaticRandomEngineSetUnset(CLHEP::HepRandomEngine * engine); ~StaticRandomEngineSetUnset(); CLHEP::HepRandomEngine* getEngine() const; @@ -48,10 +53,13 @@ namespace { } OscarProducer::OscarProducer(edm::ParameterSet const & p) -{ - StaticRandomEngineSetUnset random; - m_engine = random.getEngine(); - +{ + // Random number generation not allowed here + StaticRandomEngineSetUnset random(nullptr); + + usesResource(edm::SharedResourceNames::kGEANT); + usesResource(edm::SharedResourceNames::kCLHEPRandomEngine); + produces().setBranchAlias("SimTracks"); produces().setBranchAlias("SimVertices"); produces("TrackerHitsPixelBarrelLowTof"); @@ -95,7 +103,7 @@ OscarProducer::OscarProducer(edm::ParameterSet const & p) produces("WedgeHits"); //m_runManager = RunManager::init(p); - m_runManager = new RunManager(p); + m_runManager.reset(new RunManager(p)); //register any products m_producers= m_runManager->producers(); @@ -107,35 +115,23 @@ OscarProducer::OscarProducer(edm::ParameterSet const & p) } //UIsession manager for message handling - m_UIsession = new CustomUIsession(); + m_UIsession.reset(new CustomUIsession()); } OscarProducer::~OscarProducer() -{ - //this is causing a seg fault when an exception occurs while constructing - // an HcalSD. Need to check for memory problems. - if (m_runManager!=0) delete m_runManager; - if (m_UIsession!=0) delete m_UIsession; - -} +{ } void OscarProducer::beginRun(const edm::Run & r, const edm::EventSetup & es) { - m_runManager->initG4(es); + // Random number generation not allowed here + StaticRandomEngineSetUnset random(nullptr); + m_runManager->initG4(es); } - -void OscarProducer::beginJob() -{ - StaticRandomEngineSetUnset random(m_engine); -} - -void OscarProducer::endJob() { } - void OscarProducer::produce(edm::Event & e, const edm::EventSetup & es) { - StaticRandomEngineSetUnset random(m_engine); + StaticRandomEngineSetUnset random(e.streamID()); std::vector& sTk = m_runManager->sensTkDetectors(); std::vector& sCalo = m_runManager->sensCaloDetectors(); @@ -191,18 +187,16 @@ void OscarProducer::produce(edm::Event & e, const edm::EventSetup & es) } -StaticRandomEngineSetUnset::StaticRandomEngineSetUnset() { - - using namespace edm; - Service rng; +StaticRandomEngineSetUnset::StaticRandomEngineSetUnset(edm::StreamID const& streamID) { + edm::Service rng; if ( ! rng.isAvailable()) { throw cms::Exception("Configuration") << "The OscarProducer module requires the RandomNumberGeneratorService\n" "which is not present in the configuration file. You must add the service\n" "in the configuration file if you want to run OscarProducer"; } - m_currentEngine = &(rng->getEngine()); + m_currentEngine = &(rng->getEngine(streamID)); m_previousEngine = CLHEP::HepRandom::getTheEngine(); CLHEP::HepRandom::setTheEngine(m_currentEngine); @@ -224,4 +218,3 @@ CLHEP::HepRandomEngine* StaticRandomEngineSetUnset::getEngine() const { return m_currentEngine; } DEFINE_FWK_MODULE(OscarProducer); - diff --git a/SimG4Core/HelpfulWatchers/src/BeginOfTrackCounter.h b/SimG4Core/HelpfulWatchers/src/BeginOfTrackCounter.h index 7818b97472003..4e4434cc175a6 100644 --- a/SimG4Core/HelpfulWatchers/src/BeginOfTrackCounter.h +++ b/SimG4Core/HelpfulWatchers/src/BeginOfTrackCounter.h @@ -28,6 +28,10 @@ #include "SimG4Core/Notification/interface/BeginOfTrack.h" // forward declarations +namespace edm { + class ParameterSet; +} + namespace simwatcher { class BeginOfTrackCounter : public SimProducer, public Observer diff --git a/SimG4Core/Watcher/interface/SimProducer.h b/SimG4Core/Watcher/interface/SimProducer.h index 692c032515ae2..b95b23c1a1d9c 100644 --- a/SimG4Core/Watcher/interface/SimProducer.h +++ b/SimG4Core/Watcher/interface/SimProducer.h @@ -19,6 +19,7 @@ // // system include files +#include #include #include #include "boost/bind.hpp" @@ -26,7 +27,12 @@ // user include files #include "SimG4Core/Watcher/interface/SimWatcher.h" -#include "FWCore/Framework/interface/EDProducer.h" +#include "FWCore/Framework/interface/ProducerBase.h" + +namespace edm { + class Event; + class EventSetup; +} // forward declarations namespace simproducer { @@ -40,7 +46,7 @@ namespace simproducer { const std::string& instanceName() const { return m_instanceName; } - virtual void registerProduct(edm::EDProducer*) const = 0; + virtual void registerProduct(edm::ProducerBase*) const = 0; private: std::string m_instanceName; }; @@ -51,7 +57,7 @@ namespace simproducer { ProductInfo(const std::string& iInstanceName) : ProductInfoBase(iInstanceName) {} - void registerProduct(edm::EDProducer* iProd) const { + void registerProduct(edm::ProducerBase* iProd) const { (*iProd). template produces(this->instanceName()); } }; @@ -71,7 +77,7 @@ class SimProducer : public SimWatcher // ---------- member functions --------------------------- virtual void produce(edm::Event&, const edm::EventSetup&) = 0; - void registerProducts(edm::EDProducer& iProd) { + void registerProducts(edm::ProducerBase& iProd) { std::for_each(m_info.begin(), m_info.end(), boost::bind(&simproducer::ProductInfoBase::registerProduct,_1, &iProd)); } diff --git a/Validation/EcalHits/interface/EcalSimHitsValidProducer.h b/Validation/EcalHits/interface/EcalSimHitsValidProducer.h index a1ccec7fdaa3b..8fff50ef2f802 100644 --- a/Validation/EcalHits/interface/EcalSimHitsValidProducer.h +++ b/Validation/EcalHits/interface/EcalSimHitsValidProducer.h @@ -18,6 +18,10 @@ class G4Step; class EndOfEvent; class PEcalValidInfo; +namespace edm { + class ParameterSet; +} + class EcalSimHitsValidProducer : public SimProducer, public Observer, public Observer, diff --git a/Validation/Geometry/interface/MaterialBudgetAction.h b/Validation/Geometry/interface/MaterialBudgetAction.h index ce001508b60be..29d9e20b6dda6 100644 --- a/Validation/Geometry/interface/MaterialBudgetAction.h +++ b/Validation/Geometry/interface/MaterialBudgetAction.h @@ -26,6 +26,10 @@ class EndOfEvent; class G4StepPoint; class G4VTouchable; +namespace edm { + class ParameterSet; +} + class MaterialBudgetAction : public SimProducer, public Observer, public Observer, diff --git a/Validation/HcalHits/interface/SimG4HcalValidation.h b/Validation/HcalHits/interface/SimG4HcalValidation.h index 5ca14806e37f9..f59741f2537e3 100644 --- a/Validation/HcalHits/interface/SimG4HcalValidation.h +++ b/Validation/HcalHits/interface/SimG4HcalValidation.h @@ -27,6 +27,10 @@ class BeginOfRun; class BeginOfEvent; class EndOfEvent; +namespace edm { + class ParameterSet; +} + class SimG4HcalValidation : public SimProducer, public Observer, public Observer,