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

Adding HtoZZ UL ID configs #33517

Merged
merged 1 commit into from
May 5, 2021
Merged

Adding HtoZZ UL ID configs #33517

merged 1 commit into from
May 5, 2021

Conversation

jainshilpi
Copy link
Contributor

@jainshilpi jainshilpi commented Apr 23, 2021

This new PR (closed #33514) concerns the addition of configs needed for running VID for recently tuned HtoZZ ID for UL. The added configs can then be used by analysers with EgammaPostRecoTools to embed in the PAT objects. Files modified/added:

(1) RecoEgamma/ElectronIdentification/python/ElectronMVAValueMapProducer_cfi.py
(2) RecoEgamma/ElectronIdentification/python/Identification/mvaElectronID_Summer16UL_ID_ISO_cff.py
(3) RecoEgamma/ElectronIdentification/python/Identification/mvaElectronID_Summer17UL_ID_ISO_cff.py
(4) RecoEgamma/ElectronIdentification/python/Identification/mvaElectronID_Summer18UL_ID_ISO_cff.py

The related presentation can be seen here:
HtoZZ electron ID: https://indico.cern.ch/event/895972/contributions/4297090/attachments/2219102/3757479/ElectronID_Egamma31032021.pdf

PR for MVA files for electron (HtoZZ run II UL) IDs are here:
cms-data/RecoEgamma-ElectronIdentification#18

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33517/22273

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

RecoEgamma/ElectronIdentification

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

cms-bot commands are listed here

@perrotta
Copy link
Contributor

test parameters:

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a0a6fa/14569/summary.html
COMMIT: f8e5cad
CMSSW: CMSSW_12_0_X_2021-04-25-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33517/14569/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2877605
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2877576
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor

@jainshilpi is there a test script or anything else we can use to exercize the new configs, which are not run in the normal workflows? Just to be sure that they were correctly integrated and can run without errors: for their physics validation the attached presentation is quite appropriate, instead, and we can rely on them.

@jainshilpi
Copy link
Contributor Author

@jainshilpi is there a test script or anything else we can use to exercize the new configs, which are not run in the normal workflows? Just to be sure that they were correctly integrated and can run without errors: for their physics validation the attached presentation is quite appropriate, instead, and we can rely on them.

Hi Andrea, I have a code to test (based on my ntuplizer which runs EgammaPostRecoTools). In addition, I can also provide the configs which are to be changed in order to enable their decisions to be embedded in pat::Object. How would you prefer to test? I can put my code in the github with instructions as well.

Thanks

@perrotta
Copy link
Contributor

perrotta commented Apr 29, 2021 via email

@perrotta
Copy link
Contributor

perrotta commented May 3, 2021

ping @jainshilpi

Thank you Shilpi. The code changes are simple enough, and are only at the configuration level plus additional data files: I dont't think they deserve any special test unit for them. Since these new configs are not exercized in the standard workflows, the only thing I would like to check before signing is that everything that you already succesfully run with your private setup was correctly propagated to CMSSW, the data files are not corrupted, etc.. For that, a simple recipe about how to run them in current CMSSW + this PR would allow to verify it: as soon as it runs to completion I'll be happy with it. jainshilpi @.***> ha scritto:

@jainshilpi is there a test script or anything else we can use to > exercize the new configs, which are not run in the normal > workflows? Just to be sure that they were correctly integrated and > can run without errors: for their physics validation the attached > presentation is quite appropriate, instead, and we can rely on them. Hi Andrea, I have a code to test (based on my ntuplizer which runs EgammaPostRecoTools). In addition, I can also provide the configs which are to be changed in order to enable their decisions to be embedded in pat::Object. How would you prefer to test? I can put my code in the github with instructions as well. Thanks

@jainshilpi
Copy link
Contributor Author

Hi Andrea,

Apologies for hte delay. Please find this small setup here:
https://github.com/jainshilpi/TestAnalyser

Currently the input is /tmp/shilpi/separatePRs/CMSSW_12_0_X_2021-05-02-2300/src/1325.517_TTbar_13_reminiaod2017UL_INPUT+TTbar_13_reminiaod2017UL_INPUT+REMINIAOD_mc2017UL+HARVESTUP17_REMINIAOD_mc2017UL in lxplus752 in case you need.

Also, the implementation and call to the new ID can be seen here.

Please let me know in case you need anything else.

Thanks
Shilpi

@perrotta
Copy link
Contributor

perrotta commented May 5, 2021

Hi Shilpi.

I had to hack RecoEgamma/EgammaTools/python/EgammaPostRecoTools.py to have the test run (let me suggest you to update the allowedVersions in it for 12_X, in a separate PR).

Still, your /tmp/shilpi area in lxplus752 is protected, and I cannot access it.
I tried to rerun the 1325.517 workflow from scratch with runTheMatrix, but I get DAS errors for the inputs.

Could you please either move the input file into a non protected area, or alternatively run the test yourself in CMSSW_12_0_X_2021-05-04-2300+#33517 (the data external are already merged since that IB) and confirm here that everything runs to completion without errors?

Thank you.

@jainshilpi
Copy link
Contributor Author

Hi Andrea,

sorry, I had updated EgammaPostRecoTools separately but missed to commit to the egamma repository. I can do the test separately and post here.

thanks
Shilpi

@jainshilpi
Copy link
Contributor Author

@perrotta , I confirm that with CMSSW_12_0_X_2021-05-04-2300 + the explicit checkout of #33517 and using EgammaPostRecoTools, everything runs fine and we can see the ID decisions implemented.

@perrotta
Copy link
Contributor

perrotta commented May 5, 2021

@perrotta , I confirm that with CMSSW_12_0_X_2021-05-04-2300 + the explicit checkout of #33517 and using EgammaPostRecoTools, everything runs fine and we can see the ID decisions implemented.

Great, thank you Shilpi!
For the backport pull request #33620, please, prepare the 10_6 backport of the cms-data external from cms-data/RecoEgamma-ElectronIdentification#18: I think it can go quickly after this one.

@perrotta
Copy link
Contributor

perrotta commented May 5, 2021

+1

  • New Electron ID tuned to HtoZZ for the three years of UL are made available for analyses and possible nanoAOD productions
  • None of them enters any current workflow, so no changes are visible in the test outputs
  • The needed cms-data externals are already merged for 12_X

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2021

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

@qliphy
Copy link
Contributor

qliphy commented May 5, 2021

+1

@cmsbuild cmsbuild merged commit bc6cccd into cms-sw:master May 5, 2021
cmsbuild added a commit that referenced this pull request May 12, 2021
Backport of #33517: Adding HtoZZ UL ID configs
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.

4 participants