[9_4_X] Egamma Fall17 V2 IDs

[guitargeek]

Hi all,

this PR aims to backport the following Fall17 V2 IDs:

• Electron cut based (from Implementation of Fall17 cutbased Electron ID V2 #23477)
• Electron MVA (from Electron MVA V2 and General Restructuring of Egamma IDs - Once again #23700)
• Photon cut based and MVA (from Egamma Fall17 V2 Photon IDs #25293)

This PR depends on cms-sw/cmsdist#4546 for updated weight files.

This backport is done in a "minimal" fashion, meaning the goal was just to get the same ID functionality as in master and not backporting the full list of technical improvements between 9_4_X and master.

For the cut based IDs, backporting with cherry-picks was trivial, while the MVAs were more involved with renamed weight files for all past MVAs. For this reason, all value maps produced by the Egamma MVA machinery were validated. As a reference, the most recent 10_4_X integration build was used for the Fall17 V2 MVAs and 9_4_12 for all other MVAs. All Egamma MVAs which can be possible configured in 9_4_X were validated, not only the ones activated in the MVAValueMapProducers.

Integrating the V2 electron MVA was not as trivial as expected as variables were ordered and defined slightly different compared to V1. For now, this was solved with branchings in the ElectronMVAEstimatorFall17 class.

Validation plots for all photon ID value maps:
https://rembserj.web.cern.ch/rembserj/plots/CMSSW_PRs/25408/

• target: CMSSW_9_4_12 plus this PR and the external PR
• reference: CMSSW_10_4_0_pre2 and CMSSW_9_4_12

Validation workflow:

• Dump all relevant ID value maps to ROOT file with this configuration [1a,1b], running on DY+Jets
• Compare ROOT file from reference and target with this script [2]

[1a] https://github.com/guitargeek/cmssw/blob/EgammaID_validation/RecoEgamma/EgammaTools/test/testElectronIDs_cfg.py
[1b] https://github.com/guitargeek/cmssw/blob/EgammaID_validation/RecoEgamma/EgammaTools/test/testPhotonIDs_cfg.py
[2] https://github.com/guitargeek/cmssw/blob/EgammaID_validation/RecoEgamma/EgammaTools/test/compare_columns.py

[cmsbuild]

A new Pull Request was created by @guitargeek (Jonas Rembser) for CMSSW_9_4_X.

It involves the following packages:

PhysicsTools/PatAlgos
RecoEgamma/EgammaTools
RecoEgamma/ElectronIdentification
RecoEgamma/PhotonIdentification

@perrotta, @monttj, @cmsbuild, @fgolf, @slava77, @peruzzim can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @jainshilpi, @rappoccio, @HeinerTholen, @varuns23, @seemasharmafnal, @mmarionncern, @imarches, @ahinzmann, @smoortga, @acaudron, @lgray, @jdolen, @drkovalskyi, @ferencek, @Sam-Harper, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @clelange, @JyothsnaKomaragiri, @mverzett, @gpetruc, @mariadalfonso, @pvmulder this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[perrotta]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31964/console Started: 2018/12/04 09:48

[guitargeek]

Hi @perrotta, you have to test with the externals. Sorry, I should have referenced the needed externals at the en of the original post, as I have always done.

[perrotta]

Thank you @guitargeek . I should have remembered it, in fact, instead of automatically start tests...

[perrotta]

please abort test

[cmsbuild]

Jenkins tests are aborted.

[perrotta]

please test with cms-sw/cmsdist#4545, cms-sw/cmsdist#4546

[perrotta]

please test with cms-sw/cmsdist#4546

[cmsbuild]

The tests are being triggered in jenkins.
Using externals from cms-sw/cmsdist#4546
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32785/console
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32790/console

[cmsbuild]

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-25408/32785/git-merge-result

[perrotta]

@mrodozov @smuzaffar : sorry to bother you again, but this is re-happening: #25408 (comment)

Is there anything we can do for it (e.g. keep trying ), or it is something that only you can solve?

[smuzaffar]

@perrotta , may be our mirror in /cvmfs (which is used as reference) is causing this error. For now I have added some debug statements to see where is error happens.
I have restarted the tests.

[perrotta]

Thank you Shahzad! Malik Shahzad Muzaffar <notifications@github.com> ha scritto:

…

@perrotta , may be our mirror in /cvmfs (which is used as reference) is causing this error. For now I have added some debug statements to see where is error happens. I have restarted the tests. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #25408 (comment)

[cmsbuild]

+1
Tested at: 2ae0de6
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25408/32790/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25408/32790/summary.html

The workflows 1001.0, 1000.0, 140.53, 136.8311, 136.7611, 136.731, 4.22 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 27
• DQMHistoTests: Total histograms compared: 2721493
• DQMHistoTests: Total failures: 110
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2721221
• DQMHistoTests: Total skipped: 162
• DQMHistoTests: Total Missing objects: 0

[perrotta]

+1

• Since the reco signature in [9_4_X] Egamma Fall17 V2 IDs #25408 (comment) a conditional inclusion of the Fall17 V2 Egamma IDs only for the run2_miniAOD_devel modifier was implemented. No effect is visible now on jenkins tests, as it should. I repeat below the same comments written for the previous signature.
• This PR is not just a backport of (a selected part) of three PRs now merged in the master: it further adds several lines of brand new code needed to adapt these chunks of backports to the rest of 94X. This is potentially a dangerous exercise to be applied on top of a closed and supposedly stable release.
• However, after a detailed inspection of the code submitted I believe that the porting was done correctly, and that the parts of code that were not backported are all intended and related to the new MVA infrastructure that was decided to not backport completely (thank you @guitargeek for having checked it twice and confirmed)
• As a further confirmation, the validation plots provided show a nice agreement for all photon and electron ids: the ones that were already in 94X are not changed, and the outputs of the newly added ones correspond to the output of the same ids merged in the master (now only in correspondence of the
  run2_miniAOD_devel modifier)
• When the run2_miniAOD_devel modifier is enabled the jenkins tests do not show other differences besides the additional ids in all miniAOD outputs
• It also needs [9.4.X] Backport of Egamma weight files updates cmsdist#4546 to be merged

[fabiocos]

+operations

the modification to Eras and StandardSequences is coherent with the purpose of the PR

[fabiocos]

@smuzaffar we need to merge cms-sw/cmsdist#4546 before merging this PR. Some older files are replaced by new ones, so I would merge this later this evening to minimize the risk of problems (although the development activity on the 9_4_X branch is reduced)

[smuzaffar]

@fabiocos , cms-sw/cmsdist#4546 is merged now. Note it will be not part of 11h IB though, so merging cmssw PR got 23h IB should work.

[fabiocos]

+1

@smuzaffar thanks

[fabiocos]

merge