-
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
Low pT electrons: modify value maps storing Seed BDT outputs #25915
Low pT electrons: modify value maps storing Seed BDT outputs #25915
Conversation
@nancymarinelli @gkaratha @mverzett are interested to follow this thread. |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25915/8396
|
A new Pull Request was created by @bainbrid for master. It involves the following packages: RecoEgamma/EgammaElectronProducers @cmsbuild, @perrotta, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Verified with a 11024.0 workflow, run with the bParking era enabled, that the increase in output size is quite limited, while no sizeable effect on the CPU timing is visible:
|
+1
|
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
This PR is a small modification to the code base in the now-merged PR #25753.
The back port of this PR to 10.2.X is #25887.
The change is very small and concerns the only the
LowPtGsfElectronSeedValueMapsProducer
class, which produces ValueMaps that store the discriminator values from the BDT models used in theLowPtGsfElectronSeedProducer
.The ValueMaps originally stored the BDT outputs per GsfElectrons, but we need them per GsfTrack. There is a near (but not exact) 1-to-1 correspondence between GsfElectrons and GsfTracks, so the ValueMaps will increase moderately in size (at the few tens of % level, not orders of magnitude!) and they will use a different key.
No externals are needed nor changed for this PR.
We will need to back port this PR to the 10.2.X release cycle.
To give a sense of scale, below are the timing and footprint metrics for the original code. We do not expect these to change dramatically.
Timing:
RECO footprint: