Skip to content

Conversation

@mmasciov
Copy link
Contributor

@mmasciov mmasciov commented Oct 19, 2022

PR description:

Using option speed=0 for SiPixel template CPE with cluster repair (default SiPixel CPE for phase-1), that disables the computation of Q probability:

  • a new instance of transient track recHit builder is defined with option speed=0 (instead of -2), only used in main iterative tracking;
  • a warning is disabled for the default value of Q probability (-1) when not computed.

This is a RFC PR, to collect comments (especially in case anything downstream is affected).

PR validation:

As shown in report, disabling the Q probability computation does not affect physics (zero impact), while it allows to reduce the track final fit time by about 15-20%.

FYI, @mmusich @slava77 @pmaksim1 @tvami

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39776/32643

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmasciov (Mario Masciovecchio) for master.

It involves the following packages:

  • DataFormats/TrackerRecHit2D (reconstruction)
  • RecoLocalTracker/SiPixelRecHits (reconstruction)
  • RecoTracker/TrackProducer (reconstruction)
  • RecoTracker/TransientTrackingRecHit (reconstruction)

@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks.
@VourMa, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @mroguljic, @missirol, @dkotlins, @ebrondol, @ferencek, @mtosi, @gpetruc, @mmusich, @threus, @dgulhan, @tvami this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor

mmusich commented Oct 19, 2022

type trk, tracking, performance-improvements

@mmusich
Copy link
Contributor

mmusich commented Oct 19, 2022

assign trk-dpg

@cmsbuild
Copy link
Contributor

New categories assigned: trk-dpg

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

@mmusich
Copy link
Contributor

mmusich commented Oct 19, 2022

please test

@tvami
Copy link
Contributor

tvami commented Oct 19, 2022

It would be nice to produce a sensibly sized-AOD file so I can run my framework on the output of this. Just looking at the code changes, this should be all good!

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f4db58/28367/summary.html
COMMIT: 5aa1f4a
CMSSW: CMSSW_12_6_X_2022-10-19-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39776/28367/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testPhase2PixelNtuple had ERRORS

RelVals

----- Begin Fatal Exception 19-Oct-2022 18:12:43 CEST-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'dqmoffline_8_step'
   [2] Prefetching for module PFCandidateAnalyzerDQM/'PFCandAnalyzerDQM'
   [3] Prefetching for module PATPackedCandidateProducer/'packedPFCandidates'
   [4] Prefetching for module PFLinker/'particleFlow'
   [5] Prefetching for module PFCandidateListMerger/'particleFlowTmp'
   [6] Prefetching for module PFProducer/'particleFlowTmpBarrel'
   [7] Prefetching for module PFBlockProducer/'particleFlowBlock'
   [8] Prefetching for module PFElecTkProducer/'pfTrackElec'
   [9] Prefetching for module GsfTrackProducer/'electronGsfTracks'
   [10] Prefetching for module CkfTrackCandidateMaker/'electronCkfTrackCandidates'
   [11] Prefetching for module ElectronSeedMerger/'electronMergedSeeds'
   [12] Prefetching for module ElectronSeedProducer/'ecalDrivenElectronSeeds'
   [13] Prefetching for module SeedCreatorFromRegionConsecutiveHitsEDProducer/'highPtTripletStepSeeds'
   [14] Prefetching for module CAHitTripletEDProducer/'highPtTripletStepHitTriplets'
   [15] Prefetching for module HitPairEDProducer/'highPtTripletStepHitDoublets'
   [16] Prefetching for module SeedingLayersEDProducer/'highPtTripletStepSeedLayers'
   [17] Prefetching for module TrackClusterRemoverPhase2/'highPtTripletStepClusters'
   [18] Prefetching for module TrackProducer/'initialStepTracks'
   [19] Prefetching for EventSetup module TkTransientTrackingRecHitBuilderESProducer/'TTRHBuilderAngleAndTemplateWithoutProbQ'
   [20] Calling method for EventSetup module StripCPEESProducer/'StripCPEfromTrackAngleESProducer'
   [21] While getting dependent Record from Record TkStripCPERecord
