Clean up QuickTrackAssociatorByHits configuration #22983
Conversation
TrackerHitAssociation is off by default (except for FastSim), so there is no need to customize it. Actually with the current implementation of QuickTrackAssociatorByHitsProducer it is impossible to use Phase2OT hits.
…es() only if needed
…reduce confusion The TrackerHitAssociator mode is used only for fastsim.
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22983/4369 |
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages: SimTracker/TrackAssociatorProducers @civanch, @vazzolini, @kmaeshima, @mdhildreth, @dmitrijus, @cmsbuild, @jfernan2, @vanbesien can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild, please test |
|
The tests are being triggered in jenkins. |
| phase2_tracker.toModify( | ||
| quickTrackAssociatorByHits, | ||
| pixelSimLinkSrc = cms.InputTag("simSiPixelDigis","Pixel"), | ||
| stripSimLinkSrc = cms.InputTag("simSiPixelDigis","Tracker") |
There was a problem hiding this comment.
The starting point for these developments was this customization, which is
- incorrect: for phase 2, instead of
stripSimLinkSrcone should set
usePhase2Tracker = cms.bool(True),
phase2TrackerSimLinkSrc = cms.InputTag("simSiPixelDigis","Tracker")- useless: These parameters are for
TrackerHitAssociator, while by defaultQuickTrackAssociatorByHitsusesClusterTPAssociation
So this PR makes an attempt to remove the customization of useless and possibly confusing parameters.
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1 |
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
makortel
changed the title
|
Removed "RFC" from the title following the approval of this cleanup. |
|
+1 |
This PR suggests the following modifications to the configuration of
QuickTrackAssociatorByHits(making the use ofTrackerHitAssociatoreven more specific to FastSim, and apparently for HI as well)only ifuseClusterTPAssociationisFalse`TrackerHitAssociatorfrom the default configuration, and add them only for FastSimThe motivation is to
QuickTrackAssociatorByHitsconfiguration, especially for "are the DigiSimLinks read by the module or not"Tested in CMSSW_10_2_X_2018-04-12-2300, no changes expected.
@VinInn @mtosi @jalimena @hajohajo