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

PixelFed25 error propagation to tracking #20151

Merged
merged 12 commits into from Oct 2, 2017
Merged

PixelFed25 error propagation to tracking #20151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f94f3be
Created a new collection to describe a FED channel with FED number, l…
veszpv Aug 14, 2017
918f76e
1) Removed 'disabled FED channel' error (25) for disconnected FED cha…
veszpv Aug 14, 2017
1b2c227
Put back LogWarning in SiPixelFrameConverter::toRoc that was changed …
veszpv Aug 14, 2017
52ed124
1) Propagated cabling map to ErrorChecker to allow masking FED error …
veszpv Aug 14, 2017
aa06281
Added BadFEDChannelPositions information in PxMeasurementDetSet.
veszpv Aug 14, 2017
9c8105a
Populate PxMeasurementDetSet from PixelFEDChannelCollection per event…
veszpv Aug 14, 2017
c11c6c5
Inactivate bad channels in hasBadComponents, where the inactive ROCs …
veszpv Aug 14, 2017
e86c229
Adding new collection to default FEVT and RECO output
veszpv Aug 15, 2017
1a1fa22
Implemented comments
veszpv Sep 26, 2017
8d2d883
ran code-checks
veszpv Sep 27, 2017
ef534f3
friends no more
veszpv Sep 29, 2017
b1e1541
Minor correction to log message
veszpv Sep 29, 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: 1 addition & 1 deletion CondFormats/SiPixelObjects/src/SiPixelFrameConverter.cc
View file
Open in desktop
Expand Up @@ -38,7 +38,7 @@ PixelROC const * SiPixelFrameConverter::toRoc(int link, int roc) const {
stm << "Map shows no fed="<<theFedId
<<", link="<< link
<<", roc="<< roc;
LogDebug("SiPixelFrameConverter") << stm.str();
edm::LogWarning("SiPixelFrameConverter") << stm.str();
Copy link

Choose a reason for hiding this comment

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

@veszpv how often is this warning issued actually?
I am just concerned that this might fill up logs at Tier0.

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 changed to debug as a hack, I am putting the warning back now. Should never be issued after the PR. If so, it is a good reason to investigate.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't seen this show up in my test on run 300517 (LS ~600) on 4K events.

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 used to generate ~418 warning per event for uncabled FED channels.

Copy link

Choose a reason for hiding this comment

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

@veszpv, @slava77 thanks!

}
return rocp;
}
Expand Down
17 changes: 17 additions & 0 deletions DataFormats/SiPixelDetId/interface/PixelFEDChannel.h
View file
Open in desktop
@@ -0,0 +1,17 @@
#ifndef SiPixelDetId_PixelFEDChannel_H
#define SiPixelDetId_PixelFEDChannel_H

#include "DataFormats/Common/interface/DetSetVectorNew.h"

struct PixelFEDChannel {
unsigned int fed, link, roc_first, roc_last;
};

inline bool operator<( const PixelFEDChannel& one, const PixelFEDChannel& other) {
if (one.fed == other.fed) return one.link<other.link;
return one.fed < other.fed;
}

typedef edmNew::DetSetVector<PixelFEDChannel> PixelFEDChannelCollection;

#endif
22 changes: 22 additions & 0 deletions DataFormats/SiPixelDetId/src/classes.h
View file
Open in desktop
Expand Up @@ -2,3 +2,25 @@
#include <boost/cstdint.hpp>
#include "DataFormats/SiPixelDetId/interface/PXFDetId.h"

#include "DataFormats/SiPixelDetId/interface/PixelFEDChannel.h"
#include "DataFormats/Common/interface/EDCollection.h"
#include "DataFormats/Common/interface/DetSetVectorNew.h"
#include "DataFormats/Common/interface/Wrapper.h"

