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

L1T DQM Offline Stage 2 upgrades #18085

Merged

Conversation

cbbrainerd
Copy link

Changed L1T DQM Offline sequences to use L1T Stage 2 modules and to produce the same histograms as L1T DQM Online sequences.

Based on and supersedes #16095, #16639, #17739, and #18025.

Work by @cbbrainerd and @z4027163.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 25, 2017

A new Pull Request was created by @cbbrainerd (Christopher Brainerd) for master.

It involves the following packages:

DQM/L1TMonitorClient
DQMOffline/L1Trigger

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

cms-bot commands are listed here

@cbbrainerd cbbrainerd changed the title Implemented usage of Offline L1TStage2 modules and produce same histo… L1T DQM Offline Stage 2 upgrades Mar 25, 2017
@rekovic
Copy link
Contributor

rekovic commented Mar 28, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 28, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18758/console Started: 2017/03/28 16:12

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@rekovic
Copy link
Contributor

rekovic commented Apr 3, 2017

+1

@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2017

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 requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar

@rekovic
Copy link
Contributor

rekovic commented May 5, 2017

+1

@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented May 9, 2017

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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 4f15b52 into cms-sw:master May 9, 2017
@thomreis
Copy link
Contributor

thomreis commented May 9, 2017

Hi @cbbrainerd @z4027163 ,
we would need a backport of this to 91x.

@thomreis
Copy link
Contributor

thomreis commented May 9, 2017

Scratch that. We are going to do the backport of this PR and #18616 together in one. @astakia will take care of that.

@davidlange6
Copy link
Contributor

I see that I missed the firestorm of new printouts introduced by this PR. Please fix those asap. Run for example workflow 1325

%MSG-e L1TCaloLayer1RawToDigi: L1TCaloLayer1RawToDigi:l1tCaloLayer1Digis 11-May-2017 06:30:54 CEST Run: 1 Event: 110
Could not load FED data for 1354, putting empty collections!
%MSG
%MSG-e L1TCaloLayer1RawToDigi: L1TCaloLayer1RawToDigi:l1tCaloLayer1Digis 11-May-2017 06:30:54 CEST Run: 1 Event: 110
Could not load FED data for 1356, putting empty collections!
%MSG
%MSG-e L1TCaloLayer1RawToDigi: L1TCaloLayer1RawToDigi:l1tCaloLayer1Digis 11-May-2017 06:30:54 CEST Run: 1 Event: 110
Could not load FED data for 1358, putting empty collections!
%MSG

@davidlange6
Copy link
Contributor

or these

%MSG-w MonitorElement: L1TStage2MuonComp:l1tdeStage2uGMT@streamBeginRun 11-May-2017 12:32:02 CEST Run: 1 Stream: 6
*** MonitorElement: WARNING:setBinLabel: attempting to set label of non-existent bin number for ME: L1TEMU/L1TdeStage2uGMT/data_vs_emulator_comparison/summary

%MSG
%MSG-w MonitorElement: L1TStage2MuonComp:l1tdeStage2uGMT@streamBeginRun 11-May-2017 12:32:02 CEST Run: 1 Stream: 6
*** MonitorElement: WARNING:setBinLabel: attempting to set label of non-existent bin number for ME: L1TEMU/L1TdeStage2uGMT/data_vs_emulator_comparison/summary

%MSG
%MSG-w MonitorElement: L1TStage2MuonComp:l1tdeStage2uGMT@streamBeginRun 11-May-2017 12:32:02 CEST Run: 1 Stream: 0
*** MonitorElement: WARNING:setBinLabel: attempting to set label of non-existent bin number for ME: L1TEMU/L1TdeStage2uGMT/data_vs_emulator_comparison/summary

%MSG
%MSG-w MonitorElement: L1TStage2MuonComp:l1tdeStage2uGMT@streamBeginRun 11-May-2017 12:32:02 CEST Run: 1 Stream: 0
*** MonitorElement: WARNING:setBinLabel: attempting to set label of non-existent bin number for ME: L1TEMU/L1TdeStage2uGMT/data_vs_emulator_comparison/summary

@thomreis
Copy link
Contributor

@davidlange6 the warning messages are fixed in #18662 .

Copy link
Contributor

@rekovic rekovic left a comment

Choose a reason for hiding this comment

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

@davidlange6 Regarding the error from the CaloLayer1 unpacker.
I will turn it to LogInfo and turn it off after 5 occurrences.

from DQM.L1TMonitor.L1TStage2_cff import *

stage2UnpackPath = cms.Sequence(
l1tCaloLayer1Digis +
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be ran in case of MC, since we don't pack CaloLayer1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or we put in the CaloLayer1 packer in the DIGI2RAW, which is the right thing to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the moment, leave the packer in the sequence, and turn the LogError into LogInfo in case of no FED found, silenced after 5 occurrences.

Copy link
Contributor

Choose a reason for hiding this comment

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

packer...to say unpacker.

@davidlange6
Copy link
Contributor

davidlange6 commented May 11, 2017 via email

@Dr15Jones
Copy link
Contributor

As a reminder, what ever mechanism you use to keep track of the number of messages sent, it must be thread safe.

#

Stage2l1TriggerOnline = cms.Sequence(
stage2UnpackPath
Copy link
Contributor

Choose a reason for hiding this comment

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

We simply do not need the stage2UnpackPath when we run DQMOffline....so this should just go !

@rekovic
Copy link
Contributor

rekovic commented May 11, 2017

@davidlange6

why do you expect the error message to happen at all?

You are right...should not be there.
The problem was the new unpacker sequence which should not be there when we run Offline DQM.
https://github.com/cms-sw/cmssw/pull/18085/files#r116023664.
All needed should be done in DIGI2RAW RAW2DIGI step.

@davidlange6
Copy link
Contributor

davidlange6 commented May 11, 2017 via email

@rekovic
Copy link
Contributor

rekovic commented May 11, 2017

@davidlange6
Here is a recap.

The error messages reported in #18085 (comment) only happen when L1T DQMOffline is ran on MC.

This PR invoked the L1TDQM from Online, which in itself has the unpacker sequence, unpacking every stage of L1T, runs the emulator, and then does the comparison of unpacked L1T and emulated L1T and stores it in the DQM histograms. As L1T DQM Online is designed to run on DATA, running
it as part of the DQM Offline on DATA runs without a problem.

However, if one runs on MC RAW, there are some parts of L1T which are not packed in RAW.
So, some of the L1T unpackers complain, and also some parts of L1T DQM.
These are now adjusted properly to handle this in #18703.

The moral of the story: we need to develop the missing packers. We are working on it, and hopefully will make it in 92X.

The warning messages reported in #18085 (comment) are fixed in #18662, as mentioned by @thomreis.

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

9 participants