Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid sequence expansion in customizeHLTTrackingForPhaseI2017 #17406

Closed
wants to merge 1 commit into from
Closed

Avoid sequence expansion in customizeHLTTrackingForPhaseI2017 #17406

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions HLTrigger/Configuration/python/customizeHLTTrackingForPhaseI2017.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ def modifyHLTPhaseIPixelGeom(process):

# modify the HLT configuration to run the Phase I tracking in the particle flow sequence
def customizeHLTForPFTrackingPhaseI2017(process):
# Bit of a hack to replace a module/sequence with another without
# expanding the sequences containing it, but works
#
# In principle setattr(process) can be used to replace a
# module/sequence too, but it expands the sequences making life
# more complex
modifier = cms.Modifier()
Copy link
Contributor

Choose a reason for hiding this comment

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

HI @makortel - i really don't understand the role of modifier here. Its not the way they were designed to be used (but thats not to say we shouldn't use them this way - rather its hard to naively understand what happens..)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fully agree this is a hack. But this is the simplest way I'm aware of to replace a module/sequence in a cms.Process without causing an expansion of all sequences including the module/sequence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(ok, not so sure of "all", but at least some sequences get expanded)

Copy link
Contributor

Choose a reason for hiding this comment

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

i guess I'll need to understand more - but what is the lifetime of these customize functions rather than a contained menu?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davidlange6 The discussion in these two issues #15425 and #15792 is related to automatic sequence expansion.

modifier._setChosen()


