Remove pixel tracking from phase2

[makortel]

This PR removes pixel tracking from phase2, adds firstStepPrimaryVertices, and replaces all uses of pixelVertices with firstStepPrimaryVertices as is done in phase0/phase1 tracking.

Here are MTV plots in 100 events of ttbar+200PU in 910pre3 D11
https://cms-tracking-validation.web.cern.ch/cms-tracking-validation/PR/CMSSW_9_1_0_pre3_PR19165/
Efficiency stays essentially the same (tiny variations), fake rate decreases by 2 % (1 % in highPurity)

Timing should improve. In my test in 910pre3

• in the release
  • iterative tracking takes ~56 s/event
  • pixel tracking + vertexing takes ~6 s/event (not included in above)
• with this PR
  • firstStepPrimaryVertices (part of initialStep) takes ~2 s/event

I have not tested the timing of the sorting of firstStepPrimaryVertices from #19051 in the context of phase2 PU200, but I'd expect it to be small compared to the rest of the tracking.

Tested in 9_1_0_pre3 and 9_2_2. Expecting changes in phase2 as described above, and no changes in phase0/1.

@rovere @VinInn @ebrondol

[cmsbuild]

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

It involves the following packages:

Configuration/StandardSequences
RecoJets/JetProducers
RecoPixelVertexing/Configuration
RecoPixelVertexing/PixelTrackFitting
RecoTracker/ConversionSeedGenerators
RecoTracker/FinalTrackSelectors
RecoTracker/IterativeTracking
RecoTracker/TkTrackingRegions
Validation/RecoVertex

@perrotta, @dmitrijus, @cmsbuild, @franzoni, @slava77, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @TaiSakuma, @yslai, @felicepantaleo, @rappoccio, @Martin-Grunewald, @seemasharmafnal, @venturia, @ahinzmann, @jdolen, @GiacomoSguazzoni, @rovere, @VinInn, @nhanvtran, @gkasieczka, @schoef, @mschrode, @ebrondol, @mtosi, @dgulhan, @gpetruc, @mariadalfonso this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

[makortel]

@cmsbuild, please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20456/console Started: 2017/06/09 15:57

[cmsbuild]

-1

Tested at: 7423b86

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19165/20456/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

• RelVals:

When I ran the RelVals I found an error in the following worklfows:
20434.0 step3

runTheMatrix-results/20434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D10_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D10+RecoFullGlobal_2023D10+HARVESTFullGlobal_2023D10/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D10_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D10+RecoFullGlobal_2023D10+HARVESTFullGlobal_2023D10.log

27434.0 step3

runTheMatrix-results/27434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFull_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFull_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17.log

23234.0 step3

runTheMatrix-results/23234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D18_GenSimHLBeamSpotFull14+DigiFull_2023D18+RecoFullGlobal_2023D18+HARVESTFullGlobal_2023D18/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D18_GenSimHLBeamSpotFull14+DigiFull_2023D18+RecoFullGlobal_2023D18+HARVESTFullGlobal_2023D18.log

[cmsbuild]

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

[slava77]

20434 step3:

   [15] Calling method for module TrackClusterRemoverPhase2/'highPtTripletStepClusters'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: edm::ValueMap<int>
Looking for module label: initialStepSelector
Looking for productInstanceName: initialStep

@makortel
perhaps you have missed some commits?

[cmsbuild]

Pull request #19165 was updated. @perrotta, @dmitrijus, @cmsbuild, @franzoni, @slava77, @vanbesien, @davidlange6 can you please check and sign again.

[makortel]

@cmsbuild, please test

Should be fixed now. My testing setup was apparently a bit flawed because at first (before submitting the PR) I didn't see the problem (but eventually I was able to reproduce).

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20527/console Started: 2017/06/13 14:26

[cmsbuild]

+1
Tested at: fc87228
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19165/20527/summary.html

[cmsbuild]

Comparison job queued.

[dmitrijus]

+1

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 430 differences found in the comparisons
• DQMHistoTests: Total files compared: 22
• DQMHistoTests: Total histograms compared: 1817858
• DQMHistoTests: Total failures: 18746
• DQMHistoTests: Total nulls: 640
• DQMHistoTests: Total successes: 1798306
• DQMHistoTests: Total skipped: 166
• DQMHistoTests: Total Missing objects: 0
• Checked 90 log files, 14 edm output root files, 22 DQM output files

[slava77]

+1

for #19165 fc87228

• implemented as described; changes affect only 2023 workflows
• jenkins tests pass and comparisons with baseline show differences only in 2023 workflows and the only significant changes are the expected disappearance of the pixelTracks and pixelVertices
• local tests with higher stats muon guns and PU200 show expected behavior, to supplement the MTV plots provided in the PR description: changes on quantities downstream from early tracking steps are negligibly small.
  • CPU is reduced by about 3% total (for full reco+miniAOD): essentially all change is in tracking modules where the pixel tracking removal (-10 s) is absorbed by some increase in iterative tracking (+2 s; mostly from pixelPairStep)
  • event content is reduced by the size of the pixelTracks and pixelVertices (about 0.2 MB/event in the RECO file, which is a 0.6% reduction in this case).

[davidlange6]

merge

[kpedro88]

@slava77 please assign upgrade to future PRs like this one

[slava77]

@kpedro88
sorry.
I'll try to remember (until the next time, I guess).

Maybe some rule can be written for the bot to do it automatically.
E.g. wget the diff and grep for various patterns of Phase2, Phase-2 or similar appearing there.
You can then unassign if it's unrelated.

[kpedro88]

I'll think about it, but it seems likely prone to false positives (and also would "secretly" break the default category-based assignment that's relatively easy for developers to follow).

[slava77]

On 6/21/17 2:30 PM, Kevin Pedro wrote: I'll think about it, but it seems likely prone to false positives (and also would "secretly" break the default category-based assignment that's relatively easy for developers to follow).

this PR had 3 reviewers before it was merged. Nobody remembered to assign upgrade. It's a sampling with not a very good yield, if you expect to get an average close to 100%.

…

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