Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E/gamma T&P Offline DQM : 92X + VID + GenericTrigEventFlag Changes #19046

Merged
merged 11 commits into from Jun 7, 2017
36 changes: 22 additions & 14 deletions CommonTools/TriggerUtils/interface/GenericTriggerEventFlag.h
Expand Up @@ -41,7 +41,7 @@
class GenericTriggerEventFlag {

// Utility classes
edm::ESWatcher< AlCaRecoTriggerBitsRcd > * watchDB_;
std::unique_ptr<edm::ESWatcher< AlCaRecoTriggerBitsRcd > > watchDB_;
std::unique_ptr<L1GtUtils> l1Gt_;
std::unique_ptr<l1t::L1TGlobalUtil> l1uGt_;
HLTConfigProvider hltConfig_;
Expand Down Expand Up @@ -88,6 +88,16 @@ class GenericTriggerEventFlag {
const std::string emptyKeyError_;

public:
//so passing in the owning EDProducer is a pain for me (S. Harper)
//and its only needed for legacy/stage1 L1 info which is mostly obsolete now
//defined a new constructor which doesnt allow for the use of legacy/stage 1 L1, only stage2
//so you no longer have to pass in the EDProducer
//however I set things up such that its an error to try and configure the stage-1 L1 here
//hence the extra private constructor
//tldr: use these constructors, not the other two if unsure, if you get it wrong, there'll be an error
GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector && iC):
GenericTriggerEventFlag(config,iC){}
GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector & iC);

// Constructors must be called from the ED module's c'tor
template <typename T>
Expand All @@ -96,7 +106,6 @@ class GenericTriggerEventFlag {
template <typename T>
GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector & iC, T& module );

~GenericTriggerEventFlag();

// Public methods
bool on() { return on_ ; }
Expand All @@ -105,11 +114,9 @@ class GenericTriggerEventFlag {
bool accept( const edm::Event & event, const edm::EventSetup & setup ); // To be called from analyze/filter() methods

private:

GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector & iC, bool stage1Valid );
// Private methods

GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector & iC );

// DCS
bool acceptDcs( const edm::Event & event );
bool acceptDcsPartition( const edm::Handle< DcsStatusCollection > & dcsStatus, int dcsPartition ) const;
Expand Down Expand Up @@ -149,16 +156,17 @@ GenericTriggerEventFlag::GenericTriggerEventFlag( const edm::ParameterSet & conf

template <typename T>
GenericTriggerEventFlag::GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector & iC, T& module ) :
GenericTriggerEventFlag(config, iC) {
if ( config.exists( "andOrL1" ) )
if (stage2_)
l1uGt_.reset(new l1t::L1TGlobalUtil(config, iC));
else
l1Gt_.reset(new L1GtUtils(config, iC, false, module));
else {
l1uGt_.reset(NULL);
l1Gt_.reset(NULL);
GenericTriggerEventFlag(config, iC,true) {
if ( config.exists( "andOrL1" ) ) {
if (stage2_){
l1uGt_ = std::make_unique<l1t::L1TGlobalUtil>(config, iC);
}else{
l1Gt_ = std::make_unique<L1GtUtils>(config, iC, false, module);
}
}
//these pointers are already null so no need to reset them to a nullptr
//if andOrL1 doesnt exist
}


#endif
31 changes: 18 additions & 13 deletions CommonTools/TriggerUtils/src/GenericTriggerEventFlag.cc
Expand Up @@ -21,9 +21,19 @@ static const bool useL1EventSetup( true );
static const bool useL1GtTriggerMenuLite( false );


GenericTriggerEventFlag::GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector & iC):
GenericTriggerEventFlag(config,iC,false)
{
if ( config.exists( "andOrL1" ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Sam-Harper - given the GenericTriggerEventFlag(config,iC,false) call, is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi David,

Many thanks for the review. So I think this not strictly needed but it does simplify things for me.

I wanted to leave the original public constructor:
https://github.com/Sam-Harper/cmssw/blob/9636bcf9626f001993f6d941c1b598a8b7eb3a13/CommonTools/TriggerUtils/interface/GenericTriggerEventFlag.h#L157-L169

exactly as is. Which means all the l1 setup gets done outside the private constructor (which is now tagged with an extra argument of a bool). A dirty little secret, that bool is only there to identify the private contructor,

https://github.com/Sam-Harper/cmssw/blob/9636bcf9626f001993f6d941c1b598a8b7eb3a13/CommonTools/TriggerUtils/src/GenericTriggerEventFlag.cc#L130-L135

could easily just be at the location you mention and inside the config.exists("andOrL1") statement. But I put it in the private constructor simply because I didnt like having an unused variable.

As written, it is simpler to have all the L1 construction happening in the public constructors and a shared private constructor which does the rest of the setup. Note, the false only means it is an error that stage-1 is selected, stage-2 can be setup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, it seemed like it was just repeating things to me. Thanks to for the explanation.

if (stage2_){
l1uGt_.reset(new l1t::L1TGlobalUtil(config, iC));
}
}
}

/// To be called from the ED module's c'tor
GenericTriggerEventFlag::GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector & iC )
: watchDB_( 0 )
GenericTriggerEventFlag::GenericTriggerEventFlag( const edm::ParameterSet & config, edm::ConsumesCollector & iC, bool stage1Valid )
: watchDB_()
, hltConfigInit_( false )
, andOr_( false )
, dbLabel_( "" )
Expand Down Expand Up @@ -114,19 +124,14 @@ GenericTriggerEventFlag::GenericTriggerEventFlag( const edm::ParameterSet & conf
if ( ! onDcs_ && ! onGt_ && ! onL1_ && ! onHlt_ ) on_ = false;
else {
if ( config.exists( "dbLabel" ) ) dbLabel_ = config.getParameter< std::string >( "dbLabel" );
watchDB_ = new edm::ESWatcher< AlCaRecoTriggerBitsRcd >;
watchDB_ = std::make_unique<edm::ESWatcher< AlCaRecoTriggerBitsRcd > >();
}
}

}


