DQM: Fix empty lumis with per-lumi plots in DQMOneEDAnalyzer #29738
Conversation
As reported in cms-sw#29605, it can happen that a DQMOneEDAnalyzer does not produce it's per lumi MEs, because there were no events in the Lumisection and it only calls enterLumi as needed per event. To prevent this, we need to make sure that lumi ME are always created for every ME, but we can't have lumitransitions in DQMOneEDAnalyzer, so it needs to happen in a global callback. But there we can't safely use enterLumi, since that would corrupt the module's local MEs. The solution is to have a new method, initLumi, dedicated to initializing global MEs, but not touching local MEs. This is actually a nice thing in general, since now initLumi/cleanupLumi form a symmetrical pair (create/destroy global MEs), as well as enterLumi/leavLumi (update local MEs). All of these are idempotent, as before. There are a bunch of corner cases aorund booking: initLumi *must* be called before enterLumi, but the global callback that triggers it globally might (will) happen before booking has happened for the module. So we also need to do it after booking. There is a race related to lumi MEs if globalBeginLumi can happen *before* beginRun finishes for all plugins. This should not happen for now.
Turns out EmptySource does not really support that...
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29738/15141
|
|
A new Pull Request was created by @schneiml (Marcel Schneider) for master. It involves the following packages: DQMServices/Core @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test workflow 4.28 134.807 136.757 1360.0 25210.0 |
|
ping @cmsbuild |
|
@cmsbuild please test workflow 4.28,134.807,136.757,1360.0,25210.0 |
|
The tests are being triggered in jenkins.
|
|
@silviodonato tests were not started as the comment was not wrong. Please see http://cms-sw.github.io/cms-bot-cmssw-cmds.html for correct syntax. Basically you have to use |
thanks! |
|
+1 |
|
Comparison job queued. |
|
merge |
|
+1 |
|
Comparison job queued. |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
See #29743 |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
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 be automatically merged. |
PR description:
As reported in #29605, it can happen that a
DQMOneEDAnalyzerdoes notproduce its per lumi MEs, because there were no events in the
Lumisection and it only calls
enterLumias needed per event.To prevent this, we need to make sure that lumi ME are always created
for every ME, but we can't have lumi transitions in
DQMOneEDAnalyzer, soit needs to happen in a global callback. But there we can't safely use
enterLumi, since that would corrupt the module's local MEs.The solution is to have a new method,
initLumi, dedicated toinitializing global MEs, but not touching local MEs. This is actually a
nice thing in general, since now
initLumi/cleanupLumiform a symmetricalpair (create/destroy global MEs), as well as
enterLumi/leaveLumi(updatelocal MEs). All of these are idempotent, as before.
There are a bunch of corner cases around booking:
initLumimust becalled before
enterLumi, but the global callback that triggers itglobally might (will) happen before booking has happened for the module.
So we also need to do it after booking. There is a race related to lumi
MEs if
globalBeginLumican happen beforebeginRunfinishes for allplugins. This should not happen for now, as I understand.
This should fix #29605.
The "empty lumis" unit test is quite ugly, since
EmptySourcedoes not really support empty lumisections.PR validation:
4.28 passes in my local test. A new unit test tests empty lumisections.