Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Electron MVA-based ID for Run3 from EGamma POG #40230

Conversation

Prasant1993
Copy link
Contributor

PR description:

This PR is to add Electron MVA-based ID for Run3 from EGamma POG:

PR validation:

runTheMatrix tests have been successfully completed

  • runTheMatrix.py -l 12434.0

Validation has also been performed to cross-check the Run3 MVA ID efficiencies from the MINIAOD datasets and compared with Run2 ID decisions using 124X Relval ZEE MC samples as shown here: https://egamma-val.web.cern.ch/Run2_vs_Run3_electronIDMVA_test_plots/Tests_in_124X_relvalZEE_sample/

The electron MVA training weight files for Run3 have been already added here : https://github.com/Prasant1993/RecoEgamma-ElectronIdentification/tree/master/MVAWeightFiles
A PR is submitted and not merged yet : cms-data/RecoEgamma-ElectronIdentification#27
This current PR for electron MVA ID will take the input weight files from the above PR to work with.

Backport:

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:

This PR will require backport to 12_6_X for nanoV11 production.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2022

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2022

A new Pull Request was created by @Prasant1993 (Prasant Kumar Rout) for master.

It involves the following packages:

  • Configuration/Eras (operations)
  • Configuration/HLT (hlt)
  • PhysicsTools/NanoAOD (xpog)
  • PhysicsTools/PatAlgos (xpog, reconstruction)
  • RecoEgamma/EgammaTools (reconstruction)
  • RecoEgamma/ElectronIdentification (reconstruction)

@perrotta, @Martin-Grunewald, @rappoccio, @swertz, @vlimant, @clacaputo, @cmsbuild, @missirol, @mandrenguyen, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @jainshilpi, @hatakeyamak, @emilbols, @Martin-Grunewald, @Sam-Harper, @mbluj, @ahinzmann, @demuller, @varuns23, @seemasharmafnal, @mmarionncern, @jdolen, @makortel, @lgray, @missirol, @azotz, @a-kapoor, @silviodonato, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @fabiocos, @AlexDeMoor, @AnnikaStein, @valsdav, @JyothsnaKomaragiri, @sobhatta, @afiqaize, @gpetruc, @wrtabb, @mariadalfonso, @sameasy, @andrzejnovak, @ram1123 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

@swertz
Copy link
Contributor

swertz commented Dec 3, 2022

enable nano

@swertz
Copy link
Contributor

swertz commented Dec 3, 2022

Thanks a lot @Prasant1993 .

Is it expected that commit 564bd87 is included? Or is it an artifact of the rebasing?

@Prasant1993
Copy link
Contributor Author

Hii @swertz, this commit 564bd87 from HLT is not expected. It got picked up at the time during rebasing from your PR.

@swertz
Copy link
Contributor

swertz commented Dec 3, 2022

Unfortunately there are conflicts now, could you please rebase on master?

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40230/33277

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2022

Pull request #40230 was updated. @Martin-Grunewald, @swertz, @vlimant, @clacaputo, @cmsbuild, @missirol, @mandrenguyen can you please check and sign again.

@@ -10,7 +10,7 @@
'relval2016' : 'Fake2',
'relval2017' : 'Fake2',
'relval2018' : 'Fake2',
'relval2022' : 'GRun',
'relval2022' : '2022v15',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this change (then you do not need HLT signature). It was introduced for 12_6 but not (yet) for 13_0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hii @Martin-Grunewald,
Okay I understand.
If I understand correctly, I think I need to replace this line 'relval2022' : '2022v15' with 'relval2022' : 'GRun' and commit the changes again to my branch EGMRun3MVAID_from-CMSSW_13_0_X_2022-11-29-2300. Is this correct ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, you restore the previous line incl. blank spaces so that git no longer shows a diff.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I need to replace this line 'relval2022' : '2022v15' with 'relval2022' : 'GRun' and commit the changes again to my branch EGMRun3MVAID_from-CMSSW_13_0_X_2022-11-29-2300. Is this correct ?

@Prasant1993 , please , just remove the commit that shouldn't be in your branch in the first place, instead of adding another one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hii @missirol, Sorry for the problem caused. It was not intentional to bring this commit in this present PR. It got picked up during the rebasing on top of another PR (#40209) to pick up those changes made. I have removed this commit now. I hope it will be fine now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Prasant1993 , looks good now.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 5, 2022

Pull request #40230 was updated. @cmsbuild, @mandrenguyen, @clacaputo, @swertz, @vlimant can you please check and sign again.

@swertz
Copy link
Contributor

swertz commented Dec 5, 2022

please test with cms-data/RecoEgamma-ElectronIdentification#27

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 5, 2022

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-426c3b/29461/summary.html
COMMIT: cbe51f7
CMSSW: CMSSW_13_0_X_2022-12-04-2300/el8_amd64_gcc11
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40230/29461/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-426c3b/29461/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-426c3b/29461/git-merge-result

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 889 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3421287
  • DQMHistoTests: Total failures: 181
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3421084
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 13.781999999999998 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 1.658 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 13234.0,... ): 1.088 KiB Physics/NanoAODDQM
  • Checked 206 log files, 158 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 32 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 15298
  • DQMHistoTests: Total failures: 66
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 15232
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 10.411 KiB( 14 files compared)
  • DQMHistoSizes: changed ( 2500.311,... ): -0.346 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 2500.331,... ): -0.225 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 2500.401,... ): 1.658 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 2500.511,... ): 1.088 KiB Physics/NanoAODDQM
  • Checked 31 log files, 14 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.31 2.190 2.190 0.000 ( +0.0% ) 9.21 9.42 -2.2% 1.514 1.570
