HLT Offline DQM: add HLT_Ele30_WPTight_Gsf and change E/gamma IDs [13_0_X]

[missirol]

backport of #42049
backport of #42294

PR description:

Verbatim backport of #42049 and #42294 by @RSalvatico. From the descriptions of the two PRs:

The purpose of this PR is to:

• add HLT_Ele30_WPTight_Gsf to the OfflineDQM among the monitored paths in HLT/EGM/TagAndProbeEffs, as requested by TSG in CMSHLT-2810;

• change the offline electron and photon IDs for EGM to the newest Run3 ones (electrons, photons);

• clean up, removing HEM/HEP plots (which were relevant in 2017) and removing trigger paths which are no longer in the menu.

and

This PR is a follow up of #42049 . In the latter, the update of the egmGsfElectronIDsForDQM module to include (via VID) only new Run3 electron IDs had created issues to non-EGM DQM, such as TOP, B2G and ParticleNet. In order to address this, the current PR updates the electron IDs used by non-EGM DQM too. Other POGs and PAGs have been informed and have followed up on CMSHLT-2876.

FYI: @cms-sw/egamma-pog-l2

PR validation:

None.

If this PR is a backport, please specify the original PR and why you need to backport that PR. If this PR will be backported, please specify to which release cycle the backport is meant for:

#42049
#42294

Update of EGM-HLT offline DQM relevant to 2023 pp data-taking.

[cmsbuild]

A new Pull Request was created by @missirol (Marino Missiroli) for CMSSW_13_0_X.

It involves the following packages:

• DQMOffline/Trigger (dqm)

@nothingface0, @emanueleusai, @cmsbuild, @pmandrik, @syuvivida, @tjavaid, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@mtosi, @Fedespring, @HuguesBrun, @jhgoh, @trocino, @cericeci, @rociovilar 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

• Backported from Addition of HLT_Ele30_WPTight_Gsf to OfflineDQM and cleanups #42049

[missirol]

please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7c10f2/33604/summary.html
COMMIT: 2bf8d5a
CMSSW: CMSSW_13_0_X_2023-07-08-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42217/33604/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially added 2167 lines to the logs
• Reco comparison results: 16 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3310377
• DQMHistoTests: Total failures: 12
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3310343
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -1497.2569999999998 KiB( 48 files compared)
• DQMHistoSizes: changed ( 1000.0,... ): -262.021 KiB HLT/EGM
• DQMHistoSizes: changed ( 12434.0,... ): -324.405 KiB HLT/EGM
• Checked 213 log files, 164 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[RSalvatico]

Thanks for the backports, @missirol!

[missirol]

hold

You potentially added 2167 lines to the logs

This requires a closer look.

[cmsbuild]

Pull request has been put on hold by @missirol
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

[perrotta]

Thank you for noticing @missirol , and apologize for not having taken care of it myself when merging the master version of this PR.

There are a huge amount of messages like

%MSG-w TopMonitor:  TopMonitor:topElecMuonHLTMonitor_Mu23Ele12_ref  08-Jul-2023 18:21:25 CEST Run: 346512 Event: 243042266
Electron ID handle not valid 

@RSalvatico they must be fixed in the master first, and then the fix should be included in these backports

[missirol]

For what I understand, the changes below removed old electron IDs from a module used in DQM.
https://github.com/cms-sw/cmssw/pull/42049/files#diff-6e31c5da031ccb91cbed539cec1d6bc3a0d8582ea3452f82f7e260d93990c6b6L983

.. but the many clients of that module were not updated accordingly, e.g.

cmssw/DQMOffline/Trigger/python/TopMonitor_cfi.py

Line 89 in c2ba821

hltTOPmonitoring.elecID = "egmGsfElectronIDsForDQM:cutBasedElectronID-Fall17-94X-V1-tight" #Electron ID

In master, I guess one should move the clients to the new ID [1]. The same could be done in 13_(0|1)_X, but maybe this will complicate comparing to older runs (where DQM clients used the old ID). For the backports, the alternative is to keep the old IDs included in egmGsfElectronIDsForDQM [2].

@cms-sw/egamma-pog-l2, please suggest how to update the backports.

[1] This could be sufficient to update the clients (to be confirmed by experts).

for foo in cutBasedElectronID-Summer16-80X-V1 cutBasedElectronID-Fall17-94X-V1 cutBasedElectronID-Fall17-94X-V2 ; do
  sed -i "s|${foo}|cutBasedElectronID-RunIIIWinter22-V1|g" DQMOffline/Trigger/*/*.*
done

[2] Doing the following on top of the current backports.

-my_id_modules = ['RecoEgamma.ElectronIdentification.Identification.cutBasedElectronID_Winter22_122X_V1_cff']
+my_id_modules = ['RecoEgamma.ElectronIdentification.Identification.cutBasedElectronID_Summer16_80X_V1_cff',
+                 'RecoEgamma.ElectronIdentification.Identification.cutBasedElectronID_Fall17_94X_V1_cff',
+                 'RecoEgamma.ElectronIdentification.Identification.cutBasedElectronID_Winter22_122X_V1_cff']

[RSalvatico]

Thanks a lot for checking, I am looking into it. I was planning to interfere the least possible with other group's DQMs so I don't dislike the option of keeping the old IDs available for other clients, but let me quickly discuss this.

[RSalvatico]

Follow up: I don't think it was my changes that generated the log messages that @perrotta mentioned, or at least not entirely. I see the same when testing for example the TopMonitor in 13_1_0 out of the box. And adding back the old ID modules to egmGsfElectronIDsForDQM in my PR makes no difference. The reason why the TopMonitor (and I guess others too) don't find a valid electron ID is not clear yet, but I'm digging.

Forgive my ignorance, is there a clear way to see which lines are being added to the logs wrt before?

[missirol]

I see the same when testing for example the TopMonitor in 13_1_0 out of the box. And adding back the old ID modules to egmGsfElectronIDsForDQM in my PR makes no difference.

I do not understand that. The IBs show the warnings in CMSSW_13_2_X_2023-07-10-2300 [1] and not in CMSSW_13_1_X_2023-07-10-2300 [2]. It may not be the full story, but it is consistent with the integration of #42049.

[1] https://cmssdt.cern.ch/SDT/jenkins-artifacts/ib-baseline-tests/CMSSW_13_2_X_2023-07-10-2300/el8_amd64_gcc11/-GenuineIntel/matrix-results/12434.0_TTbar_14TeV+2023/step3_TTbar_14TeV+2023.log

[2] https://cmssdt.cern.ch/SDT/jenkins-artifacts/ib-baseline-tests/CMSSW_13_1_X_2023-07-10-2300/el8_amd64_gcc11/-GenuineIntel/matrix-results/12434.0_TTbar_14TeV+2023/step3_TTbar_14TeV+2023.log

[RSalvatico]

Yes, my bad. I have just realized what the problem was in my testing setup: I was running the TopMonitoring sequences without running the EGM ones first - hence the required electron IDs were not produced and hence the warnings.

Following the discussion we had today in the TSG meeting, I will wait one week to collect possible feedback from other PAGs and POGs. If no objections are received, I will proceed with updating the electron IDs used by other groups too.

[missirol]

Following the discussion we had today in the TSG meeting, I will wait one week to collect possible feedback from other PAGs and POGs.

@RSalvatico , I would suggest to open a JIRA ticket in CMSHLT describing briefly this change. We can then ask explicit feedback from the relevant groups without waiting one week. We should be able to converge by Monday at the latest.

[RSalvatico]

Link to the JIRA ticket CMSHLT-2876.

[cmsbuild]

Pull request #42217 was updated. @nothingface0, @emanueleusai, @cmsbuild, @pmandrik, @syuvivida, @tjavaid, @micsucmed, @rvenditti can you please check and sign again.

[missirol]

please test

[cmsbuild]

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7c10f2/33751/summary.html
COMMIT: 432afca
CMSSW: CMSSW_13_0_X_2023-07-18-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42217/33751/install.sh to create a dev area with all the needed externals and cmssw changes.

There was an issue with git-cms-merge-topic you can see the log here: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7c10f2/33751/git-merge-result

[missirol]

please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7c10f2/33754/summary.html
COMMIT: 432afca
CMSSW: CMSSW_13_0_X_2023-07-18-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42217/33754/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially removed 10 lines from the logs
• Reco comparison results: 9 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3279595
• DQMHistoTests: Total failures: 6
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3279567
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -1497.2569999999998 KiB( 48 files compared)
• DQMHistoSizes: changed ( 1000.0,... ): -262.021 KiB HLT/EGM
• DQMHistoSizes: changed ( 12434.0,... ): -324.405 KiB HLT/EGM
• Checked 213 log files, 164 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[missirol]

unhold

Including the backport of #42294 fixed the issue causing log warnings, as expected.

[emanueleusai]

+1

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_13_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_13_3_X is complete. 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