namespace DataFormats_PixelFEDChannel {
struct dictionary {
std::vector<PixelFEDChannel> vFC_;
edm::EDCollection<PixelFEDChannel> edcFC_;
edmNew::DetSet<PixelFEDChannel> dsFC_;
std::vector<edmNew::DetSet<PixelFEDChannel> > vdsFC_;
edmNew::DetSetVector<PixelFEDChannel> dsvFC_;
PixelFEDChannelCollection FCc_;

edm::Wrapper<std::vector<PixelFEDChannel> > vFCw_;
edm::Wrapper<edm::EDCollection<PixelFEDChannel> > edcFCw_;
edm::Wrapper<edmNew::DetSet<PixelFEDChannel> > dsFCw_;
edm::Wrapper<std::vector<edmNew::DetSet<PixelFEDChannel> > > vdsFCw_;
edm::Wrapper<edmNew::DetSetVector<PixelFEDChannel> > dsvFCw_;
edm::Wrapper<PixelFEDChannelCollection> FCcw_;
};
}
13 changes: 13 additions & 0 deletions DataFormats/SiPixelDetId/src/classes_def.xml
View file
Open in desktop
Expand Up @@ -7,4 +7,17 @@
<version ClassVersion="11" checksum="1218239916"/>
<version ClassVersion="10" checksum="64579558"/>
</class>
<class name="PixelFEDChannel" ClassVersion="3">
<version ClassVersion="3" checksum="2591256255"/>
</class>
<class name="std::vector<PixelFEDChannel>"/>
<class name="edm::EDCollection<PixelFEDChannel>"/>
<class name="edmNew::DetSet<PixelFEDChannel>"/>
<class name="std::vector<edmNew::DetSet<PixelFEDChannel> >"/>
<class name="edmNew::DetSetVector<PixelFEDChannel>"/>
<class name="edm::Wrapper<std::vector<PixelFEDChannel> >"/>
<class name="edm::Wrapper<edm::EDCollection<PixelFEDChannel> >"/>
<class name="edm::Wrapper<edmNew::DetSet<PixelFEDChannel> >"/>
<class name="edm::Wrapper<std::vector<edmNew::DetSet<PixelFEDChannel> > >"/>
<class name="edm::Wrapper<edmNew::DetSetVector<PixelFEDChannel> >"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

are all of these dictionaries really necessary, considering that only edmNew::DetSetVector is persisted (and only std::vector<PixelFEDChannel> is apparently also required )?

  • is EDCollection needed?
  • is vector<edmNew::DetSet needed?
  • is edmNew::DetSet 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.

DetSetVector is the one actually used for the bad channel propagation. So I believe EDCollection is the only line above that is not being used (explicitly or implicitly) for the time being; however, if there is no argument against keeping it, it would be a good to do so in case some user does not want to put the overhead of the DetId mapping in the event that otherwise comes with PixelFEDChannelCollection. (PixelFEDChannel is a generic class, not necessarily for what we use it here.)

