LowPtElectrons: switch to Autumn18 models

[bainbrid]

This PR updates the models used by the lowPtGsfElectrons chain. The old models were "Fall17"; the new are "Autumn18". The Autumn18 models are available as externals in this (merged) PR:
cms-data/RecoEgamma-ElectronIdentification#13.

This PR also switches to a "Very Loose" working point for the low pT ElectronSeed module.

1. Commit 23e8326: Trivially moves some default configuration values (concerning the models) from the fillDescriptions() method of LowPtGsfElectronIDProducer to an explicit declaration in RecoEgamma/EgammaElectronProducers/python/lowPtGsfElectronID_cff.py. This commit was (accidentally) added to 10_2_X before master in the ad31169 commit as part of Final BDT models based on 10.2 MC samples #25936.

2. Commit 48fead2: Points to the new Autumn18 files and updates the corresponding L,M, and T working points

3. Commit 9f8467b: Adds the thresholds for a Very Loose WP and makes VL the new default for the bParking era. The back port of this commit can be found in LowPtElectrons: bug fixes + minor updates + Autumn18 models + new VL WP #26013.

The timing and footprint increases w.r.t. nominal when using the Autumn18 models and the default Tight working point used by all standard sequences are provided below.

Timing, standard workflows, Tight WP:

The same excluding the first 1 events
  delta/mean delta/orJob     original                   new       module name
  ---------- ------------     --------                  ----       ------------
       added      +3.46%         0.00 ms/ev ->       540.15 ms/ev lowPtGsfElectronSeeds
       added      +0.52%         0.00 ms/ev ->        81.78 ms/ev lowPtGsfEleGsfTracks
       added      +0.09%         0.00 ms/ev ->        13.45 ms/ev lowPtGsfEleCkfTrackCandidates
       added      +0.06%         0.00 ms/ev ->        10.07 ms/ev lowPtGsfElePfTracks
       added      +0.08%         0.00 ms/ev ->        12.86 ms/ev lowPtGsfElePfGsfTracks
       added      +0.00%         0.00 ms/ev ->         0.03 ms/ev lowPtGsfElectronSeedValueMaps
       added      +0.01%         0.00 ms/ev ->         1.15 ms/ev lowPtGsfElectronID
       added      +0.03%         0.00 ms/ev ->         4.04 ms/ev lowPtGsfElectronCores
       added      +0.21%         0.00 ms/ev ->        32.58 ms/ev lowPtGsfElectrons
       added      +0.04%         0.00 ms/ev ->         6.76 ms/ev lowPtGsfElectronSuperClusters
                  +4.50%                             702.87 ms/ev
  ---------- ------------     --------                  ----       ------------
Job total:  15.6045 s/ev ==> 16.2956 s/ev

RECO footprint, standard workflows, Tight WP:

-----------------------------------------------------------------
   or, B         new, B      delta, B   delta, %   deltaJ, %    branch
-----------------------------------------------------------------
      0.0 ->       143.7        144     NEWO   0.00     recoGsfElectronCores_lowPtGsfElectronCores__RECO
      0.0 ->       120.6        121     NEWO   0.00     floatedmValueMap_lowPtGsfElectronID__RECO
      0.0 ->      5864.6       5865     NEWO   0.18     recoCaloClusters_lowPtGsfElectronSuperClusters__RECO
      0.0 ->       595.2        595     NEWO   0.02     recoSuperClusters_lowPtGsfElectronSuperClusters__RECO
      0.0 ->      1531.2       1531     NEWO   0.05     recoGsfTracks_lowPtGsfEleGsfTracks__RECO
      0.0 ->       127.5        127     NEWO   0.00     floatedmValueMap_lowPtGsfElectronSeedValueMaps_ptbiased_RECO
      0.0 ->       127.0        127     NEWO   0.00     floatedmValueMap_lowPtGsfElectronSeedValueMaps_unbiased_RECO
      0.0 ->      3456.4       3456     NEWO   0.10     recoGsfElectrons_lowPtGsfElectrons__RECO
-------------------------------------------------------------
  3348098 ->     3360658      12212            0.35     ALL BRANCHES

The lowPtGsfElectrons are not added to miniAOD for all standard workflows.

@slava77 @perrotta @mverzett @nancymarinelli @gkaratha

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26012/8533

• This PR adds an extra 16KB to repository

