expand tracker-driven coverage for electron seeds; jetCore mitigation #35892

slava77 · 2021-10-29T05:50:38Z

Changes in this PR were motivated to mitigate the situation with tracks reconstructed multiple times, one of them jet core after introduction of mkFit to the non-jetCore iterations. Such tracks are more predominantly assigned to jetCore algorithm.

EGM/electron reco has some dependence on this:

jetCore is not counted in PFEgammaAlgo as isGoodForEGMPrimary : to address this the same check is extended to track's originalAlgo to take into account also the other iteration that may have reconstructed the same track
jetCore seeds are not included in the definition of ecalDrivenElectronSeeds and in the subsequent step where ecalDrivenElectronSeeds are merged with the trackerDrivenElectronSeeds (including jetCore) the check by seed hits alone is failing to assign a matching track to the ecal-driven seeds. To address this, the loop to match the hits is extended to include all of the track hits.

This was presented in TRK POG on Nov 1: https://indico.cern.ch/event/1092090/#3-interplay-in-high-pt-electro

Illustrations of the issue:
in pre4 Z'->ee https://tinyurl.com/yfd22fxr the track algorithm is more dominantly the jetCore (starts at 100 GeV)

the reconstructed electron provenance https://tinyurl.com/yg4rfp7c is showing a drop in the tracker-driven electrons { provenance: +1 ecal; -1 tracker; 0 either; +2 ecal-only; -2 tracker-only}

After this PR, on the same sample, only 100 events
default tracking with mkFit (red is new, black is pre4)

even in the CKF tracking (reco with Run3_noMkFit) there is a noticeable increase in the tracker-driven categorization and decrease in the ecal-only

…r driven seeds

slava77 · 2021-10-29T05:51:32Z

@cms-sw/egamma-pog-l2 @cms-sw/tracking-pog-l2
this PR is following the email exchange on this subject

slava77 · 2021-10-29T05:57:59Z

enable profiling

slava77 · 2021-10-29T06:03:36Z