Exception Message:
No "SiStripLorentzAngleDepRcd" record found in the EventSetup.

 The Record is delivered by an ESSource or ESProducer but there is no valid IOV for the synchronization value.
 Please check 
   a) if the synchronization value is reasonable and report to the hypernews if it is not.
   b) else check that all ESSources have been properly configured. 
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 19-Oct-2022 18:12:53 CEST-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'dqmoffline_8_step'
   [2] Prefetching for module PFCandidateAnalyzerDQM/'PFCandAnalyzerDQM'
   [3] Prefetching for module PATPackedCandidateProducer/'packedPFCandidates'
   [4] Prefetching for module PFLinker/'particleFlow'
   [5] Prefetching for module PFCandidateListMerger/'particleFlowTmp'
   [6] Prefetching for module PFProducer/'particleFlowTmpBarrel'
   [7] Prefetching for module PFBlockProducer/'particleFlowBlock'
   [8] Prefetching for module PFElecTkProducer/'pfTrackElec'
   [9] Prefetching for module GsfTrackProducer/'electronGsfTracks'
   [10] Prefetching for module CkfTrackCandidateMaker/'electronCkfTrackCandidates'
   [11] Prefetching for module ElectronSeedMerger/'electronMergedSeeds'
   [12] Prefetching for module ElectronSeedProducer/'ecalDrivenElectronSeeds'
   [13] Prefetching for module SeedCreatorFromRegionConsecutiveHitsEDProducer/'highPtTripletStepSeeds'
   [14] Prefetching for module CAHitTripletEDProducer/'highPtTripletStepHitTriplets'
   [15] Prefetching for module HitPairEDProducer/'highPtTripletStepHitDoublets'
   [16] Prefetching for module SeedingLayersEDProducer/'highPtTripletStepSeedLayers'
   [17] Prefetching for module TrackClusterRemoverPhase2/'highPtTripletStepClusters'
   [18] Prefetching for module TrackProducer/'initialStepTracks'
   [19] Prefetching for EventSetup module TkTransientTrackingRecHitBuilderESProducer/'TTRHBuilderAngleAndTemplateWithoutProbQ'
   [20] Calling method for EventSetup module StripCPEESProducer/'StripCPEfromTrackAngleESProducer'
   [21] While getting dependent Record from Record TkStripCPERecord
Exception Message:
No "SiStripLorentzAngleDepRcd" record found in the EventSetup.

 The Record is delivered by an ESSource or ESProducer but there is no valid IOV for the synchronization value.
 Please check 
   a) if the synchronization value is reasonable and report to the hypernews if it is not.
   b) else check that all ESSources have been properly configured. 
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 19-Oct-2022 18:15:44 CEST-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'dqmoffline_8_step'
   [2] Prefetching for module PFCandidateAnalyzerDQM/'PFCandAnalyzerDQM'
   [3] Prefetching for module PATPackedCandidateProducer/'packedPFCandidates'
   [4] Prefetching for module PFLinker/'particleFlow'
   [5] Prefetching for module PFCandidateListMerger/'particleFlowTmp'
   [6] Prefetching for module PFProducer/'particleFlowTmpBarrel'
   [7] Prefetching for module PFBlockProducer/'particleFlowBlock'
   [8] Prefetching for module PFElecTkProducer/'pfTrackElec'
   [9] Prefetching for module GsfTrackProducer/'electronGsfTracks'
   [10] Prefetching for module CkfTrackCandidateMaker/'electronCkfTrackCandidates'
   [11] Prefetching for module ElectronSeedMerger/'electronMergedSeeds'
   [12] Prefetching for module ElectronSeedProducer/'ecalDrivenElectronSeeds'
   [13] Prefetching for module SeedCreatorFromRegionConsecutiveHitsEDProducer/'highPtTripletStepSeeds'
   [14] Prefetching for module CAHitTripletEDProducer/'highPtTripletStepHitTriplets'
   [15] Prefetching for module HitPairEDProducer/'highPtTripletStepHitDoublets'
   [16] Prefetching for module SeedingLayersEDProducer/'highPtTripletStepSeedLayers'
   [17] Prefetching for module TrackClusterRemoverPhase2/'highPtTripletStepClusters'
   [18] Prefetching for module TrackProducer/'initialStepTracks'
   [19] Prefetching for EventSetup module TkTransientTrackingRecHitBuilderESProducer/'TTRHBuilderAngleAndTemplateWithoutProbQ'
   [20] Calling method for EventSetup module StripCPEESProducer/'StripCPEfromTrackAngleESProducer'
   [21] While getting dependent Record from Record TkStripCPERecord