[cmsbuild]

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.
@jainshilpi, @Sam-Harper, @varuns23, @rovere, @lgray this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26012/8534

• This PR adds an extra 16KB to repository

[cmsbuild]

Pull request #26012 was updated. @cmsbuild, @perrotta, @slava77 can you please check and sign again.

[gudrutis]

Please test with cms-sw/cmsdist#4728

[cmsbuild]

The tests are being triggered in jenkins.
Using externals from cms-sw/cmsdist#4728
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/33269/console

[perrotta]

Thank you @gudrutis !

[cmsbuild]

+1
Tested at: 9f8467b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-26012/33269/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-26012/33269/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 32
• DQMHistoTests: Total histograms compared: 3098286
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3098088
• DQMHistoTests: Total skipped: 197
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
• Checked 133 log files, 14 edm output root files, 32 DQM output files

[perrotta]

@bainbrid : performance numbers seem to change quite often for those low pt electrons. The comparisons you are listing in the PR descriptions clearly do not refer to the baseline (which already includes all those lotPtEle modules). They also differ somehow to what was evaluated previously for the ancestors of this PR, see for example #25753 (comment), for which the TTbar+PU workflow 11024.0 was used.

In order to reduce the confusion, could you please specify how did you obtain the performance numbers listed in the PR description? In particular:

• Which workflow were you using for it? How many events?
• Which baseline are you comparing to?
• Do the timing includes also Validation/DQM or only the reco/miniAOD steps?

The RECO event size apparently differ significantly only in the recoCaloClusters_lowPtGsfElectronSuperClusters, and I expect that this can be correlated to the bug fixes for it that were integrated in the meanwhile with #25960 and #25974.

Timing for the tight working point also seems rather inflated here with respect to the last evaluations that we made for #25679 (but you still have to define how was it computed here). The new model should only affect ID, not the seeds: when you specify better how all those numbers where derived we can draw some conclusion on it.

[bainbrid]

@perrotta Apologies for the confusion. Hopefully the comments below will clarify.

On 26 Feb 2019, at 11:53, perrotta ***@***.***> wrote: @bainbrid <https://github.com/bainbrid> : performance numbers seem to change quite often for those low pt electrons. The comparisons you are listing in the PR descriptions clearly do not refer to the baseline (which already includes all those lotPtEle modules). They also differ somehow to what was evaluated previously for the ancestors of this PR, see for example #25753 (comment) <#25753 (comment)>, for which the TTbar+PU workflow 11024.0 was used.

