-
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
LowPtElectrons: convert Seed BDTs from XML to ROOT file format #34908
LowPtElectrons: convert Seed BDTs from XML to ROOT file format #34908
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34908/24716
|
A new Pull Request was created by @bainbrid for master. It involves the following packages:
@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
test parameters: |
please test |
Hi @perrotta, does this require "enable profiling"? (#34633 (comment)) Or do you test first? |
please abort |
You are right, thank you |
enable profiling |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dcc05a/17827/summary.html Comparison SummarySummary:
|
+reconstruction
this PR should be merged together with a cmsdist corresponding to cms-data/RecoEgamma-ElectronIdentification#22 |
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) |
PR description:
This PR converts the formats (from XML to ROOT) of two files containing the weights of two BDT models used by the low pT electron seeding step in the reconstruction chain.
The effect is to reduce the file size by a factor two for both files, and to reduce the memory consumption and CPU time when parsing the weights files.
This was done in response to the issues raised here: #34707
A similar change was made previously, but on the weights file for the BDT model used by the low pT electron ID module (as opposed to the seeding step, as above), which solved the issues raised here: #28780
This PR depends on cms-data/RecoEgamma-ElectronIdentification#22
PR validation:
Local tests were made with the wf 1304.182.
if this PR is a backport please specify the original PR and why you need to backport that PR:
No back port required.