DQM: remove support for References inside CMSSW #28038
Conversation
The only direct usages in SiStrip and Pixel phase0 DQM where actually
never called, so it is probably save to remove them. We don't load
references in production jobs for a while now, and the references used
before where massively outdated.
They main usage was Chi2 and Kologrov quality tests. One main user was
Muon DQM, however they switched (?) to an implementation outside CMSSW
a while ago ("AutoDQM"), so I assume the exisiting code did not work
correctly any more. Other users are HLT and L1T DQM.
We will not support reference histograms inside CMSSW any further, for
the future we might provide a different method of comparing to
references as a "stand-alone" module in CMSSW harvesting (without
special functionality in DQMStore).
In current practise, comparisons to reference histograms are done in
external tools such as DQMGUI, RelMon, or AutoDQM.
|
hold I still see a problem in the local tests. |
|
The code-checks are being triggered in jenkins. |
schneiml
force-pushed
the
dqm-reduce-api
branch
from
c104117
to
eb1f08c
Compare
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28038/11968
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
unhold Looks ok now, need to check if any summaries show up empty. |
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28038/11969
|
|
A new Pull Request was created by @schneiml (Marcel Schneider) for master. It involves the following packages: DQM/CSCMonitorModule @cmsbuild, @andrius-k, @kmaeshima, @schneiml, @Martin-Grunewald, @jfernan2, @fioriNTU, @fwyzard can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test Let's see what happens. |
|
The tests are being triggered in jenkins. |
|
+1 |
|
Yes, I had a private mail exchange with the author of the PR to understand it better, but then my attention got diverted by other more urgent tasks. |
|
@peruzzim I don't think there is a big urgency to integrate this PR, if it is just a matter of a few more days |
|
hold After more discussion with @schneiml, it turns out we are actually using this feature to validate nanoAOD. We have devised a quick workaround and a longer term strategy to modify our validation workflow, I will unhold as soon as the first is implemented. |
|
Pull request has been put on hold by @peruzzim |
|
@peruzzim , for my education, could you point me to the code using the internal CMSSW reference for validating nanoAOD? I personally think that, for any possible use case, having a reference outside CMSSW is much more flexible. And, also, why nanoAOD can not be validated with RelMon? |
|
unhold I have modified the nanoAOD validation script to avoid using references in CMSSW, by running up to the harvesting included without references, comparing the DQM histograms with reference ones only at the very end, and then filtering the DQM histogram file to only select only the changed histograms and upload them as a separate dataset to DQM. |
|
+xpog for the change to PhysicsTools/NanoAOD/test/dqmQualityTests.xml which removes functionality not supported anymore |
|
@peruzzim thanks a lot! We probably still want to have a discussion on what exactly you need from DQM, just so we know and can take that into account going forward. |
|
+1 |
cmsbuild
added
analysis-approved
fully-signed
and removed
analysis-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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
These were forgotten in cms-sw#28038, since the build system does not know about the dependences via QTests. The xml files only use simple comparison-to-reference QTests, so I removed them, along with the Python configs that use them, since these seem to target validation of CMSSW_2 series releases and are probably useless today.
PR description:
For the modernization of the core DQM infrastrucutre in CMSSW, we decided to drop some rarely used features. This PR removes the first of these: Reference histograms.
The goal is not to remove all of the related code (instead, we will benefit from not having to re-implement it in future), but to remove all usages to see if anything important relies on the references.
The main usage are the Kolmogorov and Chi2 based QTests, which are used in many places. However, since we did not have reasonable reference runs set for a long time now, I suspect that all the output based on these is essentially useless. This may not be true for some validation workflows, but then Validation often uses it's own implementation of comparisons.
Additionally, this PR removes some more old test cases, which happen to be the only usage of some features.
PR validation:
Compiles, and seems to run. However, some references to removed histograms might remain. Also, we need to validate that indeed nobody relies on CMSSW-side references any more.