/// To be called from d'tors by 'delete'
GenericTriggerEventFlag::~GenericTriggerEventFlag()
{

if ( on_ ) delete watchDB_;

//check to see we arent trying to setup legacy / stage-1 from a constructor call
//that does not support it
if ( config.exists( "andOrL1" ) && stage2_ == false ){ //stage-1 setup
if( stage1Valid==false ) throw cms::Exception("ConfigError") <<" Error when constructing GenericTriggerEventFlag, legacy/stage-1 is requested but the constructor called is stage2 only";
}
}


Expand Down
1 change: 1 addition & 0 deletions DQMOffline/Configuration/python/DQMOfflineHeavyIons_cff.py
Expand Up @@ -54,6 +54,7 @@
triggerOfflineDQMSource.remove(smpMonitorHLT)
triggerOfflineDQMSource.remove(topMonitorHLT)
triggerOfflineDQMSource.remove(btagMonitorHLT)
triggerOfflineDQMSource.remove(egammaMonitorHLT)

#egammaDQMOffline.remove(electronAnalyzerSequence)
egammaDQMOffline.remove(zmumugammaAnalysis)
Expand Down
77 changes: 77 additions & 0 deletions DQMOffline/Trigger/interface/FunctionDefs.h
@@ -0,0 +1,77 @@
#ifndef DQMOffline_Trigger_FunctionDefs_h
#define DQMOffline_Trigger_FunctionDefs_h

//***************************************************************************
//
// Description:
// These are the functions we wish to access via strings
// The function returns a std::function holding the function
// Have to admit, I'm not happy with the implimentation but fine
// no time to do something better
//
// There are various issues. The main is the awkward specialisations
// needed for the different types. Its a nasty hack. Probably
// can do it cleaner with ROOT dictionaries
//
// Useage:
// 1) define any extra functions you need at the top (seed scEtaFunc as example)
// 2) generic functions applicable to all normal objects are set in
// getUnaryFuncFloat (if they are floats, other types will need seperate
// functions which can be done with this as example
// 3) object specific functions are done with getUnaryFuncExtraFloat
// by specialising that function approprately for the object
// 4) user should simply call getUnaryFuncFloat()
//
// Author: Sam Harper (RAL) , 2017
//
//***************************************************************************

#include "FWCore/Utilities/interface/Exception.h"

#include <vector>
#include <functional>

#include "DataFormats/EgammaCandidates/interface/GsfElectron.h"

namespace hltdqm {
//here we define needed functions that otherwise dont exist
template<typename ObjType> float scEtaFunc(const ObjType& obj){return obj.superCluster()->eta();}


//additional functions specific to a given type (specialised later on)
template<typename ObjType>
std::function<float(const ObjType&)> getUnaryFuncExtraFloat(const std::string& varName){
std::function<float(const ObjType&)> varFunc;
return varFunc;
}

//the generic function to call
template<typename ObjType>
std::function<float(const ObjType&)> getUnaryFuncFloat(const std::string& varName){
std::function<float(const ObjType&)> varFunc;
if(varName=="et") varFunc = &ObjType::et;
else if(varName=="pt") varFunc = &ObjType::pt;
else if(varName=="eta") varFunc = &ObjType::eta;
else if(varName=="phi") varFunc = &ObjType::phi;
else varFunc = getUnaryFuncExtraFloat<ObjType>(varName);
//check if we never set varFunc and throw an error for anything but an empty input string
if(!varFunc && !varName.empty()){
throw cms::Exception("ConfigError") <<"var "<<varName<<" not recognised "<<__FILE__<<","<<__LINE__<<std::endl;
}
return varFunc;
}


template<>
std::function<float(const reco::GsfElectron&)> getUnaryFuncExtraFloat<reco::GsfElectron>(const std::string& varName){
std::function<float(const reco::GsfElectron&)> varFunc;
if(varName=="scEta") varFunc = scEtaFunc<reco::GsfElectron>;
else if(varName=="hOverE") varFunc = &reco::GsfElectron::hcalOverEcal;
return varFunc;
}


}


#endif