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.

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

Pr92x L1T UnpackerCaloLayer1 & DQMCaloLayer1 Adjust for Non-present FED data in MC #18703

Conversation

rekovic
Copy link
Contributor

@rekovic rekovic commented May 11, 2017

92x: This fixes the error messages reported by @davidlange6 in #18085 (comment), which arise only in MC, wherewe still don't pack data for CaloLayer1.

Once the CaloLayer1 packer developed, this will not be an issue.
With this PR, the unpacker issues a LogInfo if FED data not found, and is silenced after 5 times.
Again, this only happens when running on RAW MC.

Also the DQM CaloLayer1 issues a LogInfo in case the emulator of CaloLayer1 and unpacked raw data disagree. In case of MC, they do as unpacked data collection is empty.

…a and emulation. The missmatch goes into the DQM histogram towerCountMismatchesPerBx anyhow.
@cmsbuild cmsbuild added this to the CMSSW_9_2_X milestone May 11, 2017
@rekovic
Copy link
Contributor Author

rekovic commented May 11, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 11, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19779/console Started: 2017/05/12 00:49

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rekovic for master.

It involves the following packages:

DQM/L1TMonitor
EventFilter/L1TXRawToDigi

@dmitrijus, @cmsbuild, @rekovic, @vanbesien, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @kreczko, @thomreis this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3376 differences found in the comparisons
  • DQMHistoTests: Total files compared: 24
  • DQMHistoTests: Total histograms compared: 1830138
  • DQMHistoTests: Total failures: 47474
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1782484
  • DQMHistoTests: Total skipped: 180
  • DQMHistoTests: Total Missing objects: 0
  • Checked 98 log files, 14 edm output root files, 24 DQM output files

@@ -168,7 +171,10 @@ L1TCaloLayer1RawToDigi::produce(Event& iEvent, const EventSetup& iSetup)
const uint64_t *fedRawDataArray = (const uint64_t *) fedRawData.data();

if ( fedRawData.size() == 0 || fedRawDataArray == nullptr ) {
LogError("L1TCaloLayer1RawToDigi") << "Could not load FED data for " << fed << ", putting empty collections!";
if (warnsa<5) {
Copy link
Contributor

@fwyzard fwyzard May 12, 2017

Choose a reason for hiding this comment

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

Note that this will print the LogInfo 5 times per stream, not just 5 times per job.

@Dr15Jones is there a way to set the default limit on the MessageLogger from within the C++ code ?
In the full process configuration one can do

process.MessageLogger.categories.append("L1TCaloLayer1RawToDigi")
process.MessageLogger.L1TCaloLayer1RawToDigi = cms.untracked.PSet(
    limit = cms.untracked.int32( 5 )
)

Is there a way to do the same in C++ within the module that will print the messages ?
Something like

if (edm::Service<MessageLogger>().isAvailable())
  edm::Service<MessageLogger>()->category("L1TCaloLayer1RawToDigi").setDefaultLimit(5);

?

@rekovic
Copy link
Contributor Author

rekovic commented May 12, 2017

+1

@rekovic
Copy link
Contributor Author

rekovic commented May 12, 2017

I see now the above comment/question of @fwyzard. Thanks.

If there is such a way to control the number of MessageLogger messages within C++, will be happy to change it.

@davidlange6
Copy link
Contributor

davidlange6 commented May 12, 2017 via email

…nfo not a LogError, and silence after 5 times."

This reverts commit cb8218b.
@cmsbuild
Copy link
Contributor

cmsbuild commented May 15, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19816/console Started: 2017/05/15 09:44

@cmsbuild
Copy link
Contributor

Pull request #18703 was updated. @dmitrijus, @cmsbuild, @rekovic, @vanbesien, @mulhearn, @davidlange6 can you please check and sign again.

…s in data and emulation. The missmatch goes into the DQM histogram towerCountMismatchesPerBx anyhow."

This reverts commit cb4b9f6.
…ta and emulation. The missmatch goes into the DQM histogram towerCountMismatchesPerBx anyhow.
@rekovic
Copy link
Contributor Author

rekovic commented May 15, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 15, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19817/console Started: 2017/05/15 10:02

@cmsbuild
Copy link
Contributor

Pull request #18703 was updated. @dmitrijus, @cmsbuild, @rekovic, @vanbesien, @mulhearn, @davidlange6 can you please check and sign again.

@rekovic
Copy link
Contributor Author

rekovic commented May 15, 2017

@davidlange6

unfortunately printing 5 times (let alone 5*nThreads) does not scale (imagine if all 1000s of modules were programmed with that design). If the error is expected, reduce it to LogDebug. (and how can we not have an unpacker for this stuff?)

Done.
It is not that we don't have the unpacker. It is the packers that we are missing.

@davidlange6
Copy link
Contributor

davidlange6 commented May 15, 2017 via email

@rekovic
Copy link
Contributor Author

rekovic commented May 15, 2017

@davidlange6
We should. It is one of the priorities...that would enable us to treat RAW EventContent in MC and DATA the same way, and simplify things.

@rekovic
Copy link
Contributor Author

rekovic commented May 15, 2017

@davidlange6 Goal is to have it in 92X.

@rekovic
Copy link
Contributor Author

rekovic commented May 15, 2017

@dmitrijus can you please review & comment/sign this and @18703.
Thanks.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • You potentially added 2893 lines to the logs
  • Reco comparison results: 3376 differences found in the comparisons
  • DQMHistoTests: Total files compared: 24
  • DQMHistoTests: Total histograms compared: 1830730
  • DQMHistoTests: Total failures: 32884
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1797666
  • DQMHistoTests: Total skipped: 180
  • DQMHistoTests: Total Missing objects: 0
  • Checked 98 log files, 14 edm output root files, 24 DQM output files

@davidlange6
Copy link
Contributor

@rekovic - so this does reduce the L1 noise - things still remaining that seem to happen every event..

%MSG-e L1TStage2CaloLayer2Offline: L1TStage2CaloLayer2Offline:l1tStage2CaloLayer2OfflineDQMEmu 16-May-2017 10:41:04 CEST Run: 1 Event: 13
invalid collection: L1 ET sums
%MSG
%MSG-e L1TStage2CaloLayer2Offline: L1TStage2CaloLayer2Offline:l1tStage2CaloLayer2OfflineDQMEmu 16-May-2017 10:41:04 CEST Run: 1 Event: 13
invalid collection: L1 jets
%MSG
%MSG-e L1TEGammaOffline: L1TEGammaOffline:l1tEGammaOfflineDQMEmu 16-May-2017 10:41:04 CEST Run: 1 Event: 13
invalid collection: L1 EGamma
%MSG

@davidlange6 davidlange6 merged commit e0c1840 into cms-sw:master May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants