Fix for the access of haloMVATaggerVal in the Run 1+2 files

[jainshilpi]

This PR fixes the problem caused by the PR: #36901 , first caught here: #37143
PR #36901 was missing the correct treatment to Run 1+2 files (e.g. WF 136.7611) which essentially picks the 2016 AOD files and tries to find the value of haloTaggerMVAVal_. Since this variable does not exist in those AOD files (which was added by PR: #36901 ), this was just taking some junk value and puts in haloTaggerMVAVal().
This PR fixes this issue by having the appropriate ioread rules.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37156/28706

• This PR adds an extra 20KB to repository

[cmsbuild]

A new Pull Request was created by @jainshilpi for master.

It involves the following packages:

• DataFormats/EgammaCandidates (reconstruction)

@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @rovere, @lgray, @sobhatta, @afiqaize, @wrtabb, @varuns23, @ram1123 this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[perrotta]

please test
(Thank you @jainshilpi !)

[perrotta]

type bugfix

[swagata87]

hi Shilpi,
do I understand correctly that the BDT score returned by haloTaggerMVAVal() is such that beam halos peak at a lower value (around 0) and signal photons peak at higher value (around 1). And in analysis level, people will check something like
if haloTaggerMVAVal()<say 0.7 then reject the photon as it is beam halo.
If this is the case, then +99 might be better than -99, ie, <![CDATA[haloTaggerMVAVal_ = 99;]]>
Although it is unlikely that someone will reminiAOD Run2 data again, but +99 is safer.. just in case..

[jainshilpi]

hi Shilpi, do I understand correctly that the BDT score returned by haloTaggerMVAVal() is such that beam halos peak at a lower value (around 0) and signal photons peak at higher value (around 1). And in analysis level, people will check something like if haloTaggerMVAVal()<say 0.7 then reject the photon as it is beam halo. If this is the case, then +99 might be better than -99, ie, <![CDATA[haloTaggerMVAVal_ = 99;]]> Although it is unlikely that someone will reminiAOD Run2 data again, but +99 is safer.. just in case..

Hi Swagata, I was thinking about it. If that is preferred default value, I can do the update.

[jainshilpi]

As @swagata87 mentioned above, I also think its safer to have 99 instead of -99 so that the photons are not rejected just in case this variable is used on the Run1/2 files. The corresponding update is done.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37156/28710

• This PR adds an extra 20KB to repository

[cmsbuild]

Pull request #37156 was updated. @jpata, @cmsbuild, @clacaputo, @slava77 can you please check and sign again.

[jpata]

@cmsbuild please test

[jainshilpi]

Just pushed the changes based on the recent suggestions - checked on 136.7611 and the one with run 3 conditions, it seems all fine.

[jpata]

test parameters:

• workflows = 136.7611

[jpata]

@jainshilpi should this value be changed, too?

cmssw/DataFormats/EgammaCandidates/src/Photon.cc

Line 11 in 24fa880

haloTaggerMVAVal_(-999) {}

Otherwise, in the future, it might be confusing why some photons have mva=99, some mva=-999, and some have a meaningful value.

[jainshilpi]

@jainshilpi should this value be changed, too?

cmssw/DataFormats/EgammaCandidates/src/Photon.cc

Line 11 in 24fa880

haloTaggerMVAVal_(-999) {}

Otherwise, in the future, it might be confusing why some photons have mva=99, some mva=-999, and some have a meaningful value.

ah yes sorry - fixed that now - thanks @jpata for catching.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37156/28717

• This PR adds an extra 28KB to repository

[cmsbuild]

Pull request #37156 was updated. @jpata, @cmsbuild, @clacaputo, @slava77 can you please check and sign again.

[perrotta]

@cmsbuild please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6c7286/22902/summary.html
COMMIT: 4939ca6
CMSSW: CMSSW_12_3_X_2022-03-07-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37156/22902/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 55 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3984447
• DQMHistoTests: Total failures: 7
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 3984417
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -0.004 KiB( 48 files compared)
• DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
• Checked 204 log files, 45 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[jpata]

+reconstruction

• bugfix on top of Implementation of beam halo tagger algorithm for the endcap photons #36901, taking care of the uninitialized values of the haloMVATaggerVal

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

[perrotta]

+1

• This is expected to clean up the comparisons in the bot tests
• Default value of the haloMVATagger has been updated everywhere, as agreed here above