-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Photon MVA-based ID for Run3 from EGamma POG #39927
Photon MVA-based ID for Run3 from EGamma POG #39927
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39927/32847
|
A new Pull Request was created by @Prasant1993 (Prasant Kumar Rout) for master. It involves the following packages:
@cmsbuild, @mandrenguyen, @clacaputo, @swertz, @vlimant can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
type egamma |
please test with cms-data/RecoEgamma-PhotonIdentification#13 |
EGM POG maintains 4 types of IDs (electron cutbased+MVA; photon cutbased+MVA). All of them are being reoptimised for Run3. Tagging EGM nanoAOD contact @rgoldouz |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aaed28/28645/summary.html Comparison SummarySummary:
|
enable nano |
Do we want to disable the Run3 IDs (also in #39839) for Run2 samples? |
Run3 IDs are not specifically needed in Run2 samples. But we are anyway not going to reNano any of the Run2 data or MC, right? So maybe we can leave it out for simplicity for now? We can come back to cleanups later on..? |
The idea is to keep Run2 and Run3 samples as much as possible in sync for combined analyses, there will for sure be a re-nano of Run2 at some point (if not in 126X then in 130X), so we should keep things clean in master. This is also valid for MINI, btw (not running the Run3 IDs for Run2 MINI). |
yes we are just trying to make sure that Run3 IDs get integrated now, as we do not want to miss the deadline of last open pre-release of 12_6_0. The cleanups can be done afterwards too, if that's not a problem. |
+xpog |
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) |
@Prasant1993 @swagata87 are the many differences in the Run3 workflows (tracking, etc.) explained somehow? I doubt they come from this PR, not even from the technical PR tested together with this one, but it would be nice to pinpoint a possible origin of them before merging |
@perrotta can you please point out one of the relmon directory that makes you worried ? |
@vlimant I see you were able to find them yourself, and you also reached my conclusion that it cannot be originated by this PR. |
please test with cms-data/RecoEgamma-PhotonIdentification#13 |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aaed28/28875/summary.html 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: Comparison SummarySummary:
NANO Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
Nano size comparison Summary:
|
+1 Expected new features into nano. |
Errors in IBs:
|
cms-data/RecoEgamma-PhotonIdentification#13 was needed but it did not go in the IB. I have merged it and started a new IB an hour ago |
Great, thanks! |
PR description:
This PR is to add Photon MVA-based ID for Run3 from EGamma POG:
PR validation:
runTheMatrix tests have been successfully run for the following workflows :
Validation has also been performed to cross-check the Run3 MVA ID efficiencies from the MINIAOD datasets and compared with Run2 ID decisions using G+Jets, Relval H125, Relval ZEE and Relval QCD MC samples as shown in the below plots:
The photon MVA training weight files for Run3 have been already added here : https://github.com/cms-data/RecoEgamma-PhotonIdentification/tree/master/MVA
A PR is submitted and not merged yet : cms-data/RecoEgamma-PhotonIdentification#13
This current PR for photon MVA ID will take the input weight files from the above PR to work with.
Backport:
This PR does not need any backportation.
We would like to keep both RunIIFall17V2 photon MVA ID decision and this new Run3Winter22V1 photon MVA ID decision in the upcoming nano because several early analysis are using Run2 ID for Run3, and it is working quite well, so they might or might not want to switch. The necessary changes for adding the Run3 MVA photon ID in NanoAOD has already been made in this PR.