Use also the fourth hit of pixel track in SeedGeneratorFromProtoTracksEDProducer #26322

makortel · 2019-04-02T12:13:24Z

PR description:

I noticed that SeedGeneratorFromProtoTracksEDProducer does not include the possible fourth hit of a pixel tracks when creating TrajectorySeeds from pixel tracks (when useProtoTrackKinematics == False). This PR adds also the fourth hit.

In addition of affecting HLT with phase1 pixel, also phase0 HI tracking is affected, as TrackCleaner is used there for pixel tracks and the cleaner merges tracks.

PR validation:

I have here MTV plots (done in 10_4_0_patch1 with ttbar+PU 50 events on 2018 realistic conditions) comparing the effect with both the "offline MTV cuts" and "online MTV cuts"
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/
The main effects seem to be

iter0 efficiency increases and iter1 decreases (fakes and duplicates stay as they are)
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hlt_hltIter0PFlowTrackSelectionHighPurity/effandfakePtEtaPhi.pdf
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hlt_hltIter1PFlowTrackSelectionHighPurity/effandfakePtEtaPhi.pdf
iter1Merged efficiency stays roughly the same, but duplicate rate increases
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hlt_hltIter1Merged/effandfakePtEtaPhi.pdf
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hlt_hltIter1Merged/dupandfakePtEtaPhi.pdf
hltMerged efficiency stays roughly the same, but duplicate rate increases
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hlt_hltMerged/effandfakePtEtaPhi.pdf
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hlt_hltMerged/dupandfakePtEtaPhi.pdf
iter0 produces more tracks with 4 pixel layers
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hlt_hltIter0PFlowTrackSelectionHighPurity/distHitsLayers.pdf
which is probably the reason why the mean number of pixel hits decreases
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hlt_hltIter0PFlowTrackSelectionHighPurity/hitsLayers.pdf

@mtosi @JanFSchulte

cmsbuild · 2019-04-02T12:13:52Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-04-02T12:22:45Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26322/9025

This PR adds an extra 16KB to repository

cmsbuild · 2019-04-02T12:23:10Z

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

RecoTracker/TkSeedGenerator

@cmsbuild, @perrotta, @slava77 can you please review it and eventually sign? Thanks.
@felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @gpetruc, @ebrondol, @dgulhan 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

makortel · 2019-04-02T12:25:11Z

@cmsbuild, please test

cmsbuild · 2019-04-02T12:25:38Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/33904/console Started: 2019/04/02 14:35

cmsbuild · 2019-04-02T14:41:48Z

+1
Tested at: 6b4fa8b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-26322/33904/summary.html

cmsbuild · 2019-04-02T14:41:58Z

Comparison job queued.

cmsbuild · 2019-04-02T16:24:57Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 236 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3139747
DQMHistoTests: Total failures: 7477
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3132073
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

slava77 · 2019-04-04T14:48:46Z

I see differences in the run-1 reco for heavy ions, wf 140.53.
@makortel are these expected?
I thought that we did things correctly for phase-0 with this code.

makortel · 2019-04-04T15:00:45Z

Probably everything is a consequence of this

What I don't understand is how there can be 4 hits in the pixel track.

VinInn · 2019-04-04T15:34:34Z

overlaps.
It requires seed-region rebuild though

makortel · 2019-04-04T15:39:35Z

@VinInn These are pixel tracks (used as seeds). IIRC we don't have any mechanism in (legacy) seeding to go beyond 3 hits.

I added some printouts and the pattern seems to be (e.g.) BPix1+BPix2+BPix3+FPix1.

makortel · 2019-04-04T15:41:10Z

I saw also one BPix1+BPix2+BPix3+BPix3

makortel · 2019-04-04T15:48:07Z

Seems that TrackCleaner (used for producing hiPixelTracks) actually merges tracks
https://github.com/cms-sw/cmssw/blob/master/RecoPixelVertexing/PixelLowPtUtilities/src/TrackCleaner.cc#L161
so that explains the existence of 4-hit pixel tracks with phase0 pixel. So the changes are expected (I'll update the description).

slava77 · 2019-05-15T13:45:25Z

Now master moved to 11_0_X, maybe we could proceed with this PR?

in the meantime, was the impact of the changes discussed in the TSG already?
@mmasciov @mtosi @JanFSchulte

slava77 · 2019-05-15T20:52:13Z

@cmsbuild please test

cmsbuild · 2019-05-15T20:52:46Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/236/console Started: 2019/05/15 22:55

cmsbuild · 2019-05-15T22:22:26Z

+1
Tested at: 6b4fa8b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15aafa/236/summary.html

cmsbuild · 2019-05-15T22:22:31Z

Comparison job queued.

cmsbuild · 2019-05-16T01:13:52Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 241 differences found in the comparisons
DQMHistoTests: Total files compared: 33
DQMHistoTests: Total histograms compared: 3212004
DQMHistoTests: Total failures: 11091
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3200579
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
Checked 137 log files, 14 edm output root files, 33 DQM output files

slava77 · 2019-05-16T14:01:38Z

assign hlt

mostly because the most affected parts are in the code used by HLT.

cmsbuild · 2019-05-16T14:02:08Z

New categories assigned: hlt

@Martin-Grunewald,@fwyzard you have been requested to review this Pull request/Issue and eventually sign? Thanks

slava77 · 2019-05-17T00:50:39Z

Here are some notes based on the jenkins tests.

Changes in reco are only in the heavyIons scenario, in Run1 wf 140.53. pixel tracking is used there and changes are expected. There are a few more tracks, and the tracks are generally longer.

This is a consequence of having more hits on the track seeds as mentioned earlier in Use also the fourth hit of pixel track in SeedGeneratorFromProtoTracksEDProducer #26322 (comment).
All other differences are in the HLT and appear to be in line with the observations provided in the PR description.
I'm posting a few of the plots here inline based on https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_10_4_0_patch1_hlt/plots_hltOnline.html

For hltMerged:
The duplicate rate goes up by around a factor of 4

The number of layers with measurements in PXL is up (mostly around eta 1.5), but the number of hits goes down (most visible for eta > 2)

more details from wf 250202.18 show that all the reduction in the number of hits is in PXF

It looks like the changes proposed with this PR are exposing some "features" of tracking that may need some mitigation

to get all relevant hits on the tracks
to reduce the duplicate rate

I think that this PR is incomplete.
Perhaps some tests that can show that the HLT rates remain roughly unchanged can be reassuring that there are no adverse consequences in a big picture.

slava77 · 2019-05-23T13:43:13Z

@makortel @JanFSchulte @mtosi @mmasciov
please let me know if there were any actions from the tracking side on this PR and my comments to it.
Thank you.

slava77 · 2019-05-30T13:38:04Z

@makortel @JanFSchulte @mtosi @mmasciov
please let me know if there were any actions from the tracking side on this PR and my comments to it.
Thank you.

ping

JanFSchulte · 2019-05-30T14:25:52Z

@slava77 Sorry for the late reply. We discussed this and think that we would want to understand the effects of this PR better before merging it. However, as we are going to revamp the HLT tracking anyway for Run 3 we will look into this in that context later on. So this PR can be closed for the moment.

makortel · 2019-05-30T15:57:29Z

It occurred to me that the effect of the bug is actually close to the "seed extension" approach that we studied in 2016, i.e. you take the pixel triplet seed and let the pattern recognition to figure out the right hit(s) on the fourth layer (except in this case the seeds are already guaranteed to have a compatible hit (with some definition) on the fourth layer, and the pattern recognition is not strictly required to include a hit from the fourth layer). I suppose that leads to the increase in duplicates.

Here are some old slides on the subject
https://indico.cern.ch/event/508493/contribution/3/attachments/1242946/1828945/slides.pdf
https://indico.cern.ch/event/527844/contributions/2161117/attachments/1269921/1881271/slides_phase1_20160509.pdf

The (pixel) seed extension was enabled in initialStep in #14621 (for the triplet propagation seeding), and was disabled (for the default sequence) in #16911 when the seeding approach was changed to CA. I don't think we ever tested the seed extension approach with CA.

slava77 · 2019-05-31T16:42:23Z

@makortel
if you agree with #26322 (comment)
please close this PR.

makortel · 2019-05-31T17:58:15Z

Fine.

Use also the fourth hit of pixel track for the seed

6b4fa8b

cmsbuild added this to the CMSSW_10_6_X milestone Apr 2, 2019

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 2, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 2, 2019

cmsbuild added tests-started and removed tests-pending labels Apr 2, 2019

cmsbuild added tests-approved and removed tests-started labels Apr 2, 2019

cmsbuild added comparison-available and removed comparison-pending labels Apr 2, 2019

cmsbuild removed the hold label May 15, 2019

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels May 15, 2019

cmsbuild added tests-started and removed tests-pending labels May 15, 2019

cmsbuild added tests-approved and removed tests-started labels May 15, 2019

cmsbuild added comparison-available and removed comparison-pending labels May 16, 2019

cmsbuild added the hlt-pending label May 16, 2019

makortel closed this May 31, 2019

makortel mentioned this pull request Aug 13, 2019

Integration to CMSSW master trackreco/mkFit#236

Closed

18 tasks

makortel mentioned this pull request Nov 26, 2019

Make it possible to use the fourth hit of a pixel track in SeedGeneratorFromProtoTracksEDProducer #28473

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use also the fourth hit of pixel track in SeedGeneratorFromProtoTracksEDProducer #26322

Use also the fourth hit of pixel track in SeedGeneratorFromProtoTracksEDProducer #26322

makortel commented Apr 2, 2019 •

edited

Loading

cmsbuild commented Apr 2, 2019

cmsbuild commented Apr 2, 2019

cmsbuild commented Apr 2, 2019

makortel commented Apr 2, 2019

cmsbuild commented Apr 2, 2019 •

edited

Loading

cmsbuild commented Apr 2, 2019

cmsbuild commented Apr 2, 2019

cmsbuild commented Apr 2, 2019

slava77 commented Apr 4, 2019

makortel commented Apr 4, 2019

VinInn commented Apr 4, 2019

makortel commented Apr 4, 2019

makortel commented Apr 4, 2019

makortel commented Apr 4, 2019

slava77 commented May 15, 2019

slava77 commented May 15, 2019

cmsbuild commented May 15, 2019 •

edited

Loading

cmsbuild commented May 15, 2019

cmsbuild commented May 15, 2019

cmsbuild commented May 16, 2019

slava77 commented May 16, 2019

cmsbuild commented May 16, 2019

slava77 commented May 17, 2019 •

edited

Loading

slava77 commented May 23, 2019

slava77 commented May 30, 2019

JanFSchulte commented May 30, 2019

makortel commented May 30, 2019

slava77 commented May 31, 2019

makortel commented May 31, 2019

Use also the fourth hit of pixel track in SeedGeneratorFromProtoTracksEDProducer #26322

Use also the fourth hit of pixel track in SeedGeneratorFromProtoTracksEDProducer #26322

Conversation

makortel commented Apr 2, 2019 • edited Loading

PR description:

PR validation:

cmsbuild commented Apr 2, 2019

cmsbuild commented Apr 2, 2019

cmsbuild commented Apr 2, 2019

makortel commented Apr 2, 2019

cmsbuild commented Apr 2, 2019 • edited Loading

cmsbuild commented Apr 2, 2019

cmsbuild commented Apr 2, 2019

cmsbuild commented Apr 2, 2019

slava77 commented Apr 4, 2019

makortel commented Apr 4, 2019

VinInn commented Apr 4, 2019

makortel commented Apr 4, 2019

makortel commented Apr 4, 2019

makortel commented Apr 4, 2019

slava77 commented May 15, 2019

slava77 commented May 15, 2019

cmsbuild commented May 15, 2019 • edited Loading

cmsbuild commented May 15, 2019

cmsbuild commented May 15, 2019

cmsbuild commented May 16, 2019

slava77 commented May 16, 2019

cmsbuild commented May 16, 2019

slava77 commented May 17, 2019 • edited Loading

slava77 commented May 23, 2019

slava77 commented May 30, 2019

JanFSchulte commented May 30, 2019

makortel commented May 30, 2019

slava77 commented May 31, 2019

makortel commented May 31, 2019

makortel commented Apr 2, 2019 •

edited

Loading

cmsbuild commented Apr 2, 2019 •

edited

Loading

cmsbuild commented May 15, 2019 •

edited

Loading

slava77 commented May 17, 2019 •

edited

Loading