Avoid discarding createEngine’s returned reference , or Down with art ’s getEngine! Kyle J. Knoepfel LArSoft Coordination Meeting 29 January 2019
Random number engines in art /LArSoft • art provides random-number management via the RandomNumberGenerator service. – Engines are attributed to modules, using the createEngine(…) base-class member function. • To access the engine, just cache the returned reference from the createEngine call CLHEP::HepRandomEngine& engine = createEngine(…); • Directly using the RandomNumberGenerator service has rarely been required. 2 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
Random number engines in art /LArSoft • art provides random-number management via the RandomNumberGenerator service. – Engines are attributed to modules, using the createEngine(…) base-class member function. • To access the engine, just cache the returned reference from the createEngine call CLHEP::HepRandomEngine& engine = createEngine(…); • Directly using the RandomNumberGenerator service has rarely been required. • For nutools users, the NuRandomService has its own createEngine function: – The NuRandomService::createEngine function wraps the art -provided function. – Until recently, the only way you could gain access to the engine was to call art ’s RandomNumberGenerator::getEngine function. 3 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
Random number engines in art /LArSoft • art provides random-number management via the RandomNumberGenerator service. – Engines are attributed to modules, using the createEngine(…) base-class member function. • To access the engine, just cache the returned reference from the createEngine call CLHEP::HepRandomEngine& engine = createEngine(…); • Directly using the RandomNumberGenerator service has rarely been required. • For nutools users, the NuRandomService has its own createEngine function: – The NuRandomService::createEngine function wraps the art -provided function. – Until recently, the only way you could gain access to the engine was to call art ’s RandomNumberGenerator::getEngine function. RNG::getEngine will be deprecated in art 3.02, removed in art 3.03 4 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
Problems with getEngine ( art 2) • It’s confusing. • Which engine does the following code use? double random_number() { // Common to see in art 2 ServiceHandle<RNG> rng; auto& engine = rng->getEngine(); CLHEP::RandFlat rand{engine}; return rand.fire(); } 5 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
Problems with getEngine ( art 3) • It’s confusing • Which engine does the following code use? double random_number() { // Common to see in art 3 ServiceHandle<RNG> rng; auto& engine = rng->getEngine(module_label, // Where do you get schedule_id); // these values? CLHEP::RandFlat rand{engine}; return rand.fire(); } 6 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
Problems with getEngine Common class MyProducer { public: MyProducer(ParameterSet const& pset) { ServiceHandle<NuRandomService>{} ->createEngine(...); } void produce(art::Event& e) override { auto& engine = ServiceHandle<RandomNumberGenerator>{} ->getEngine(...); CLHEP::RandFlat flatDist{engine}; flatDist.fire(...); } }; 7 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
Problems with getEngine Common class MyProducer { public: MyProducer(ParameterSet const& pset) { ServiceHandle<NuRandomService>{} ->createEngine(...); } void produce(art::Event& e) override { Expensive operations: auto& engine = ServiceHandle created for each event ServiceHandle<RandomNumberGenerator>{} ->getEngine(...); getEngine called on each event CLHEP::RandFlat flatDist{engine}; RandFlat distribution created for each event flatDist.fire(...); } }; 8 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
What’s the better option? Common Better class MyProducer { class MyProducer { public: CLHEP::RandFlat flatDist_; MyProducer(ParameterSet const& pset) public: { ServiceHandle<NuRandomService>{} MyProducer(ParameterSet const& pset) ->createEngine(...); : flatDist_{ServiceHandle<NuRandomService>{} } ->createEngine(...)} {} void produce(art::Event& e) override { void produce(art::Event& e) override auto& engine = { ServiceHandle<RandomNumberGenerator>{} flatDist_.fire(...); ->getEngine(...); } CLHEP::RandFlat flatDist{engine}; }; flatDist.fire(...); } }; 9 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
What’s the better option? Common Better class MyProducer { class MyProducer { public: CLHEP::RandFlat flatDist_; MyProducer(ParameterSet const& pset) public: { ServiceHandle<NuRandomService>{} MyProducer(ParameterSet const& pset) createEngine returns art-owned reference ->createEngine(...); : flatDist_{ServiceHandle<NuRandomService>{} } ->createEngine(...)} to engine; no need to directly interact with it {} void produce(art::Event& e) override { void produce(art::Event& e) override auto& engine = { ServiceHandle<RandomNumberGenerator>{} flatDist_.fire(...); ->getEngine(...); } CLHEP::RandFlat flatDist{engine}; }; flatDist.fire(...); } }; 10 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
What’s the better option? Common Better class MyProducer { class MyProducer { public: CLHEP::RandFlat flatDist_; MyProducer(ParameterSet const& pset) public: { ServiceHandle<NuRandomService>{} MyProducer(ParameterSet const& pset) ->createEngine(...); : flatDist_{ServiceHandle<NuRandomService>{} } ->createEngine(...)} {} void produce(art::Event& e) override { void produce(art::Event& e) override auto& engine = { ServiceHandle<RandomNumberGenerator>{} flatDist_.fire(...); ->getEngine(...); } CLHEP::RandFlat flatDist{engine}; }; flatDist.fire(...); } }; You can do this now. 11 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
To encourage better usage… • I would like to make it a compile-time warning to discard the reference returned when calling NuRandomService::createEngine . // The following statement is supported: engine_t& engine = ServiceHandle<NuRandomService>{}->createEngine(...); // The following statement generates a compile-time warning: ServiceHandle<NuRandomService>{}->createEngine(...); 12 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
To encourage better usage… • I would like to make it a compile-time warning to discard the reference returned when calling NuRandomService::createEngine . // The following statement is supported: engine_t& engine = ServiceHandle<NuRandomService>{}->createEngine(...); // The following statement generates a compile-time warning: ServiceHandle<NuRandomService>{}->createEngine(...); /home/knoepfel/scratch/larsoft-devel/srcs/ larsim/larsim/EventGenerator/LightSource_module.cc:198 :83: warning: ignoring return value of ‘std::reference_wrapper<CLHEP::HepRandomEngine> rndm::NuRandomService::createEngine (…) […]’, declared with attribute nodiscard [-Werror=unused-result] art::ServiceHandle<rndm::NuRandomService>{}->createEngine(*this, pset, "Seed2"); 13 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
To encourage better usage… • I would like to make it a compile-time warning to discard the reference returned when calling NuRandomService::createEngine . // The following statement is supported: engine_t& engine = ServiceHandle<NuRandomService>{}->createEngine(...); // The following statement generates a compile-time warning: ServiceHandle<NuRandomService>{}->createEngine(...); /home/knoepfel/scratch/larsoft-devel/srcs/ larsim/larsim/EventGenerator/LightSource_module.cc:198 :83: warning: ignoring return value of ‘std::reference_wrapper<CLHEP::HepRandomEngine> rndm::NuRandomService::createEngine (…) […]’, declared with attribute nodiscard [-Werror=unused-result] art::ServiceHandle<rndm::NuRandomService>{}->createEngine(*this, pset, "Seed2"); • In rare circumstances, the warning can be silenced by casting to void: (void) ServiceHandle<NuRandomService>{}->createEngine(...); 14 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
To encourage better usage… • I would like to make it a compile-time warning to discard the reference returned when calling NuRandomService::createEngine . // The following statement is supported: engine_t& engine = ServiceHandle<NuRandomService>{}->createEngine(...); // The following statement generates a compile-time warning: ServiceHandle<NuRandomService>{}->createEngine(...); /home/knoepfel/scratch/larsoft-devel/srcs/ larsim/larsim/EventGenerator/LightSource_module.cc:198 :83: Making this change now will position LArSoft for warning: ignoring return value of ‘std::reference_wrapper<CLHEP::HepRandomEngine> rndm::NuRandomService::createEngine (…) […]’, declared with attribute nodiscard [-Werror=unused-result] art 3.02, when getEngine is deprecated by art . art::ServiceHandle<rndm::NuRandomService>{}->createEngine(*this, pset, "Seed2"); • In rare circumstances, the warning can be silenced by casting to void: (void) ServiceHandle<NuRandomService>{}->createEngine(...); 15 1/29/19 Kyle J. Knoepfel | LArSoft coordination meeting
Recommend
More recommend