RecoParticleFlow/PFTracking/plugins/ElectronSeedMerger.cc

      }
      if (hitShared == (hitSeed - 1)) {
-        NewSeed.setCtfTrack(TSeed[it].ctfTrack());
+        newSeed.setCtfTrack(tSeed.ctfTrack());
+      } else if (hitShared > 0 && !newSeed.isTrackerDriven()) {


to reduce the number of times the loop over the full track hits is used, I'm requiring here a partial match with at least one hit from the seed.

This will not help for deepCore when it gets enabled
IIUC, the deepCore seeds have no hits and this part will block the option to match to the track hits. But perhaps for this case the solution would be to actually fill the hits for deepCore

here, can we add in || something like if (newSeed.nHits() == 0) so that deepCore will also pass this condition and enter the following loop. So, if the seed has no hit then this partial match of at least 1 hit is not required. Will that be okay?

slava77 · 2021-10-29T06:14:59Z

@smuzaffar
Somehow in the console for code-checks
https://cmssdt.cern.ch/jenkins/job/run-pr-code-checks/26294/console
the job was stuck on
07:53:51 + git repack -a -d --threads 16
for 20 minutes.
Is it a problem with the node, or did dealing with git become so much slower?

mmusich · 2021-10-29T06:15:15Z

This is now apparently targeted for 12.2.x. I just want to check if that's fine for POG/PAG sample production view, the schedule changed so many times, I lost track of what is the plan.

cmsbuild · 2021-10-29T06:15:58Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35892/26294

This PR adds an extra 36KB to repository

cmsbuild · 2021-10-29T06:16:17Z

A new Pull Request was created by @slava77 (Slava Krutelyov) for master.

It involves the following packages:

RecoParticleFlow/PFProducer (reconstruction)
RecoParticleFlow/PFTracking (reconstruction)

@jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @cbernet, @rovere, @lgray, @lecriste, @hatakeyamak, @ebrondol, @seemasharmafnal 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

slava77 · 2021-10-29T06:16:45Z

This is now apparently targeted for 12.2.x. I just want to check if that's fine for POG/PAG sample production view, the schedule changed so many times, I lost track of what is the plan.

there will be one open pre-release in 12_2.
Some more details are available in the PPD news yesterday.

slava77 · 2021-10-29T06:16:56Z

@cmsbuild please test

perrotta · 2021-10-29T07:43:09Z

This is now apparently targeted for 12.2.x. I just want to check if that's fine for POG/PAG sample production view, the schedule changed so many times, I lost track of what is the plan.

Ciao @mmusich

The name of the releases changed, in fact, but the actual schedule from the point of view of the developers did not actually moved significantly from what was decided some time ago, see https://twiki.cern.ch/twiki/bin/viewauth/CMS/ReleaseSchedule

The last open pre-release available for POG developments meant to Run3 is on 2021/11/09: it is now called CMSSW_12_2_0_pre2 instead of CMSSW_12_1_0_preSomething

mmusich · 2021-10-29T08:56:41Z

thanks @perrotta and @slava77

swagata87 · 2021-10-29T11:02:25Z

here are plots from my validation,
https://www.dropbox.com/s/qat8nyvuh6tsw81/mkFit_EGM_v2.pdf?dl=0
things look good now.
Thank you Slava

cmsbuild · 2021-10-29T12:33:57Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4c1245/20034/summary.html
COMMIT: ec07080
CMSSW: CMSSW_12_1_X_2021-10-28-2300/slc7_amd64_gcc900
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35892/20034/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: 1169 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 2901440
DQMHistoTests: Total failures: 179
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2901239
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

slava77 · 2021-10-31T17:19:20Z

I added updates for deepCore.
The tests will have to be started only after the next IB shows up (the last IB is broken), so that there is a proper reference for comparisons.

cmsbuild · 2021-10-31T17:23:54Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35892/26328

This PR adds an extra 44KB to repository

cmsbuild · 2021-10-31T17:24:14Z

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

slava77 · 2021-11-01T12:23:55Z

@cmsbuild please test

slava77 · 2021-11-01T14:43:02Z

In the PR tests for the previous commit #35892 (comment) (which I expect to be repeated in the latest version) there were some changes in the PF CH vs ele assignments, e.g. in 136.793 DoubleEG2017C (100 events) there is one more electron vs 92 in the baseline (well, there is one more gedGsfElectron there as well; so it's not a pure reassignment inside PF).
So, the changes are not particularly alarming.

@cms-sw/pf-l2 @laurenhay @marksan87
please clarify if you would like to make some standalone validation with a bit more focus on the less pure electrons like in jets before this PR is merged or if it's OK to merge and wait for the relvals.
Thank you.

cmsbuild · 2021-11-01T20:44:39Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4c1245/20152/summary.html
COMMIT: 76ceb8e
CMSSW: CMSSW_12_2_X_2021-10-31-2300/slc7_amd64_gcc900
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35892/20152/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: 1167 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 2901890
DQMHistoTests: Total failures: 173
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2901695
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

slava77 · 2021-11-02T15:46:02Z

a link to the TRK POG presentation(s) is added in the PR description
https://indico.cern.ch/event/1092090/#3-interplay-in-high-pt-electro

slava77 · 2021-11-04T13:15:52Z

+reconstruction

for #35892 76ceb8e

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show small differences in egamma-related objects (there are not enough high-pt electrons in these samples to notice more than a few particle change)
additional checks on signal were posted in the PR description and also presented in https://indico.cern.ch/event/1092090/#3-interplay-in-high-pt-electro and show a recovery in the ecal-and-tracker driven electrons
standalone tests by PF experts show about 1% increase of electrons in the QCD sample (mostly fakes) and essentially no changes in the lower-pt signal (e.g. Z->EE) https://mattermost.web.cern.ch/cms-exp/pl/simkjfrh1png7y6cebini4e8rc The ~1% increase in fakes is considered acceptable.

cmsbuild · 2021-11-04T13:16:12Z

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)

perrotta · 2021-11-04T14:44:13Z

+1

Small differences in egamma related objects look as expected
About 1% increase of fake electrons in the QCD sample was considered acceptable
Igprof comparisons do not show significant differences in performance

…JetCore expand tracker-driven coverage for electron seeds; jetCore mitigation (bp of #35892)

slava77devel added 3 commits October 29, 2021 02:41

modernize and cleanup

b00799e

include isGoodForEGMPrimary by originalAlgo

0327bbe

add a loop over the track hits to improve coverage of ecal-and-tracke…

ec07080

…r driven seeds

cmsbuild added this to the CMSSW_12_1_X milestone Oct 29, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Oct 29, 2021

cmsbuild modified the milestones: CMSSW_12_1_X, CMSSW_12_2_X Oct 29, 2021

slava77 commented Oct 29, 2021

View reviewed changes

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 29, 2021

cmsbuild added tests-started and removed tests-pending labels Oct 29, 2021

cmsbuild added tests-rejected and removed tests-started labels Oct 29, 2021

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved labels Oct 31, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 31, 2021

cmsbuild added tests-started and removed tests-pending labels Nov 1, 2021

cmsbuild added tests-approved and removed tests-started labels Nov 1, 2021

slava77 mentioned this pull request Nov 2, 2021

expand tracker-driven coverage for electron seeds; jetCore mitigation (bp of #35892) #35958

Merged

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Nov 4, 2021

cmsbuild added orp-approved and removed orp-pending labels Nov 4, 2021

cmsbuild merged commit cc5a950 into cms-sw:master Nov 4, 2021

cmsbuild added a commit that referenced this pull request Nov 4, 2021

Merge pull request #35958 from slava77/CMSSW_12_1_0_pre4/sign1125/egm…

b5695d3

…JetCore expand tracker-driven coverage for electron seeds; jetCore mitigation (bp of #35892)

slava77 mentioned this pull request Dec 4, 2021

check both algo and originalAlgo in electron track isolation calculators #36364

Merged

swagata87 mentioned this pull request Dec 10, 2021

Crash in GsfTrackProducer:electronGsfTracks (observed in wf 11723.17) #36369

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expand tracker-driven coverage for electron seeds; jetCore mitigation #35892

expand tracker-driven coverage for electron seeds; jetCore mitigation #35892

slava77 commented Oct 29, 2021 •

edited

slava77 commented Oct 29, 2021

slava77 commented Oct 29, 2021

slava77 Oct 29, 2021

swagata87 Oct 29, 2021

slava77 commented Oct 29, 2021

mmusich commented Oct 29, 2021

cmsbuild commented Oct 29, 2021

cmsbuild commented Oct 29, 2021

slava77 commented Oct 29, 2021

slava77 commented Oct 29, 2021

perrotta commented Oct 29, 2021

mmusich commented Oct 29, 2021

swagata87 commented Oct 29, 2021

cmsbuild commented Oct 29, 2021

slava77 commented Oct 31, 2021

cmsbuild commented Oct 31, 2021

cmsbuild commented Oct 31, 2021

slava77 commented Nov 1, 2021

slava77 commented Nov 1, 2021

cmsbuild commented Nov 1, 2021

slava77 commented Nov 2, 2021

slava77 commented Nov 4, 2021

cmsbuild commented Nov 4, 2021

perrotta commented Nov 4, 2021

expand tracker-driven coverage for electron seeds; jetCore mitigation #35892

expand tracker-driven coverage for electron seeds; jetCore mitigation #35892

Conversation

slava77 commented Oct 29, 2021 • edited

slava77 commented Oct 29, 2021

slava77 commented Oct 29, 2021

slava77 Oct 29, 2021

Choose a reason for hiding this comment

swagata87 Oct 29, 2021

Choose a reason for hiding this comment

slava77 commented Oct 29, 2021

mmusich commented Oct 29, 2021

cmsbuild commented Oct 29, 2021

cmsbuild commented Oct 29, 2021

slava77 commented Oct 29, 2021

slava77 commented Oct 29, 2021

perrotta commented Oct 29, 2021

mmusich commented Oct 29, 2021

swagata87 commented Oct 29, 2021

cmsbuild commented Oct 29, 2021

RelVals-INPUT

Comparison Summary

slava77 commented Oct 31, 2021

cmsbuild commented Oct 31, 2021

cmsbuild commented Oct 31, 2021

slava77 commented Nov 1, 2021

slava77 commented Nov 1, 2021

cmsbuild commented Nov 1, 2021

Comparison Summary

slava77 commented Nov 2, 2021

slava77 commented Nov 4, 2021

cmsbuild commented Nov 4, 2021

perrotta commented Nov 4, 2021

slava77 commented Oct 29, 2021 •

edited