Exception Message:
No "SiStripLorentzAngleDepRcd" record found in the EventSetup.

 The Record is delivered by an ESSource or ESProducer but there is no valid IOV for the synchronization value.
 Please check 
   a) if the synchronization value is reasonable and report to the hypernews if it is not.
   b) else check that all ESSources have been properly configured. 
----- End Fatal Exception -------------------------------------------------
Expand to see more relval errors ...

TTRHBuilderAngleAndTemplateWithoutProbQ = TTRHBuilderAngleAndTemplate.clone(ComponentName = 'WithAngleAndTemplateWithoutProbQ')

from Configuration.Eras.Modifier_trackingPhase2PU140_cff import trackingPhase2PU140
trackingPhase2PU140.toModify(TTRHBuilderAngleAndTemplate,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on the failures in tests being all in phase-2 wfs
I guess a trackingPhase2PU140.toModify(TTRHBuilderAngleAndTemplateWithoutProbQ, is missing here ... and the PixelCPEGeneric.toModify (despite of it kind of being obviously without ProbQ).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the PixelCPEGeneric.toModify (despite of it kind of being obviously without ProbQ)

The point of that modifier is to replace template reco with generic, when the former is not available (e.g. certain phase2 pixel sensor technologies) , so if TTRHBuilderAngleAndTemplateWithoutProbQ becomes the CPE of choice also for phase2, the modifier should replace that one too (as you correctly did in 2bde976)

@mmusich
Copy link
Contributor

mmusich commented Oct 27, 2022

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f4db58/28565/summary.html
COMMIT: 716680c
CMSSW: CMSSW_12_6_X_2022-10-27-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39776/28565/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: 6 differences found in the comparisons
  • Reco comparison had 6 failed jobs
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3384029
  • DQMHistoTests: Total failures: 1090
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3382917
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 201 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@slava77
Copy link
Contributor

slava77 commented Oct 27, 2022

@cmsbuild
Copy link
Contributor

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

@slava77
Copy link
Contributor

slava77 commented Oct 27, 2022

@rappoccio
we are back to a signed-off commit
please reconsider to merge , considering the first part of the comment #39776 (comment) to your earlier concern/suggestion

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f4db58/28582/summary.html
COMMIT: bd6dab7
CMSSW: CMSSW_12_6_X_2022-10-27-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39776/28582/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testSiStripHitEfficiency had ERRORS

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 11 differences found in the comparisons
  • Reco comparison had 6 failed jobs
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3384029
  • DQMHistoTests: Total failures: 96
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3383911
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 201 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@slava77
Copy link
Contributor

slava77 commented Oct 28, 2022

---> test testSiStripHitEfficiency had ERRORS

this is a file I/O issue

Failed to open the file 'root://cms-xrd-global.cern.ch//store/express/Run2018D/StreamExpress/ALCARECO/SiStripCalMinBias-Express-v1/000/325/172/00000/E3D6AECF-3F12-6540-97DC-4A6994CFEBF3.root'

DQMHistoTests: Total failures: 96

these seem to be false-positives: mainly in JetMET in phase 2 wfs

@mmusich
Copy link
Contributor

mmusich commented Oct 28, 2022

---> test testSiStripHitEfficiency had ERRORS

this is a file I/O issue

this is addressed at #39872

@rappoccio
Copy link
Contributor

+1

  • Should be safe to include this and the unit tests should also pass subsequently.

@slava77
Copy link
Contributor

slava77 commented Oct 28, 2022

+1

* Should be safe to include this and the unit tests should also pass subsequently.

it looks like +1 was not enough, need a "merge". I'm guessing it's due to the unrelated failed unit test

@perrotta
Copy link
Contributor

merge
(in the status of "test-rejected" an explicit merge is needed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants