-
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
Final BDT models based on 10.2 MC samples #25936
Final BDT models based on 10.2 MC samples #25936
Conversation
A new Pull Request was created by @bainbrid for CMSSW_10_2_X. It involves the following packages: PhysicsTools/PatAlgos @cmsbuild, @perrotta, @santocch, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Incremental differences on top of #25887 seem rather limited (codewise): some better evaluation of this PR can be provided when #25887 is merged and this one will be rebased. A few preliminary observations:
In the meanwhile, please prepare also the corresponding pull request for the master release with just the incremental changes |
@perrotta @Dr15Jones @slava77 |
Thank you @bainbrid |
The code was making a temporary copy of a PFBrem, then holding onto the address of internal data after the temporary was deleted. This change avoids the unnecessary copy. The problem was found by the address sanitizer.
1c2aef5
to
71cfe25
Compare
@perrotta Rebased on CMSSW_10_2_X_2019-02-21-1100 |
Thank you @bainbrid |
please test |
The tests are being triggered in jenkins. |
Comparison is ready Comparison Summary:
|
Description: cms-data/RecoEgamma-ElectronIdentification#13 Resolves: #4702 Needed for: cms-sw/cmssw#25936
please test with cms-sw/cmsdist#4724 |
The tests are being triggered in jenkins. |
Thank you @mrodozov ! |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
-1 |
Ok, i will close. |
This PR provides incremental changes on top of #25887.
This PR includes fixes to the SuperCluster class included in #25960 and #25974.
This PR requires the following external: cms-data/RecoEgamma-ElectronIdentification#13
This PR provide three changes:
1) It includes fixes to the SuperCluster class.
The fixes are back ported from #25960 and #25974:
The timing and footprint numbers below have been updated to reflect these fixes.
2) It provides the back port of #25915.
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. Quantitive numbers on timing and footprint can be found in the conversation of #25915.
3) It uses new BDT models and working points.
The models are found in cms-data/RecoEgamma-ElectronIdentification#13.
The working points have been tuned to rate balance w.r.t. the old WPs. Updated numbers are provided below, based on 11024 TTbar (PU50).
Timing:
RECO footprint:
MINIAOD footprint: