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

Pixel simulated bad channel info packed into FED raw data #25748

Merged
merged 5 commits into from
Feb 5, 2019
Merged

Pixel simulated bad channel info packed into FED raw data #25748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4e1eb4f
Removing mechanism that bypassed DigiToRaw+RawToDigi and transfered t…
veszpv Jan 23, 2019
fe18d5e
Input label not needed any longer
veszpv Jan 23, 2019
36089c2
Reading PixelFEDChannelCollection created by the digitizer using same…
veszpv Jan 23, 2019
06db132
Print log error if a channel flagged bad has digis (should never happ…
veszpv Jan 23, 2019
3f89ec1
Fixing clang warning
veszpv Jan 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion EventFilter/SiPixelRawToDigi/interface/PixelDataFormatter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "DataFormats/Common/interface/DetSetVector.h"
#include "EventFilter/SiPixelRawToDigi/interface/ErrorChecker.h"
#include "FWCore/Utilities/interface/typedefs.h"
#include "DataFormats/SiPixelDetId/interface/PixelFEDChannel.h"

#include <vector>
#include <map>
Expand All @@ -64,6 +65,8 @@ class PixelDataFormatter {
typedef std::pair<DetDigis::const_iterator, DetDigis::const_iterator> Range;
typedef std::vector<SiPixelRawDataError> DetErrors;
typedef std::map<cms_uint32_t, DetErrors> Errors;
typedef std::vector<PixelFEDChannel> DetBadChannels;
typedef std::map<cms_uint32_t, DetBadChannels> BadChannels;

typedef cms_uint32_t Word32;
typedef cms_uint64_t Word64;
Expand All @@ -80,7 +83,7 @@ class PixelDataFormatter {

void interpretRawData(bool& errorsInEvent, int fedId, const FEDRawData & data, Collection & digis, Errors & errors);

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

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

Expand Down
27 changes: 25 additions & 2 deletions EventFilter/SiPixelRawToDigi/plugins/SiPixelDigiToRaw.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
#include "EventFilter/SiPixelRawToDigi/interface/PixelDataFormatter.h"
#include "CondFormats/SiPixelObjects/interface/PixelFEDCabling.h"

#include "DataFormats/SiPixelDetId/interface/PixelFEDChannel.h"

#include <atomic>
#include <memory>

Expand Down Expand Up @@ -63,10 +65,11 @@ class SiPixelDigiToRaw final : public edm::global::EDProducer<edm::LuminosityBlo

private:

mutable std::atomic_flag lock_{ ATOMIC_FLAG_INIT };
mutable std::atomic_flag lock_ = ATOMIC_FLAG_INIT;
CMS_THREAD_GUARD(lock_) mutable edm::ESWatcher<SiPixelFedCablingMapRcd> recordWatcher;
CMS_THREAD_GUARD(lock_) mutable std::shared_ptr<pr::Cache> previousCache_;
const edm::EDGetTokenT<edm::DetSetVector<PixelDigi>> tPixelDigi;
const edm::EDGetTokenT<PixelFEDChannelCollection> theBadPixelFEDChannelsToken;
const edm::EDPutTokenT<FEDRawDataCollection> putToken_;
const bool usePilotBlade = false; // I am not yet sure we need it here?
const bool usePhase1;
Expand All @@ -76,6 +79,7 @@ using namespace std;

SiPixelDigiToRaw::SiPixelDigiToRaw( const edm::ParameterSet& pset ) :
tPixelDigi{ consumes<edm::DetSetVector<PixelDigi> >(pset.getParameter<edm::InputTag>("InputLabel")) },
theBadPixelFEDChannelsToken{ consumes<PixelFEDChannelCollection>(pset.getParameter<edm::InputTag>("InputLabel")) },
putToken_{produces<FEDRawDataCollection>()},
usePhase1{ pset.getParameter<bool> ("UsePhase1") }
{
Expand Down Expand Up @@ -130,6 +134,25 @@ void SiPixelDigiToRaw::produce( edm::StreamID, edm::Event& ev,

LogDebug("SiPixelDigiToRaw") << cache->cablingTree_->version();

PixelDataFormatter::BadChannels badChannels;
edm::Handle<PixelFEDChannelCollection> pixelFEDChannelCollectionHandle;
if (ev.getByToken(theBadPixelFEDChannelsToken, pixelFEDChannelCollectionHandle)){
Copy link
Contributor

Choose a reason for hiding this comment

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

What if this "if" is not satisfied? Shouldn't it throw, or give some error at least?
if so, badChannels would remain uninitialized: wouldn't it give troubles to formatter.formatRawData below, as such?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the bad component simulation is turned OFF in the digitizer, currently the default setting, the digitizer does not produce any FEDChannelCollection output. Then the if is not satisfied, and badChannels remains an empty map. In this case, no FED25 error will be generated in the RAW output.

for (auto const& fedChannels: *pixelFEDChannelCollectionHandle) {
PixelDataFormatter::DetBadChannels detBadChannels;
for(const auto& fedChannel: fedChannels) {
sipixelobjects::CablingPathToDetUnit path={fedChannel.fed, fedChannel.link, 1};
if (cache->cablingTree_->findItem(path)!=nullptr) {
detBadChannels.push_back(fedChannel);
} else {
edm::LogError("SiPixelDigiToRaw")
Copy link
Contributor

Choose a reason for hiding this comment

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

Which is the expected frequency of this Error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically should never happen if the payload based on which the digitizer kills modules lists only existing channels. This is just a safety net here.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about throwing an exception then?

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 regular production, we would definitely want to notice if this happens (frequent warning would probably trigger attention), but not sure if we would want to enforce only one possible usage of the code by preventing any exotic used cases.
E.g. should one be able to run DigiToRaw+RawToDigi in private jobs for single FEDs while processing a file with a digi collection produced once for the full detector?

Copy link
Contributor

Choose a reason for hiding this comment

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

E.g. should one be able to run DigiToRaw+RawToDigi in private jobs for single FEDs while processing a file with a digi collection produced once for the full detector?

I don't know. My experience is just that log messages are usually ignored until their amount becomes excessive. One option would be to have a boolean flag to control whether to throw or not (default being to throw).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @makortel IMHO, throwing here would be a bit of an overkill, the check that Viktor does here, if failed, doesn't lead to corrupt data or to random failures. It just checks that whatever we have put in the payload for the simulation does correspond to an actual FED channel, i.e. logs an error if we are trying to kill anything which is not physically a FED channel (e.g. single-ROCs, etc.), so it's more a question of book-keeping of inefficiency source. Also there are other failsafes in the payload producers that are meant to avoid fall in this case and indeed it should never happen.

<<" FED "<<fedChannel.fed<<" Link "<<fedChannel.link<<" for module "<<fedChannels.detId()
<<" marked bad, but this channel does not exist in the cabling map" <<endl;
}
} // channels reading a module
if (!detBadChannels.empty()) badChannels.insert({fedChannels.detId(), std::move(detBadChannels)});
} // loop on detId-s
}

//PixelDataFormatter formatter(cablingTree_.get());
PixelDataFormatter formatter(cache->cablingTree_.get(), usePhase1);

Expand All @@ -139,7 +162,7 @@ void SiPixelDigiToRaw::produce( edm::StreamID, edm::Event& ev,
FEDRawDataCollection buffers;

// convert data to raw
formatter.formatRawData( ev.id().event(), rawdata, digis );
formatter.formatRawData( ev.id().event(), rawdata, digis, badChannels);

// pack raw data into collection
for (auto const* fed: cache->cablingTree_->fedList()) {
Expand Down
21 changes: 1 addition & 20 deletions EventFilter/SiPixelRawToDigi/plugins/SiPixelRawToDigi.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ SiPixelRawToDigi::SiPixelRawToDigi( const edm::ParameterSet& conf )
usererrorlist = config_.getParameter<std::vector<int> > ("UserErrorList");
}
tFEDRawDataCollection = consumes <FEDRawDataCollection> (config_.getParameter<edm::InputTag>("InputLabel"));
theBadPixelFEDChannelsLabel = consumes<PixelFEDChannelCollection>(config_.getParameter<edm::InputTag>("BadPixelFEDChannelsInputLabel"));

//start counters
ndigis = 0;
Expand Down Expand Up @@ -153,7 +152,6 @@ SiPixelRawToDigi::fillDescriptions(edm::ConfigurationDescriptions& descriptions)
desc.add<bool>("UsePhase1",false)->setComment("## Use phase1");
desc.add<std::string>("CablingMapLabel","")->setComment("CablingMap label"); //Tav
desc.addOptional<bool>("CheckPixelOrder"); // never used, kept for back-compatibility
desc.add<edm::InputTag>("BadPixelFEDChannelsInputLabel",edm::InputTag("mix"));
descriptions.add("siPixelRawToDigi",desc);

}
Expand Down Expand Up @@ -317,29 +315,12 @@ void SiPixelRawToDigi::produce( edm::Event& ev,
hDigi->Fill(formatter.nDigis());
}


//------------------------------------
//send digis and errors back to framework

edm::Handle<PixelFEDChannelCollection> pixelFEDChannelCollectionHandle;
std::unique_ptr<PixelFEDChannelCollection> PixelFEDChannelCollection_ = nullptr;
if (ev.getByToken(theBadPixelFEDChannelsLabel, pixelFEDChannelCollectionHandle)){

const PixelFEDChannelCollection * pfcc= pixelFEDChannelCollectionHandle.product();
PixelFEDChannelCollection_ = std::make_unique<PixelFEDChannelCollection>(*pfcc);
}

ev.put(std::move(collection));
if(includeErrors){
ev.put(std::move(errorcollection));
ev.put(std::move(tkerror_detidcollection));
ev.put(std::move(usererror_detidcollection), "UserErrorModules");
if (PixelFEDChannelCollection_ == nullptr){
ev.put(std::move(disabled_channelcollection));
}
else{
ev.put(std::move(PixelFEDChannelCollection_));
}
ev.put(std::move(disabled_channelcollection));
}
}
// declare this as a framework plugin
Expand Down
2 changes: 0 additions & 2 deletions EventFilter/SiPixelRawToDigi/plugins/SiPixelRawToDigi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "DataFormats/FEDRawData/interface/FEDRawDataCollection.h"
#include "FWCore/Framework/interface/ConsumesCollector.h"
#include "FWCore/Utilities/interface/CPUTimer.h"
#include "DataFormats/SiPixelDetId/interface/PixelFEDChannel.h"

class SiPixelFedCablingTree;
class SiPixelFedCabling;
Expand Down Expand Up @@ -45,7 +44,6 @@ class SiPixelRawToDigi : public edm::stream::EDProducer<> {
const SiPixelQuality* badPixelInfo_;
PixelUnpackingRegions* regions_;
edm::EDGetTokenT<FEDRawDataCollection> tFEDRawDataCollection;
edm::EDGetTokenT<PixelFEDChannelCollection> theBadPixelFEDChannelsLabel;
TH1D *hCPU, *hDigi;
std::unique_ptr<edm::CPUTimer> theTimer;
bool includeErrors;
Expand Down
2 changes: 0 additions & 2 deletions EventFilter/SiPixelRawToDigi/python/SiPixelRawToDigi_cfi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,3 @@

from Configuration.Eras.Modifier_phase1Pixel_cff import phase1Pixel
phase1Pixel.toModify(siPixelDigis, UsePhase1=True)
from Configuration.ProcessModifiers.premix_stage2_cff import premix_stage2
premix_stage2.toModify(siPixelDigis, BadPixelFEDChannelsInputLabel = "mixData")
42 changes: 33 additions & 9 deletions EventFilter/SiPixelRawToDigi/src/PixelDataFormatter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ void PixelDataFormatter::interpretRawData(bool& errorsInEvent, int fedId, const
// }


void PixelDataFormatter::formatRawData(unsigned int lvl1_ID, RawData & fedRawData, const Digis & digis)
void PixelDataFormatter::formatRawData(unsigned int lvl1_ID, RawData & fedRawData, const Digis & digis, const BadChannels & badChannels)
{
std::map<int, vector<Word32> > words;

Expand All @@ -294,27 +294,51 @@ void PixelDataFormatter::formatRawData(unsigned int lvl1_ID, RawData & fedRawDat
if(barrel) layer = PixelROC::bpixLayerPhase1(rawId);
//if(DANEK) cout<<" layer "<<layer<<" "<<phase1<<endl;

BadChannels::const_iterator detBadChannels=badChannels.find(rawId);

hasDetDigis++;
const DetDigis & detDigis = im->second;
for (DetDigis::const_iterator it = detDigis.begin(); it != detDigis.end(); it++) {
theDigiCounter++;
const PixelDigi & digi = (*it);
int status=0;
int fedId=0;

if(layer==1 && phase1) status = digi2wordPhase1Layer1( rawId, digi, words);
else status = digi2word( rawId, digi, words);
if(layer==1 && phase1) fedId = digi2wordPhase1Layer1( rawId, digi, words);
else fedId = digi2word( rawId, digi, words);

if (status) {
if (fedId<0) {
LogError("FormatDataException")
<<" digi2word returns error #"<<status
<<" digi2word returns error #"<<fedId
<<" Ndigis: "<<theDigiCounter << endl
<<" detector: "<<rawId<< endl
<< print(digi) <<endl;
} // if (status)
} else if (detBadChannels!=badChannels.end()) {
auto badChannel=std::find_if(detBadChannels->second.begin(),
detBadChannels->second.end(),
[&] (const PixelFEDChannel& ch) {
return (int(ch.fed)==fedId && ch.link==linkId(words[fedId].back()));
});
if (badChannel!=detBadChannels->second.end()) {
LogError("FormatDataException")
Copy link
Contributor

Choose a reason for hiding this comment

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

Which is the expected frequency of this Error?
(Just to avoid floading the output with errors...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, just a safety net, should never be needed. It just checks that the digitizer consistently killed pixels in channels that it marked bad, or that the digitizer uses the same cabling map as the DigiToRaw module that called the formatRawData.

<<" while marked bad, found digi for FED "<<fedId<<" Link "<<linkId(words[fedId].back())
<<" on module "<<rawId<< endl
<< print(digi) <<endl;
}
} // if (fedId)
} // for (DetDigis
} // for (Digis
LogTrace(" allDetDigis/hasDetDigis : ") << allDetDigis<<"/"<<hasDetDigis;

// fill FED error 25 words
for(const auto& detBadChannels: badChannels) {
for(const auto& badChannel: detBadChannels.second) {
unsigned int FEDError25=25;
Word32 word = (badChannel.link << LINK_shift) | (FEDError25 << ROC_shift);
words[badChannel.fed].push_back(word);
theWordCounter++;
}
}

typedef std::map<int, vector<Word32> >::const_iterator RI;
for (RI feddata = words.begin(); feddata != words.end(); feddata++) {
int fedId = feddata->first;
Expand Down Expand Up @@ -382,7 +406,7 @@ int PixelDataFormatter::digi2word( cms_uint32_t detId, const PixelDigi& digi,
words[fedId].push_back(word);
theWordCounter++;

return 0;
return fedId;
}
int PixelDataFormatter::digi2wordPhase1Layer1( cms_uint32_t detId, const PixelDigi& digi,
std::map<int, vector<Word32> > & words) const
Expand Down Expand Up @@ -412,7 +436,7 @@ int PixelDataFormatter::digi2wordPhase1Layer1( cms_uint32_t detId, const PixelDi
words[fedId].push_back(word);
theWordCounter++;

return 0;
return fedId;
}


Expand Down