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.

Sign up for GitHub

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

Jump to bottom

migration from EDProducer to stream::EDProducer (BeamSpotOnlineProducer) #6671

Merged
merged 2 commits into from Dec 9, 2014
Merged

migration from EDProducer to stream::EDProducer (BeamSpotOnlineProducer) #6671

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8225778
migrate EDProducer to stream::EDProducer
mtosi Nov 27, 2014
12557fe
fix boost::int16 to uint32
mtosi Nov 28, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 10 additions & 7 deletions RecoVertex/BeamSpotProducer/interface/BeamSpotOnlineProducer.h
View file
Open in desktop
Expand Up @@ -12,7 +12,7 @@

________________________________________________________________**/

#include "FWCore/Framework/interface/EDProducer.h"
#include "FWCore/Framework/interface/stream/EDProducer.h"
#include "FWCore/Framework/interface/Event.h"
#include "DataFormats/Common/interface/Handle.h"
#include "FWCore/Framework/interface/ESHandle.h"
Expand All @@ -21,7 +21,7 @@ ________________________________________________________________**/
#include "DataFormats/L1GlobalTrigger/interface/L1GlobalTriggerEvmReadoutRecord.h"


class BeamSpotOnlineProducer: public edm::EDProducer {
class BeamSpotOnlineProducer: public edm::stream::EDProducer<> {

public:
typedef std::vector<edm::ParameterSet> Parameters;
Expand All @@ -32,14 +32,17 @@ class BeamSpotOnlineProducer: public edm::EDProducer {
~BeamSpotOnlineProducer();

/// produce a beam spot class
virtual void produce(edm::Event& iEvent, const edm::EventSetup& iSetup);
virtual void produce(edm::Event& iEvent, const edm::EventSetup& iSetup) override;

private:

bool changeFrame_;
double theMaxZ,theMaxR2,theSetSigmaZ;
edm::EDGetTokenT<BeamSpotOnlineCollection> scalerToken_;
edm::EDGetTokenT<L1GlobalTriggerEvmReadoutRecord> l1GtEvmReadoutRecordToken_;
const bool changeFrame_;
const double theMaxZ,theSetSigmaZ;
double theMaxR2;
const edm::EDGetTokenT<BeamSpotOnlineCollection> scalerToken_;
const edm::EDGetTokenT<L1GlobalTriggerEvmReadoutRecord> l1GtEvmReadoutRecordToken_;

const boost::uint16_t theBeamShoutMode;
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need boost:: type here.
It's also not good that the underlying parameter in python is a plain int, which can lead to surprises.
Please use the same type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was boost in the original code,
and sorry but I do not know the answer to your question :(

but I can follow your suggestion ....
I'm a little bit confused, though,
because looking at https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideAboutPythonConfigFile#Parameters
I do not find any boost::uint16_t

I'll set in line 23 like
, theBeamShoutMode ( iconf.getUntrackedParameter ("beamMode",11) )

is it fine ?
thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

The original code in .cc was

 const boost::uint16_t beamModeValue = (gtEvmReadoutRecord->gtfeWord()).beamMode();
if (beamModeValue == 11) shoutMODE=true;

note that the typedef for the beamMode is actually cms_uint16_t without boost::.
I suggest to switch to unsigned int theBeamShoutMode and uint32 in python

Copy link
Contributor

Choose a reason for hiding this comment

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

 const boost::uint16_t beamModeValue = (gtEvmReadoutRecord->gtfeWord()).beamMode();
if (beamModeValue == 11) shoutMODE=true;

==>

if (gtEvmReadoutRecord->gtfeWord()).beamMode() == theBeamShoutMode) shoutMODE=true;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks !
I'm updating the PR

};

#endif
21 changes: 8 additions & 13 deletions RecoVertex/BeamSpotProducer/plugins/BeamSpotOnlineProducer.cc
View file
Open in desktop
Expand Up @@ -15,21 +15,16 @@ using namespace edm;


BeamSpotOnlineProducer::BeamSpotOnlineProducer(const ParameterSet& iconf)
: changeFrame_ ( iconf.getParameter<bool> ("changeToCMSCoordinates") )
, theMaxZ ( iconf.getParameter<double>("maxZ") )
, theSetSigmaZ ( iconf.getParameter<double>("setSigmaZ") )
, scalerToken_ ( consumes<BeamSpotOnlineCollection> ( iconf.getParameter<InputTag>("src") ) )
, l1GtEvmReadoutRecordToken_ ( consumes<L1GlobalTriggerEvmReadoutRecord>( iconf.getParameter<InputTag>("gtEvmLabel")) )
, theBeamShoutMode ( iconf.getUntrackedParameter<int> ("beamMode",11) )
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be a tracked parameter?

Copy link
Contributor

Choose a reason for hiding this comment

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

I take that back, it is just used to make the code verbose. Untracked is the proper type.

{

scalerToken_ = consumes<BeamSpotOnlineCollection>(
iconf.getParameter<InputTag>("src"));

changeFrame_ = iconf.getParameter<bool>("changeToCMSCoordinates");

theMaxR2 = iconf.getParameter<double>("maxRadius");
theMaxR2*=theMaxR2;
theMaxZ = iconf.getParameter<double>("maxZ");

theSetSigmaZ = iconf.getParameter<double>("setSigmaZ");

l1GtEvmReadoutRecordToken_ = consumes<L1GlobalTriggerEvmReadoutRecord>(
iconf.getParameter<InputTag>("gtEvmLabel"));

produces<reco::BeamSpot>();

Expand All @@ -45,7 +40,7 @@ BeamSpotOnlineProducer::produce(Event& iEvent, const EventSetup& iSetup)
edm::Handle<L1GlobalTriggerEvmReadoutRecord> gtEvmReadoutRecord;
if (iEvent.getByToken(l1GtEvmReadoutRecordToken_, gtEvmReadoutRecord)){
const boost::uint16_t beamModeValue = (gtEvmReadoutRecord->gtfeWord()).beamMode();
if (beamModeValue == 11) shoutMODE=true;
if (beamModeValue == theBeamShoutMode) shoutMODE=true;
}
else{
shoutMODE=true;
Expand Down Expand Up @@ -87,7 +82,7 @@ BeamSpotOnlineProducer::produce(Event& iEvent, const EventSetup& iSetup)

aSpot = reco::BeamSpot( apoint,
sigmaZ,
spotOnline.dxdz(),
spotOnline.dxdz(),
f* spotOnline.dydz(),
spotOnline.width_x(),
matrix);
Expand Down