process.hltPixelLayerTriplets.layerList = cms.vstring(
'BPix1+BPix2+BPix3',
Expand Down Expand Up @@ -115,7 +124,7 @@ def customizeHLTForPFTrackingPhaseI2017(process):

process.hltPixelTracks.SeedingHitSets = "hltPixelTracksHitQuadruplets"

process.HLTDoRecoPixelTracksSequence = cms.Sequence(
process.HLTDoRecoPixelTracksSequence = cms.Sequence( # this sequence doesn't exist, but why to add it?
process.hltPixelLayerQuadruplets +
process.hltPixelTracksTrackingRegions +
process.hltPixelTracksHitDoublets +
Expand All @@ -129,7 +138,7 @@ def customizeHLTForPFTrackingPhaseI2017(process):
process.hltIter0PFlowTrackCutClassifier.mva.min3DLayers = cms.vint32( 0, 3, 4 )
process.hltIter0PFlowTrackCutClassifier.mva.minPixelHits = cms.vint32( 0, 3, 4 )

process.hltIter1PixelLayerTriplets = cms.EDProducer( "SeedingLayersEDProducer",
modifier.toReplaceWith(process.hltIter1PixelLayerTriplets, cms.EDProducer( "SeedingLayersEDProducer",
layerList = cms.vstring(
'BPix1+BPix2+BPix3',
'BPix1+BPix2+FPix1_pos',
Expand Down Expand Up @@ -161,7 +170,7 @@ def customizeHLTForPFTrackingPhaseI2017(process):
hitErrorRPhi = cms.double( 0.0027 )
),
TIB = cms.PSet( )
)
))

process.HLTIter1PSetTrajectoryFilterIT = cms.PSet(
ComponentType = cms.string('CkfBaseTrajectoryFilter'),
Expand Down Expand Up @@ -226,7 +235,7 @@ def customizeHLTForPFTrackingPhaseI2017(process):
useSameTrajFilter = cms.bool(False) # new ! other iteration should have it set to True
)

process.HLTIterativeTrackingIteration1 = cms.Sequence( process.hltIter1ClustersRefRemoval + process.hltIter1MaskedMeasurementTrackerEvent + process.hltIter1PixelLayerTriplets + process.hltIter1PFlowPixelTrackingRegions + process.hltIter1PFlowPixelClusterCheck + process.hltIter1PFlowPixelHitDoublets + process.hltIter1PFlowPixelHitTriplets + process.hltIter1PFlowPixelSeeds + process.hltIter1PFlowCkfTrackCandidates + process.hltIter1PFlowCtfWithMaterialTracks + process.hltIter1PFlowTrackCutClassifierPrompt + process.hltIter1PFlowTrackCutClassifierDetached + process.hltIter1PFlowTrackCutClassifierMerged + process.hltIter1PFlowTrackSelectionHighPurity )
modifier.toReplaceWith(process.HLTIterativeTrackingIteration1, cms.Sequence( process.hltIter1ClustersRefRemoval + process.hltIter1MaskedMeasurementTrackerEvent + process.hltIter1PixelLayerTriplets + process.hltIter1PFlowPixelTrackingRegions + process.hltIter1PFlowPixelClusterCheck + process.hltIter1PFlowPixelHitDoublets + process.hltIter1PFlowPixelHitTriplets + process.hltIter1PFlowPixelSeeds + process.hltIter1PFlowCkfTrackCandidates + process.hltIter1PFlowCtfWithMaterialTracks + process.hltIter1PFlowTrackCutClassifierPrompt + process.hltIter1PFlowTrackCutClassifierDetached + process.hltIter1PFlowTrackCutClassifierMerged + process.hltIter1PFlowTrackSelectionHighPurity ))

process.hltIter2PixelLayerTriplets = cms.EDProducer( "SeedingLayersEDProducer",
layerList = cms.vstring(
Expand Down Expand Up @@ -286,10 +295,10 @@ def _copy(old, new, skip=[]):
if key not in skipSet:
setattr(new, key, getattr(old, key))
from RecoTracker.TkSeedGenerator.seedCreatorFromRegionConsecutiveHitsTripletOnlyEDProducer_cfi import seedCreatorFromRegionConsecutiveHitsTripletOnlyEDProducer as _seedCreatorFromRegionConsecutiveHitsTripletOnlyEDProducer
process.hltIter2PFlowPixelSeeds = _seedCreatorFromRegionConsecutiveHitsTripletOnlyEDProducer.clone(seedingHitSets="hltIter2PFlowPixelHitTriplets")
modifier.toReplaceWith(process.hltIter2PFlowPixelSeeds, _seedCreatorFromRegionConsecutiveHitsTripletOnlyEDProducer.clone(seedingHitSets="hltIter2PFlowPixelHitTriplets"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change alone would be sufficient to fix the @silviodonato's recipe, but I implemented/tested the wider solution first and it doesn't hurt (and IMHO makes edmConfigDumps easier to understand/correlate with configuration files/snippets as less sequences get expanded).

_copy(process.HLTSeedFromConsecutiveHitsTripletOnlyCreator, process.hltIter2PFlowPixelSeeds, skip=["ComponentName"])

process.HLTIterativeTrackingIteration2 = cms.Sequence( process.hltIter2ClustersRefRemoval + process.hltIter2MaskedMeasurementTrackerEvent + process.hltIter2PixelLayerTriplets + process.hltIter2PFlowPixelTrackingRegions + process.hltIter2PFlowPixelClusterCheck + process.hltIter2PFlowPixelHitDoublets + process.hltIter2PFlowPixelHitTriplets + process.hltIter2PFlowPixelSeeds + process.hltIter2PFlowCkfTrackCandidates + process.hltIter2PFlowCtfWithMaterialTracks + process.hltIter2PFlowTrackCutClassifier + process.hltIter2PFlowTrackSelectionHighPurity )
modifier.toReplaceWith(process.HLTIterativeTrackingIteration2, cms.Sequence( process.hltIter2ClustersRefRemoval + process.hltIter2MaskedMeasurementTrackerEvent + process.hltIter2PixelLayerTriplets + process.hltIter2PFlowPixelTrackingRegions + process.hltIter2PFlowPixelClusterCheck + process.hltIter2PFlowPixelHitDoublets + process.hltIter2PFlowPixelHitTriplets + process.hltIter2PFlowPixelSeeds + process.hltIter2PFlowCkfTrackCandidates + process.hltIter2PFlowCtfWithMaterialTracks + process.hltIter2PFlowTrackCutClassifier + process.hltIter2PFlowTrackSelectionHighPurity ))

# Need to operate on Paths as well...
for seqs in [process.sequences_(), process.paths_()]:
Expand Down