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

veszpv · 2019-01-23T15:14:08Z

This is the follow-up of the PR:
#25466
in which simulation of bad components grouped by FED channels, randomly chosen from a collision data-driven set of configurations, were introduced.

Reminder: the killing of digis happens in the digitizer during simulation. The digitizer places a list of killed channels into the event in order for the tracking to be able to inactivate rechits in the empty modules. In real collisions, the FED FW generates an error code 25 associated to the channel from which no response is received upon a trigger (e.g. the module is turned off or the TBM gets stuck). The RawToDigi module unpacks these errors and places a list of bad channels into the event for tracking. In PR25466, the RawToDigi step replaced the default empty channel list output with the list from the digitizer.

In this PR, the channel list prepared by the digitizer is packed into the simulated FEDRawData, so that the unpacker do not need to handle simulation differently from data. By default, the bad channel simulation is turned off (in the digitizer config), no FED error 25 is simulated, and the unpacker produces an empty bad channel list.

Validation was done to check that the channel list produced by the unpacker agrees with the one from the digitizer, and running the same events before and after this modification to 10_4_0 with the feature turned on, the same number of clusters are created.

@mmusich @tsusa

…he PixelFEDChannelCollection created by the digitizer directly to reco

… label as for the digis, and verifying that channels exist in the cabling map. No extra output is generated, as channel info will be packed into the FEDRawData

…en), but in any case, pack FED error 25 into data

cmsbuild · 2019-01-23T15:14:41Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-01-23T15:21:19Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25748/8146

This PR adds an extra 28KB to repository

cmsbuild · 2019-01-23T15:21:47Z

A new Pull Request was created by @veszpv (Viktor Veszpremi) for master.

It involves the following packages:

EventFilter/SiPixelRawToDigi

@cmsbuild, @perrotta, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @dkotlins, @VinInn this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

slava77 · 2019-01-23T23:27:55Z

@cmsbuild please test

cmsbuild · 2019-01-23T23:28:19Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32806/console Started: 2019/01/24 00:28

cmsbuild · 2019-01-23T23:35:10Z

-1

Tested at: 06db132

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25748/32806/summary.html

I found follow errors while testing this PR

Failed tests: ClangBuild

Clang:

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 16 COMPILER='llvm compile'

See details on the summary page.

cmsbuild · 2019-01-23T23:35:13Z

Comparison not run due to Build errors/Fireworks only changes/No short matrix requested (RelVals and Igprof tests were also skipped)

cmsbuild · 2019-01-25T14:04:44Z

+1
Tested at: 3f89ec1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25748/32840/summary.html

cmsbuild · 2019-01-25T14:04:49Z

Comparison job queued.

cmsbuild · 2019-01-25T15:55:24Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25748/32840/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3097440
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3097242
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 133 log files, 14 edm output root files, 32 DQM output files

perrotta · 2019-01-28T15:38:28Z

EventFilter/SiPixelRawToDigi/plugins/SiPixelDigiToRaw.cc

