DQM: Make DQMProvInfo not use lumi transitions. #29907
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29907/15499
|
|
A new Pull Request was created by @schneiml (Marcel Schneider) for master. It involves the following packages: DQMServices/Components @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
Ok, 700kB added is more than I expected, but exactly consistent with the reportSummarMap now being a profile. The comparison seems to be somewhere between perfectly clean and confused, I'll look at it in more detail tomorrow. |
|
Ah, the GUI seems to have trouble rendering the TProfile based plot, and it is blacklisted now. Good to have checked... Edit: The render plugin strictly expects a TH2F, which is not that surprising in hindsight. I'll convert the code to keep producing a TH2F; that is a bit less elegant but more robust than changing the render plugin. |
The render plugin does not like the Profile. Also saves space. This is more complicated/fragile, but fine.
|
The code-checks are being triggered in jenkins. |
|
I missed part of the logic used for the legacy (SCAL) DCS status extraction. Should be fixed now. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29907/15532
|
|
Pull request #29907 was updated. @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please check and sign again. |
|
please test now things should be fine. |
|
The tests are being triggered in jenkins. |
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1 looks indeed good. |
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 now be reviewed by the release team before it's merged. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
As pointed out in #25090 ,
DQMProvInfois still aDQMOneLumiEDAnalyzer.Rather than giving it the 1:1 port to lumi caches, I restructured it in a way that it does not need lumi transitions at all: In part, to have a nice example for how to do this; in part, because I think this is easier to understand; and in part, because it gives more correct output.
This is done by using a
TProfile(edit: not any more) to aggregate data within a lumisection, and keeping state in local variables rather than instance variables. The code could easily run asDQMEDAnalyzerorDQMGlobalEDAnalyzerapart from one logical race condition aroundsetupLumiSection, which is needed to treat "never seen" different from "empty". There would be other ways to do this, butDQMOneEDAnalyzershould be ok for such a small/fast analyzer.However, this means the semantics have slightly changed, and a more thorough validation is needed.
Themain difference is that now(Edit: switched back to TH2F for compatibility with the render plugin).reportSummaryMapis aTProfile2D, where each bin is the percentage (0..1) of events where this flag was set. This matches the old definition in the corner cases, but provides more information on the transitions. It also handles merging more correctly (though we should never merge inside lumis, so this is not really relevant).A side effect of the design change is that empty LS now show up as not valid, while they were valid before. In case of missing information, bins will be white, instead of red. I think this is acceptable.
Finally, the valid flag for each lumi is set as soon as at least one event of it is processed, while before it was set when the lumisection is completely processed. The latter semantics are hard to achieve in modern CMSSW, since we never really know if we have seen the full lumi or not. (It also does not really mean much in online, since this plot is created in a different client compared to most others, and its indistinguishable in the offline output).
PR validation:
Not much so far. PR tests should provide a good check on whether things work correctly.