Migrate legacy type modules to one type

[wddgit]

PR description:

Migrate three legacy type modules to one type

ESDaqInfoTask
ESDataCertificationTask
ESDcsInfoTask

PR validation:

Relies on existing tests. Minimal changes were done to migrate this code. They should behave the same.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39616/32413

• This PR adds an extra 16KB to repository

[cmsbuild]

A new Pull Request was created by @wddgit (W. David Dagenhart) for master.

It involves the following packages:

• DQM/EcalPreshowerMonitorModule (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@rchatter, @simonepigazzini, @thomreis, @argiro this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

[wddgit]

please test

[thomreis]

Should these not rather be migrated to DQMEDAnalyzer?

[makortel]

Should these not rather be migrated to DQMEDAnalyzer?

In principle yes, but

• ESDaqInfoTask does its work in beginLuminosityBlock, which wouldn't work with DQMEDAnalyzer. It might work with DQMOneLumiEDAnalyzer, but we'd prefer to leave further improvements for @cms-sw/dqm-l2
• ESDataCertificationTask doesn't seem to do much useful work (or I failed to see it) but seems to follow the same pattern
• ESDcsInfoTask does work in beginLuminosityBlock (in addition to analyze()), so in similar arguments only minimal work away from legacy modules is done in this PR

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-997c38/28002/summary.html
COMMIT: b51c53c
CMSSW: CMSSW_12_6_X_2022-10-04-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39616/28002/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-997c38/41834.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3432650
• DQMHistoTests: Total failures: 0
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3432628
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
• Checked 204 log files, 49 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[wddgit]

ESDataCertificationTask books some histograms but never fills them. It's remotely possible some other module retrieves them from DQMStore and fills them but I doubt it. It has been in this state since 2013.

If we are going to do more than the minimal migration, then I would recommend deleting the entire module. Possibly there are some other files needing modification, maybe configurations that try to run or read the histograms. I'm not sure.

I suppose I could try to do this instead. Do you want me to delete it?

[makortel]

I suppose I could try to do this instead. Do you want me to delete it?

Since the module is being run as part of runTheMatrix, I'd leave it for DQM experts to clean up if it is indeed deemed unnecessary (unless @cms-sw/dqm-l2 instructs otherwise)

[emanueleusai]

Let's leave it like this for the time being. we will discuss with the ECAL DQM developers if they are ok with the module being removed.

[emanueleusai]

+1

[cmsbuild]

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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

[rappoccio]

+1