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

Filter invalid hcal upgrade data #19100

Merged
merged 4 commits into from Jun 9, 2017
Merged

Filter invalid hcal upgrade data #19100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05018a2
Allow filtering of invalid upgrade digis
christopheralanwest Jun 5, 2017
b84744e
get QIE11 calibrations from db for filter
kpedro88 Jun 5, 2017
1d6ce1c
initialize filtered digi collections with correct size
kpedro88 Jun 6, 2017
e590f4c
use template specializations
kpedro88 Jun 8, 2017
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
2 changes: 2 additions & 0 deletions EventFilter/HcalRawToDigi/BuildFile.xml
View file
Open in desktop
Expand Up @@ -4,6 +4,8 @@
<use name="FWCore/MessageLogger"/>
<use name="CondFormats/HcalObjects"/>
<use name="FWCore/Utilities"/>
<use name="CalibFormats/HcalObjects"/>
<use name="CalibFormats/CaloObjects"/>
<use name="boost"/>
<export>
<lib name="1"/>
Expand Down
8 changes: 8 additions & 0 deletions EventFilter/HcalRawToDigi/interface/HcalDataFrameFilter.h
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@

#include "DataFormats/HcalDigi/interface/HcalDigiCollections.h"
#include "DataFormats/HcalDigi/interface/HcalUnpackerReport.h"
#include "CalibFormats/HcalObjects/interface/HcalDbService.h"