I have not given the incremental difference w.r.t. the latest IB, but instead w.r.t. the nominal RECO chain. i.e. the latest version of the complete low pT electron chain adds 4.5% for standard workflows, which is consistent with numbers given previously, as I try to summarise below. [Here](#25679 (comment)) you report a CPU increase for "up to GsfTracks" of 4.3%. [Here](#25753 (comment)) I report numbers from myself, broken down according to "Up to GsfTracks" (4.41%) and the full chain (4.79%). (To give an example of the source of the largest increase, the lowPtGsfElectronSeeds module alone adds 3.31%.) [Here](#25753 (comment)) you report numbers for the full chain _in addition_ to "Up to GsfTracks": an additional 52 ms/ev. [Here](#26012 (comment)) is the latest summary from me, which shows an increase in CPU of 4.50% w.r.t. the nominal RECO, without low pT electrons. (The lowPtGsfElectronSeeds alone adds 3.46%.) I would suggest all these numbers are consistent, at the level of 5-10%, within the context of statistical uncertainties and bug fixes, details below.

In order to reduce the confusion, could you please specify how did you obtain the performance numbers listed in the PR description? In particular: Which workflow were you using for it? How many events?

I am using 11024, 100 events.

Which baseline are you comparing to?

The numbers I quote in this thread refer to a "baseline" that does not execute the low pT ele chain __at all__. i.e. the nominal RECO chain. So the increases fold in the _total_ addition to RECO from the low pT electrons (and do not factorise into "up to GsfTracks" or "Full chain" as we have done for previously, for the different PRs).

Do the timing includes also Validation/DQM or only the reco/miniAOD steps?

Only RECO/miniAOD. (I think I remember being told to do this - is it correct?)

The RECO event size apparently differ significantly only in the recoCaloClusters_lowPtGsfElectronSuperClusters, and I expect that this is could be correlated to the bug fixes for it that were integrated in the meanwhile with #25974 <#25974>. On the other hand, that bug fix should have moved in the direction of slimming the SuperClusters, not the opposite: do you have an explantion for it?

I'm not sure I can draw the same conclusion. We were accessing freed memory (so who knows what that was), so I cannot say for certain whether the SuperCluster should increase or decrease. What I can say is that the SuperClusters distributions now look a lot healthier ...

Timing for the tight working point also seems rather inflated here with respect to the last evaluations that we made for #25679 <#25679> (but you still have to define how was it computed here). The new model should only affect ID, not the seeds: when you specify better how all those numbers where derived we can draw some conclusion on it.

This PR updates __all__ models, both Seeds and ID, from the Fall17 versions to the Autumn18 versions. The corresponding working points have been updated, and were done so to rate balance w.r.t. what was observed for the Fall17 models. So the new models + Tight WPs gives a 4.5% increase in CPU w.r.t. the nominal RECO. And this should be compared with the number obtained for the Fall17 models, which is 4.8%, reported [here](#25753 (comment)).

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26012 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABEfkszH2hgL1OzTndy79Hsxgb2xnNvrks5vRSAmgaJpZM4bPyek>.

[perrotta]

Thank you @bainbrid : the key point is that you are normalizing timings on the RECO/miniAOD (good!) and the wf used is the same as in the previous evaluations (also good!).

As a comparison, could you please provides the numbers also for the VL working point? Or should we rely on the evaluations that you provide in #26013 for it, even if measured on a different base release?

[bainbrid]

On 26 Feb 2019, at 14:06, perrotta ***@***.***> wrote: Thank you @bainbrid <https://github.com/bainbrid> : the key point is that you are normalizing timings on the RECO/miniAOD (good!) and the wf used is the same as in the previous evaluations (also good!). As a comparison, could you please provides the numbers also for the VL working point? Or should we rely on the evaluations that you provide in #26013 <#26013> for it, even if measured on a different base release?

Yes, please rely on those listed in the conversion for #26013. My thinking was: - use master for the evaluation of the standard workflows - use 10_2_X for the evaluation of the bParking era. I hope this makes sense.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26012 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABEfkhq6nFKrFGNgjDIx4fsLDj3X3M7Lks5vRT-BgaJpZM4bPyek>.

[perrotta]

And what do you use for the baseline? 10_2 default in both cases, or 10_2_X baseline in one case and a "managed" master to remove even the tight wp from it for 10_6_X?

[bainbrid]

All numbers are w.r.t. the __nominal RECO__ *without* low pT electrons. i.e. "default 10_2_X". In both cases. (Similar to what I did for the Tight WP studies for master.)

…

On 26 Feb 2019, at 14:17, perrotta ***@***.***> wrote: And what do you use for the baseline? 10_2 default in both cases, or 10_2_X baseline in one case and a "managed" master to remove even the tight wp from it for 10_6_X? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26012 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABEfksYN8HodqJ9zm4rhFnSdIg88DZGmks5vRUH6gaJpZM4bPyek>.

[perrotta]

+1

• Models used by the lowPtGsfElectrons chain are updated as reported in the PR description
• It requires Upgrade RecoEgamma-ElectronIdentification to V01-01-03 on IB/CMSSW_10_6_X/gcc700 cmsdist#4728
• This being (hopefully) the last integration in the master for the low pt electron reco meant to the bParking, the computing performance are reported in the same description.
  • Even with the tight working point which will be activated by default in the standard workflows the cpu time requested for these low pt electrons adds up to some 4.5% of the whole RECO+miniAOD chain.
  • This is not negligible, but in line with what already evaluated (and considered acceptable) with the previous models
• Jenkins tests show no differences because the new low pt electrons are not yet monitored by them. The new models however end up in an enlarged selection for the low pt electrons with the tight working point, see for example in the wf 136.85_RunEGamma2018A where the number of reconstructed low pt electrons, tight wp, increases by some 10%:
  • Pt of the low pt electrons with the original model in the 10_6_X IB baseline, tight wp:
    
  • Pt of the low pt electrons with the new model as in this PR, tight wp:
    

[cmsbuild]

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)

[fabiocos]

+1

the needed external cms-sw/cmsdist#4728 has been merged has well