</lcgdict>
2 changes: 1 addition & 1 deletion DataFormats/SiPixelRawData/src/SiPixelRawDataError.cc
View file
Open in desktop
Expand Up @@ -56,7 +56,7 @@ fedId_ = fedId;
void SiPixelRawDataError::setMessage() {
switch (errorType_) {
case(25) : {
errorMessage_ = "Error: ROC=25";
errorMessage_ = "Error: Disabled FED channel (ROC=25)";
break;
}
case(26) : {
Expand Down
2 changes: 2 additions & 0 deletions EventFilter/SiPixelRawToDigi/interface/ErrorChecker.h
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@
class FEDRawData;

class SiPixelFrameConverter;
class SiPixelFedCabling;

class ErrorChecker {

Expand All @@ -37,6 +38,7 @@ class ErrorChecker {
bool checkTrailer(bool& errorsInEvent, int fedId, int nWords, const Word64* trailer, Errors& errors);

bool checkROC(bool& errorsInEvent, int fedId, const SiPixelFrameConverter* converter,
const SiPixelFedCabling* theCablingTree,
Word32& errorWord, Errors& errors);


Expand Down
3 changes: 3 additions & 0 deletions EventFilter/SiPixelRawToDigi/interface/PixelDataFormatter.h
View file
Open in desktop
Expand Up @@ -82,6 +82,8 @@ class PixelDataFormatter {

void formatRawData( unsigned int lvl1_ID, RawData & fedRawData, const Digis & digis);

cms_uint32_t linkId(cms_uint32_t word32) { return (word32 >> LINK_shift) & LINK_mask; }

private:
mutable int theDigiCounter;
mutable int theWordCounter;
Expand All @@ -107,6 +109,7 @@ class PixelDataFormatter {
int maxROCIndex;
bool phase1;


int checkError(const Word32& data) const;

int digi2word( cms_uint32_t detId, const PixelDigi& digi,
Expand Down
81 changes: 57 additions & 24 deletions EventFilter/SiPixelRawToDigi/plugins/SiPixelRawToDigi.cc
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@
// enabled by: process.siPixelDigis.UseQualityInfo = True (BY DEFAULT NOT USED)
// 20-10-2010 Andrew York (Tennessee)
// Jan 2016 Tamas Almos Vami (Tav) (Wigner RCP) -- Cabling Map label option
// Jul 2017 Viktor Veszpremi -- added PixelFEDChannel

#include "SiPixelRawToDigi.h"

Expand Down Expand Up @@ -31,6 +32,7 @@

#include "CondFormats/SiPixelObjects/interface/SiPixelQuality.h"

#include "DataFormats/SiPixelDetId/interface/PixelFEDChannel.h"
#include "EventFilter/SiPixelRawToDigi/interface/PixelUnpackingRegions.h"
#include "FWCore/Framework/interface/ConsumesCollector.h"

Expand All @@ -42,9 +44,9 @@ using namespace std;
// -----------------------------------------------------------------------------
SiPixelRawToDigi::SiPixelRawToDigi( const edm::ParameterSet& conf )
: config_(conf),
badPixelInfo_(0),
regions_(0),
hCPU(0), hDigi(0)
badPixelInfo_(nullptr),
regions_(nullptr),
hCPU(nullptr), hDigi(nullptr)
{

includeErrors = config_.getParameter<bool>("IncludeErrors");
Expand All @@ -67,11 +69,12 @@ SiPixelRawToDigi::SiPixelRawToDigi( const edm::ParameterSet& conf )
produces< edm::DetSetVector<SiPixelRawDataError> >();
produces<DetIdCollection>();
produces<DetIdCollection>("UserErrorModules");
produces<edmNew::DetSetVector<PixelFEDChannel> >();
}

// regions
if (config_.exists("Regions")) {
if(config_.getParameter<edm::ParameterSet>("Regions").getParameterNames().size() > 0)
if(!config_.getParameter<edm::ParameterSet>("Regions").getParameterNames().empty())
{
regions_ = new PixelUnpackingRegions(config_, consumesCollector());
}
Expand All @@ -96,7 +99,7 @@ SiPixelRawToDigi::SiPixelRawToDigi( const edm::ParameterSet& conf )
usePhase1 = false;
if (config_.exists("UsePhase1")) {
usePhase1 = config_.getParameter<bool> ("UsePhase1");
if(usePhase1) edm::LogInfo("SiPixelRawToDigi") << " Use pilot blade data (FED 40)";
if(usePhase1) edm::LogInfo("SiPixelRawToDigi") << " Using phase1";
}
//CablingMap could have a label //Tav
cablingMapLabel = config_.getParameter<std::string> ("CablingMapLabel");
Expand Down Expand Up @@ -191,6 +194,7 @@ void SiPixelRawToDigi::produce( edm::Event& ev,
auto errorcollection = std::make_unique<edm::DetSetVector<SiPixelRawDataError>>();
auto tkerror_detidcollection = std::make_unique<DetIdCollection>();
auto usererror_detidcollection = std::make_unique<DetIdCollection>();
auto disabled_channelcollection = std::make_unique<edmNew::DetSetVector<PixelFEDChannel> >();

//PixelDataFormatter formatter(cabling_.get()); // phase 0 only
PixelDataFormatter formatter(cabling_.get(), usePhase1); // for phase 1 & 0
Expand Down Expand Up @@ -241,30 +245,58 @@ void SiPixelRawToDigi::produce( edm::Event& ev,
// in the configurable error list in the job option cfi.
// Code needs to be here, because there can be a set of errors for each
// entry in the for loop over PixelDataFormatter::Errors
if(!tkerrorlist.empty() || !usererrorlist.empty()){
DetId errorDetId(errordetid);
edm::DetSet<SiPixelRawDataError>::const_iterator itPixelError=errorDetSet.begin();
for(; itPixelError!=errorDetSet.end(); ++itPixelError){
// fill list of detIds to be turned off by tracking
if(!tkerrorlist.empty()) {
std::vector<int>::iterator it_find = find(tkerrorlist.begin(), tkerrorlist.end(), itPixelError->getType());
if(it_find != tkerrorlist.end()){

std::vector<PixelFEDChannel> disabledChannelsDetSet;

for (auto const& aPixelError : errorDetSet) {
// For the time being, we extend the error handling functionality with ErrorType 25
// In the future, we should sort out how the usage of tkerrorlist can be generalized
if (aPixelError.getType()==25) {
assert(aPixelError.getFedId()==fedId);
const sipixelobjects::PixelFEDCabling* fed = cabling_->fed(fedId);
if (fed) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should be fed != nullptr (surprising that clang-tidy did not flag this)

cms_uint32_t linkId = formatter.linkId(aPixelError.getWord32());
const sipixelobjects::PixelFEDLink* link = fed->link(linkId);
if (link) {
// The "offline" 0..15 numbering is fixed by definition, also, the FrameConversion depends on it
// in contrast, the ROC-in-channel numbering is determined by hardware --> better to use the "offline" scheme
PixelFEDChannel ch = {fed->id(), linkId, 25, 0};
for (unsigned int iRoc=1; iRoc<=link->numberOfROCs(); iRoc++) {
const sipixelobjects::PixelROC * roc = link->roc(iRoc);
if (roc->idInDetUnit()<ch.roc_first) ch.roc_first=roc->idInDetUnit();
if (roc->idInDetUnit()>ch.roc_last) ch.roc_last=roc->idInDetUnit();
}
disabledChannelsDetSet.push_back(ch);
}
}
} else {
// fill list of detIds to be turned off by tracking
if(!tkerrorlist.empty()) {
std::vector<int>::iterator it_find = find(tkerrorlist.begin(), tkerrorlist.end(), aPixelError.getType());
if(it_find != tkerrorlist.end()){
tkerror_detidcollection->push_back(errordetid);
}
}
}
// fill list of detIds with errors to be studied
if(!usererrorlist.empty()) {
std::vector<int>::iterator it_find = find(usererrorlist.begin(), usererrorlist.end(), itPixelError->getType());
if(it_find != usererrorlist.end()){
usererror_detidcollection->push_back(errordetid);
}
}

// fill list of detIds with errors to be studied
if(!usererrorlist.empty()) {
std::vector<int>::iterator it_find = find(usererrorlist.begin(), usererrorlist.end(), aPixelError.getType());
if(it_find != usererrorlist.end()){
usererror_detidcollection->push_back(errordetid);
}
}

} // loop on DetSet of errors

if (!disabledChannelsDetSet.empty()) {
disabled_channelcollection->insert(errordetid, disabledChannelsDetSet.data(), disabledChannelsDetSet.size());
}
}
}
}
}

} // if error assigned to a real DetId
} // loop on errors in event for this FED
} // if errors to be included in the event
} // loop on FED data to be unpacked

if(includeErrors) {
edm::DetSet<SiPixelRawDataError>& errorDetSet = errorcollection->find_or_insert(dummydetid);
Expand All @@ -289,5 +321,6 @@ void SiPixelRawToDigi::produce( edm::Event& ev,
ev.put(std::move(errorcollection));
ev.put(std::move(tkerror_detidcollection));
ev.put(std::move(usererror_detidcollection), "UserErrorModules");
ev.put(std::move(disabled_channelcollection));
}
}
4 changes: 2 additions & 2 deletions EventFilter/SiPixelRawToDigi/plugins/SiPixelRawToDigi.h
View file
Open in desktop
Expand Up @@ -30,12 +30,12 @@ class SiPixelRawToDigi : public edm::stream::EDProducer<> {
explicit SiPixelRawToDigi( const edm::ParameterSet& );

/// dtor
virtual ~SiPixelRawToDigi();
~SiPixelRawToDigi() override;

static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);

/// get data, convert to digis attach againe to Event
virtual void produce( edm::Event&, const edm::EventSetup& ) override;
void produce( edm::Event&, const edm::EventSetup& ) override;

private:

Expand Down
33 changes: 18 additions & 15 deletions EventFilter/SiPixelRawToDigi/src/ErrorChecker.cc
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ namespace {
constexpr int PXID_bits = 8;
constexpr int ADC_bits = 8;
constexpr int OMIT_ERR_bits = 1;
constexpr int AUTO_MASK_bits = 1;

constexpr int CRC_shift = 2;
constexpr int ADC_shift = 0;
Expand All @@ -33,6 +34,7 @@ namespace {
constexpr int ROC_shift = DCOL_shift + DCOL_bits;
constexpr int LINK_shift = ROC_shift + ROC_bits;
constexpr int OMIT_ERR_shift = 20;
constexpr int AUTO_MASK_shift = 20;

constexpr cms_uint32_t dummyDetId = 0xffffffff;

Expand Down Expand Up @@ -110,13 +112,16 @@ bool ErrorChecker::checkTrailer(bool& errorsInEvent, int fedId, int nWords, cons
return fedTrailer.moreTrailers();
}

bool ErrorChecker::checkROC(bool& errorsInEvent, int fedId, const SiPixelFrameConverter* converter, Word32& errorWord, Errors& errors)
bool ErrorChecker::checkROC(bool& errorsInEvent, int fedId, const SiPixelFrameConverter* converter,
const SiPixelFedCabling* theCablingTree, Word32& errorWord, Errors& errors)
{
int errorType = (errorWord >> ROC_shift) & ERROR_mask;
if likely(errorType<25) return true;

switch (errorType) {
case(25) : {
CablingPathToDetUnit cablingPath = { unsigned(fedId), (errorWord >> LINK_shift) & LINK_mask, 1 };
if (!theCablingTree->findItem(cablingPath)) return false;
LogDebug("")<<" invalid ROC=25 found (errorType=25)";
errorsInEvent = true;
break;
Expand Down Expand Up @@ -145,6 +150,15 @@ bool ErrorChecker::checkROC(bool& errorsInEvent, int fedId, const SiPixelFrameCo
}
case(30) : {
LogDebug("")<<" TBM error trailer (errorType=30)";
int StateMatch_bits = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

please make these const and unsigned ints in a follow up pr.

int StateMatch_shift = 8;
uint32_t StateMatch_mask = ~(~uint32_t(0) << StateMatch_bits);
int StateMatch = (errorWord >> StateMatch_shift) & StateMatch_mask;
if( StateMatch!=1 && StateMatch!=8 ) {
LogDebug("")<<" FED error 30 with unexpected State Bits (errorType=30)";
return false;
}
if( StateMatch==1 ) errorType = 40; // 1=Overflow -> 40, 8=number of ROCs -> 30
errorsInEvent = true;
break;
}
Expand All @@ -157,15 +171,6 @@ bool ErrorChecker::checkROC(bool& errorsInEvent, int fedId, const SiPixelFrameCo
};

if(includeErrors) {
// check to see if overflow error for type 30, change type to 40 if so
if(errorType==30) {
int StateMach_bits = 4;
int StateMach_shift = 8;
uint32_t StateMach_mask = ~(~uint32_t(0) << StateMach_bits);
int StateMach = (errorWord >> StateMach_shift) & StateMach_mask;
if( StateMach==4 || StateMach==9 ) errorType = 40;
}

// store error
SiPixelRawDataError error(errorWord, errorType, fedId);
cms_uint32_t detId;
Expand Down Expand Up @@ -232,16 +237,15 @@ cms_uint32_t ErrorChecker::errorDetId(const SiPixelFrameConverter* converter,

switch (errorType) {
case 25 : case 30 : case 31 : case 36 : case 40 : {
// set dummy values for cabling just to get detId from link if in Barrel
// set dummy values for cabling just to get detId from link
cabling.dcol = 0;
cabling.pxid = 2;
cabling.roc = 1;
cabling.link = (word >> LINK_shift) & LINK_mask;

DetectorIndex detIdx;
int status = converter->toDetector(cabling, detIdx);
if (status) break;
if(DetId(detIdx.rawId).subdetId() == static_cast<int>(PixelSubdetector::PixelBarrel)) return detIdx.rawId;
if (!status) return detIdx.rawId;
break;
}
case 29 : {
Expand Down Expand Up @@ -274,8 +278,7 @@ cms_uint32_t ErrorChecker::errorDetId(const SiPixelFrameConverter* converter,
cabling.link = chanNmbr;
DetectorIndex detIdx;
int status = converter->toDetector(cabling, detIdx);
if (status) break;
if(DetId(detIdx.rawId).subdetId() == static_cast<int>(PixelSubdetector::PixelBarrel)) return detIdx.rawId;
if (!status) return detIdx.rawId;
break;
}
case 37 : case 38: {
Expand Down
4 changes: 2 additions & 2 deletions EventFilter/SiPixelRawToDigi/src/PixelDataFormatter.cc
View file
Open in desktop
Expand Up @@ -55,7 +55,7 @@ namespace {
}

PixelDataFormatter::PixelDataFormatter( const SiPixelFedCabling* map, bool phase)
: theDigiCounter(0), theWordCounter(0), theCablingTree(map), badPixelInfo(0), modulesToUnpack(0), phase1(phase)
: theDigiCounter(0), theWordCounter(0), theCablingTree(map), badPixelInfo(nullptr), modulesToUnpack(nullptr), phase1(phase)
{
int s32 = sizeof(Word32);
int s64 = sizeof(Word64);
Expand Down Expand Up @@ -183,7 +183,7 @@ void PixelDataFormatter::interpretRawData(bool& errorsInEvent, int fedId, const

if ( (nlink!=link) | (nroc!=roc) ) { // new roc
link = nlink; roc=nroc;
skipROC = likely(roc<maxROCIndex) ? false : !errorcheck.checkROC(errorsInEvent, fedId, &converter, ww, errors);
skipROC = likely(roc<maxROCIndex) ? false : !errorcheck.checkROC(errorsInEvent, fedId, &converter, theCablingTree, ww, errors);
if (skipROC) continue;
rocp = converter.toRoc(link,roc);
if unlikely(!rocp) {
Expand Down