2500.311 2.312 2.312 0.000 ( +0.0% ) 8.90 9.09 -2.1% 1.880 1.934
2500.312 2.264 2.264 0.000 ( +0.0% ) 9.08 9.27 -2.0% 1.871 1.925
2500.33 1.083 1.083 0.000 ( +0.0% ) 21.39 21.00 +1.9% 1.689 1.690
2500.331 1.379 1.379 0.000 ( +0.0% ) 15.76 15.89 -0.9% 1.833 1.835
2500.332 1.312 1.312 0.000 ( +0.0% ) 17.64 17.80 -0.9% 1.792 1.793
2500.4 2.097 2.097 0.000 ( +0.0% ) 9.87 10.10 -2.3% 1.410 1.471
2500.401 2.145 2.135 0.010 ( +0.5% ) 10.12 10.23 -1.1% 1.210 1.265
2500.5 1.668 1.668 0.000 ( +0.0% ) 16.13 16.38 -1.5% 1.138 1.193
2500.501 1.712 1.705 0.007 ( +0.4% ) 16.13 16.38 -1.5% 1.135 1.188
2500.51 1.071 1.071 0.000 ( +0.0% ) 28.48 28.09 +1.4% 1.417 1.483
2500.511 1.122 1.113 0.009 ( +0.8% ) 27.90 28.29 -1.4% 1.437 1.498
2500.6 2.006 2.006 0.000 ( +0.0% ) 12.99 12.93 +0.4% 1.185 1.240
2500.601 2.055 2.045 0.010 ( +0.5% ) 12.91 12.94 -0.3% 1.201 1.250

@swertz
Copy link
Contributor

swertz commented Dec 5, 2022

please test with cms-data/RecoEgamma-ElectronIdentification#27

Retrying because of timeout

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 5, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-426c3b/29465/summary.html
COMMIT: cbe51f7
CMSSW: CMSSW_13_0_X_2022-12-05-1100/el8_amd64_gcc11
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40230/29465/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
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 888 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3421287
  • DQMHistoTests: Total failures: 181
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3421084
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 13.781999999999998 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 1.658 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 13234.0,... ): 1.088 KiB Physics/NanoAODDQM
  • Checked 206 log files, 158 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 32 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 15298
  • DQMHistoTests: Total failures: 66
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 15232
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 10.411 KiB( 14 files compared)
  • DQMHistoSizes: changed ( 2500.311,... ): -0.346 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 2500.331,... ): -0.225 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 2500.401,... ): 1.658 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 2500.511,... ): 1.088 KiB Physics/NanoAODDQM
  • Checked 31 log files, 14 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.31 2.190 2.190 0.000 ( +0.0% ) 9.03 9.53 -5.2% 1.518 1.509
2500.311 2.312 2.312 0.000 ( +0.0% ) 8.79 9.10 -3.4% 1.883 1.874
2500.312 2.264 2.264 0.000 ( +0.0% ) 8.91 9.27 -3.8% 1.874 1.866
2500.33 1.083 1.083 0.000 ( +0.0% ) 19.89 21.89 -9.1% 1.681 1.684
2500.331 1.379 1.379 0.000 ( +0.0% ) 14.91 15.98 -6.7% 1.832 1.830
2500.332 1.312 1.312 0.000 ( +0.0% ) 16.64 17.90 -7.1% 1.788 1.787
2500.4 2.097 2.097 0.000 ( +0.0% ) 9.66 10.05 -3.9% 1.418 1.405
2500.401 2.145 2.135 0.010 ( +0.5% ) 9.86 10.25 -3.8% 1.215 1.211
2500.5 1.668 1.668 0.000 ( +0.0% ) 15.07 16.40 -8.2% 1.133 1.120
2500.501 1.712 1.705 0.007 ( +0.4% ) 15.08 16.44 -8.3% 1.141 1.125
2500.51 1.071 1.071 0.000 ( +0.0% ) 26.48 28.90 -8.4% 1.428 1.414
2500.511 1.122 1.113 0.009 ( +0.8% ) 26.08 28.48 -8.4% 1.450 1.435
2500.6 2.006 2.006 0.000 ( +0.0% ) 12.16 13.07 -7.0% 1.194 1.181
2500.601 2.055 2.045 0.010 ( +0.5% ) 12.06 13.09 -7.9% 1.215 1.185

@swertz
Copy link
Contributor

swertz commented Dec 6, 2022

+1

  • No changes in Run2 or Run3 V10 nano
  • electron MVA ID changes for Run3
  • Fall17V2 MVA ID added as backup for Run3

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 8, 2022

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
Copy link
Contributor

@cmsbuild cmsbuild merged commit e64b97e into cms-sw:master Dec 8, 2022
cmsbuild added a commit that referenced this pull request Dec 9, 2022
…-CMSSW_12_6_X_2022-12-04-0000

[Backport of PR #40230] Electron MVA-based ID for Run3 from EGamma POG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants