[EGM] Undo relaxing of GSF nHit cuts for phase2

[swagata87]

PR description:

In the context of eta-extended-electrons (#35309, which was merged some time ago) we had relaxed cuts on number of hits in tracker while reconstructing the GSF track. This is mostly relevant for Run3, i.e. Phase1 tracker geometry, where outer-tracker coverage ends soon after |eta|=2.5. In this PR, we are stopping to propagate these relaxation of cuts to Phase2, because phase2 outer-tracker has much better eta coverage, and thus the electrons would be eta-extended anyway.

It was found in validation that, in phase2 the relaxation of cuts actually does more harm than good in very high PU scenario.
Link: Slide 13 of https://indico.cern.ch/event/1085302/contributions/4563231/attachments/2332176/3974663/Release_Validation_Report_2021_10_21.pdf

This PR aims to solve that issue.

This PR should not have any effect on Run3 (or Run2), the only effect is expected in Phase2. The effect should be visible only in high PU scenario.

PR validation:

runTheMatrix.py -l 34834.999 ran fine. It is Phase2 ttbar with PU=50.
Then analysed its output AOD file, and compared some quantities before(in red) and after(in blue) this PR.

1. numberOfValidHits() in GSF tracks from collection Handle("vector<reco::GsfTrack>"), "electronGsfTracks". As expected, after this PR there is no GSF track with nhit<5.

2. eta of electron from the collection Handle("vector<reco::GsfElectron>"), "ecalDrivenGsfElectronsHGC". Only change observed after this PR is at high eta.

This PR is not a backport.
Backport not needed.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35887/26284

• This PR adds an extra 16KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File TrackingTools/GsfTracking/python/CkfElectronCandidateMaker_cff.py modified in PR(s): [WIP] add fillPSetDescription to CkfTrackCandidateMakerBase and propagate it to CkfTrackCandidateMaker #35385

[cmsbuild]

A new Pull Request was created by @swagata87 (Swagata Mukherjee) for master.

It involves the following packages:

• TrackingTools/GsfTracking (reconstruction)

@jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @bellan, @ebrondol, @lecriste, @gpetruc, @mmusich, @mtosi, @dgulhan this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[srimanob]

@swagata87
Thanks very much. I am wondering if we can test it before this PR is approved? For example, run RECO from RAW of PU 200 Relvals. I am not sure if triggering the test of PU200 is suitable for PR test infra.

[srimanob]

@cmsbuild please test

[cmsbuild]

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-af63e2/20026/summary.html
COMMIT: adca5c3
CMSSW: CMSSW_12_1_X_2021-10-27-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35887/20026/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

• 138.3138.3_RunMinimumBias2021Splash+RunMinimumBias2021Splash+RECODR3Splash+HARVESTDR3/step2_RunMinimumBias2021Splash+RunMinimumBias2021Splash+RECODR3Splash+HARVESTDR3.log

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 354 differences found in the comparisons
• DQMHistoTests: Total files compared: 42
• DQMHistoTests: Total histograms compared: 2901440
• DQMHistoTests: Total failures: 278
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 2901139
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -0.004 KiB( 41 files compared)
• DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
• Checked 177 log files, 37 edm output root files, 42 DQM output files
• TriggerResults: no differences found

[swagata87]

@swagata87 Thanks very much. I am wondering if we can test it before this PR is approved? For example, run RECO from RAW of PU 200 Relvals. I am not sure if triggering the test of PU200 is suitable for PR test infra.

that's a rather significant amount of work.
Is there anything particularly worrying about this PR that this additional check is asked?

[slava77]

assign upgrade

based on the interest expressed in #35887 (comment)

[cmsbuild]

New categories assigned: upgrade

@AdrianoDee,@srimanob you have been requested to review this Pull request/Issue and eventually sign? Thanks

[slava77]

what about particleFlowTmp.PFEGammaFiltersParameters.allowEEEinPF ?

BTW, why are the settings in #35309 not restricted to trackingPhase1? doesn't the default degrade the 2016 (and older) reco?

[srimanob]

Hi @swagata87
I've done 300 events RECO-only with 12_1_0_pre4 and 12_1_0_pre4+#35887

Here is a plot shown in PdmV slide,
https://tinyurl.com/yeuzm967
it seems to confirm the recovery from this PR.

[srimanob]

@cmsbuild please test

It should not crash now after #35890 is merged.

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-af63e2/20107/summary.html
COMMIT: adca5c3
CMSSW: CMSSW_12_2_X_2021-10-30-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35887/20107/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: 356 differences found in the comparisons
• DQMHistoTests: Total files compared: 42
• DQMHistoTests: Total histograms compared: 2901440
• DQMHistoTests: Total failures: 273
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 2901144
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -0.004 KiB( 41 files compared)
• DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
• Checked 177 log files, 37 edm output root files, 42 DQM output files
• TriggerResults: no differences found

[swagata87]

what about particleFlowTmp.PFEGammaFiltersParameters.allowEEEinPF ?

The switch allowEEEinPF in particleFlow_cff.py is a temporary hack to stop eta-extended-electrons(EEE) from entering particle-flow. This is because for EEEs we do not have proper PF Egamma ID and proper energy regression. So if EEEs enter PF now, it might slightly mess with Jet/MET (the effect is very small, as found in PF validation, but it was found after the EEE PR was merged).

We will request O(100M) events to be produced with 12_1, with these samples we will retrain our PF Egamma ID and energy regression including EEEs. Then we will allow the EEEs into PF in 12_3 onwards. The situation is described in this github issue: #35374

[swagata87]

BTW, why are the settings in #35309 not restricted to trackingPhase1? doesn't the default degrade the 2016 (and older) reco?

Before EEE PR (#35309), the default was, minimum 5 tracker hits should be there in GSF. After EEE PR, the default is, minimum 5 tracker hits until |eta|<2.5 and minimum 3 tracker hits for |eta|>2.5.

I thought that this relaxation of cuts will not harm 2016 and older. But if I'm missing something here, please let me know. It should be possible to restrict the EEE setting only to trackingPhase1 or even only to run3, as it's aimed for run3 analyses. I did not restrict as I thought that 2016 and before will either stay same or slightly improve with relaxed nHit cuts at high eta.

[swagata87]

Hi @swagata87 I've done 300 events RECO-only with 12_1_0_pre4 and 12_1_0_pre4+#35887

Here is a plot shown in PdmV slide, https://tinyurl.com/yeuzm967 it seems to confirm the recovery from this PR.

Many thanks for checking this

[slava77]

+reconstruction

for #35887 adca5c3

• code changes are in line with the PR description and the follow up checks in e.g. [EGM] Undo relaxing of GSF nHit cuts for phase2 #35887 (comment)
• jenkins tests pass and comparisons with the baseline show differences only in phase-2 workflow 34834.999 , the changes start from the GSF tracks and propagate downstream, e.g. it's visible in the reduction of the number of short tracks, as expected from this PR change

[slava77]

I thought that this relaxation of cuts will not harm 2016 and older. But if I'm missing something here, please let me know. It should be possible to restrict the EEE setting only to trackingPhase1 or even only to run3, as it's aimed for run3 analyses. I did not restrict as I thought that 2016 and before will either stay same or slightly improve with relaxed nHit cuts at high eta.

Thanks for clarifying. It's OK to stay as is, if you expect that the 2016 will not degrade.

[srimanob]

+Upgrade

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

[qliphy]

+1