/** \class HcalDataFrameFilter

Expand All @@ -28,14 +29,21 @@ class HcalDataFrameFilter {
HcalCalibDigiCollection filter(const HcalCalibDigiCollection& incol, HcalUnpackerReport& r);
/// filter ZDC data frames
ZDCDigiCollection filter(const ZDCDigiCollection& incol, HcalUnpackerReport& r);
/// filter QIE10 data frames
QIE10DigiCollection filter(const QIE10DigiCollection& incol, HcalUnpackerReport& r);
/// filter QIE11 data frames
QIE11DigiCollection filter(const QIE11DigiCollection& incol, HcalUnpackerReport& r);
/// whether any filters are on
bool active() const;
/// get conditions
void setConditions(const HcalDbService* conditions);
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be passed in the constructor?

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 is needed on an event-by-event basis.

private:
bool requireCapid_;
bool requireDVER_;
bool energyFilter_;
int firstSample_, lastSample_;
double minimumAmplitude_;
const HcalDbService* conditions_;
Copy link
Contributor

Choose a reason for hiding this comment

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

since I was looking I'll comment early - I find this dependency very odd.... would naively belong as a es product.. (maybe) - thats how it seems to be used elsewhere..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what change you're suggesting here. The HcalDataFrameFilter class needs to be able to access QIE information from the database, in order to convert ADC to fC for the threshold calculation.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok - maybe all is fine and there is just a strange design behind - i'll let reco confirm

Copy link
Contributor

Choose a reason for hiding this comment

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

Only the HcalRawToDigi class uses the HcalDataFrameFilter:

https://github.com/cms-sw/cmssw/search?utf8=%E2%9C%93&q=HcalDataFrameFilter&type=

and HcalRawToDigi initializes the energy filter to false:

cmssw/EventFilter/HcalRawToDigi/plugins/HcalRawToDigi.cc

Line 17 in 09c3fce

false,

so I think that the energy filter code is unreachable in any case -- also a strange design.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still think it's best to have it set up correctly in case it's ever activated, but good to know.

Copy link
Contributor

Choose a reason for hiding this comment

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

pointer caching to payloads is very dangerous.
What is the lifetime of HcalDataFrameFilter ?
If it's one event, it's OK.
If it's potentially longer than an IOV, it is bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In HcalRawToDigi::produce(), this line is called in every event: filter_.setConditions(pSetup.product());.

This pattern has been used frequently in other areas of CMSSW.

};


Expand Down
5 changes: 5 additions & 0 deletions EventFilter/HcalRawToDigi/plugins/HcalRawToDigi.cc
View file
Open in desktop
Expand Up @@ -107,6 +107,7 @@ void HcalRawToDigi::produce(edm::Event& e, const edm::EventSetup& es)
edm::ESHandle<HcalElectronicsMap> item;
es.get<HcalElectronicsMapRcd>().get(electronicsMapLabel_, item);
const HcalElectronicsMap* readoutMap = item.product();
filter_.setConditions(pSetup.product());

// Step B: Create empty output : three vectors for three classes...
std::vector<HBHEDataFrame> hbhe;
Expand Down Expand Up @@ -223,10 +224,14 @@ void HcalRawToDigi::produce(edm::Event& e, const edm::EventSetup& es)
HBHEDigiCollection filtered_hbhe=filter_.filter(*hbhe_prod,*report);
HODigiCollection filtered_ho=filter_.filter(*ho_prod,*report);
HFDigiCollection filtered_hf=filter_.filter(*hf_prod,*report);
QIE10DigiCollection filtered_qie10=filter_.filter(*qie10_prod,*report);
QIE11DigiCollection filtered_qie11=filter_.filter(*qie11_prod,*report);

hbhe_prod->swap(filtered_hbhe);
ho_prod->swap(filtered_ho);
hf_prod->swap(filtered_hf);
qie10_prod->swap(filtered_qie10);
qie11_prod->swap(filtered_qie11);
}


Expand Down
69 changes: 67 additions & 2 deletions EventFilter/HcalRawToDigi/src/HcalDataFrameFilter.cc
View file
Open in desktop
@@ -1,4 +1,8 @@
#include "EventFilter/HcalRawToDigi/interface/HcalDataFrameFilter.h"
#include "CalibFormats/HcalObjects/interface/HcalCoderDb.h"
#include "CalibFormats/CaloObjects/interface/CaloSamples.h"
#include "CondFormats/HcalObjects/interface/HcalQIECoder.h"
#include "CondFormats/HcalObjects/interface/HcalQIEShape.h"

namespace HcalDataFrameFilter_impl {

Expand All @@ -18,20 +22,56 @@ namespace HcalDataFrameFilter_impl {
return true;
}

template<>
bool check<QIE10DataFrame>(const QIE10DataFrame& df, bool capcheck, bool linkerrcheck) {
if (linkerrcheck && df.linkError()) return false;
if (capcheck) {
for (int i=0; i<df.samples(); i++) {
if (!df[i].ok()) return false;
}
}
return true;
}

template<>
bool check<QIE11DataFrame>(const QIE11DataFrame& df, bool capcheck, bool linkerrcheck) {
if (linkerrcheck && df.linkError()) return false;
if (capcheck && df.capidError()) return false;
return true;
}


template <class DataFrame>
double energySum(const DataFrame& df, int fs, int ls) {
double energySum(const DataFrame& df, int fs, int ls, const HcalDbService* conditions=nullptr) {
double es=0;
for (int i=fs; i<=ls && i<=df.size(); i++)
es+=df[i].nominal_fC();
return es;
}

template <>
double energySum<QIE11DataFrame>(const QIE11DataFrame& df, int fs, int ls, const HcalDbService* conditions) {
const HcalQIECoder* channelCoder = conditions->getHcalCoder(df.id());
const HcalQIEShape* shape = conditions->getHcalShape(channelCoder);
CaloSamples tool;
HcalCoderDb coder(*channelCoder, *shape);
coder.adc2fC(df, tool);
double es=0;
for (int i=fs; i<=ls && i<=(int)df.samples(); i++)
es+=tool[i];
return es;
}

}


HcalDataFrameFilter::HcalDataFrameFilter(bool requireCapid, bool requireDVER, bool energyFilter, int firstSample, int lastSample, double minAmpl) :
requireCapid_(requireCapid), requireDVER_(requireDVER), energyFilter_(energyFilter),
firstSample_(firstSample), lastSample_(lastSample), minimumAmplitude_(minAmpl) {
firstSample_(firstSample), lastSample_(lastSample), minimumAmplitude_(minAmpl), conditions_(nullptr) {
}

void HcalDataFrameFilter::setConditions(const HcalDbService* conditions) {
conditions_ = conditions;
}

HBHEDigiCollection HcalDataFrameFilter::filter(const HBHEDigiCollection& incol, HcalUnpackerReport& r) {
Expand Down Expand Up @@ -92,6 +132,31 @@ ZDCDigiCollection HcalDataFrameFilter::filter(const ZDCDigiCollection& incol, Hc
return output;
}

QIE10DigiCollection HcalDataFrameFilter::filter(const QIE10DigiCollection& incol, HcalUnpackerReport& r) {
QIE10DigiCollection output(incol.samples());
for (QIE10DigiCollection::const_iterator i=incol.begin(); i!=incol.end(); i++) {
QIE10DataFrame df(*i);
if (!HcalDataFrameFilter_impl::check(df,requireCapid_,requireDVER_))
r.countBadQualityDigi(i->id());
// Never exclude QIE10 digis as their absence would be
// treated as a digi with zero charged deposited in that channel
output.push_back(df);
}
return output;
}

QIE11DigiCollection HcalDataFrameFilter::filter(const QIE11DigiCollection& incol, HcalUnpackerReport& r) {
QIE11DigiCollection output(incol.samples());
for (QIE11DigiCollection::const_iterator i=incol.begin(); i!=incol.end(); i++) {
QIE11DataFrame df(*i);
if (!HcalDataFrameFilter_impl::check(df,requireCapid_,requireDVER_))
r.countBadQualityDigi(i->id());
else if (!energyFilter_ || minimumAmplitude_<HcalDataFrameFilter_impl::energySum(df,firstSample_,lastSample_,conditions_))
output.push_back(df);
}
return output;
}


bool HcalDataFrameFilter::active() const {
return requireCapid_|requireDVER_|energyFilter_;
Expand Down