@@ -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)){


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?

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.

perrotta · 2019-01-28T15:39:48Z

EventFilter/SiPixelRawToDigi/plugins/SiPixelDigiToRaw.cc

+	if (cache->cablingTree_->findItem(path)!=nullptr) {
+	  detBadChannels.push_back(fedChannel);
+	} else {
+	  edm::LogError("SiPixelDigiToRaw")


Which is the expected frequency of this Error?

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.

How about throwing an exception then?

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?

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).

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.

perrotta · 2019-01-28T15:43:55Z

EventFilter/SiPixelRawToDigi/src/PixelDataFormatter.cc

+				       return (int(ch.fed)==fedId && ch.link==linkId(words[fedId].back()));
+				     });
+	if (badChannel!=detBadChannels->second.end()) {
+	  LogError("FormatDataException")


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

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.

tsusa · 2019-02-04T09:24:54Z

Hi @perrotta, is there anything missing to move on with the PR? We would appreciate if it could enter in 10_5_0_pre2...

perrotta · 2019-02-04T17:11:18Z

+1

Reading and propagation to tracking of the bad channels list is moved from the previous temporary location in SiPixelRawToDigi to SiPixelDigiToRaw
Default setting is that this feature stays off, and as such no effect is expected in the tests outputs, and none is seen
Validation of the bad channels list when the feature is switched on was done by the proponents, as reported in the PR description

cmsbuild · 2019-02-04T17:11:43Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

fabiocos · 2019-02-05T11:10:10Z

code-checks

cmsbuild · 2019-02-05T11:10:35Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-02-05T11:15:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25748/8324

This PR adds an extra 28KB to repository

fabiocos · 2019-02-05T20:56:41Z

+1

veszpv added 4 commits January 23, 2019 15:08

Removing mechanism that bypassed DigiToRaw+RawToDigi and transfered t…

4e1eb4f

…he PixelFEDChannelCollection created by the digitizer directly to reco

Input label not needed any longer

fe18d5e

Reading PixelFEDChannelCollection created by the digitizer using same…

36089c2

… label as for the digis, and verifying that channels exist in the cabling map. No extra output is generated, as channel info will be packed into the FEDRawData

Print log error if a channel flagged bad has digis (should never happ…

06db132

…en), but in any case, pack FED error 25 into data

cmsbuild added this to the CMSSW_10_5_X milestone Jan 23, 2019

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jan 23, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 23, 2019

cmsbuild added tests-started and removed tests-pending labels Jan 23, 2019

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Jan 23, 2019

Fixing clang warning

3f89ec1

cmsbuild removed code-checks-approved comparison-notrun tests-rejected labels Jan 25, 2019

cmsbuild added tests-approved and removed tests-started labels Jan 25, 2019

cmsbuild added comparison-available and removed comparison-pending labels Jan 25, 2019

perrotta reviewed Jan 28, 2019

View reviewed changes

slava77 mentioned this pull request Jan 31, 2019

Fix clang warnings in EventFilter/SiPixelRawToDigi #25805

Merged

cmsbuild added fully-signed and removed pending-signatures reconstruction-pending labels Feb 4, 2019

cmsbuild added the reconstruction-approved label Feb 4, 2019

cmsbuild added code-checks-pending and removed code-checks-approved labels Feb 5, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 5, 2019

cmsbuild added orp-approved and removed orp-pending labels Feb 5, 2019

cmsbuild merged commit 8ceb012 into cms-sw:master Feb 5, 2019

This was referenced Mar 1, 2019

Add alias for PixelFEDChannelCollection. #26042

Merged

[10_5_X] Add alias for PixelFEDChannelCollection #26043

Closed

tsusa mentioned this pull request May 5, 2019

Output BadFED Channel only for Phase 1 #26636

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

veszpv commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

slava77 commented Jan 23, 2019

cmsbuild commented Jan 23, 2019 •

edited

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 25, 2019

cmsbuild commented Jan 25, 2019

cmsbuild commented Jan 25, 2019

perrotta Jan 28, 2019

veszpv Jan 30, 2019

perrotta Jan 28, 2019

veszpv Jan 30, 2019

makortel Jan 30, 2019

veszpv Jan 30, 2019

makortel Jan 30, 2019

mmusich Jan 31, 2019

perrotta Jan 28, 2019

veszpv Jan 30, 2019

tsusa commented Feb 4, 2019

perrotta commented Feb 4, 2019

cmsbuild commented Feb 4, 2019

fabiocos commented Feb 5, 2019

cmsbuild commented Feb 5, 2019

cmsbuild commented Feb 5, 2019

fabiocos commented Feb 5, 2019

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

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

Conversation

veszpv commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

slava77 commented Jan 23, 2019

cmsbuild commented Jan 23, 2019 • edited

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 25, 2019

cmsbuild commented Jan 25, 2019

cmsbuild commented Jan 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsusa commented Feb 4, 2019

perrotta commented Feb 4, 2019

cmsbuild commented Feb 4, 2019

fabiocos commented Feb 5, 2019

cmsbuild commented Feb 5, 2019

cmsbuild commented Feb 5, 2019

fabiocos commented Feb 5, 2019

cmsbuild commented Jan 23, 2019 •

edited