-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
add vertex trimming to the phase2 hlt tracking iterations #47113
base: master
Are you sure you want to change the base?
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47113/43320 |
A new Pull Request was created by @lguzzi for master. It involves the following packages:
@Martin-Grunewald, @antoniovilela, @cmsbuild, @davidlange6, @fabiocos, @mandrenguyen, @mmusich, @rappoccio can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
||
# This modifier is used to enable vertex trimming in the tracking sequences | ||
# and in all related reconstruction modules | ||
vertexTrimming = cms.Modifier() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can it be renamed to signal it's about the phase-2 HLT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed it to phase2_hlt_vertexTrimming in c05a4f2
scalingStartNPix = cms.double(0.0), | ||
scalingEndNPix = cms.double(1.0), | ||
), | ||
mightGet = cms.optional.untracked.vstring, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mightGet = cms.optional.untracked.vstring, |
unneeded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done d17f0fb
useFakeVertices = cms.bool(False), | ||
maxNVertices = cms.int32(-1), | ||
nSigmaZ = cms.double(4.0), | ||
pixelClustersForScaling = cms.InputTag('siPixelClusters'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an offline product?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I copied it from the defaults. It is not used in this configuration, however I changed it to hltSiPixelClusters in d17f0fb
test parameters:
|
@lguzzi please add workflows that can exercise the trimming. cmssw/Configuration/PyReleaseValidation/python/upgradeWorkflowComponents.py Lines 1647 to 1685 in 547258c
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47113/43321 |
Pull request #47113 was updated. @Martin-Grunewald, @antoniovilela, @cmsbuild, @davidlange6, @fabiocos, @mandrenguyen, @mmusich, @rappoccio can you please check and sign again. |
# This modifier is used to enable vertex trimming in the tracking sequences | ||
# and in all related reconstruction modules | ||
phase2_hlt_vertexTrimming = cms.Modifier() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line is uneeded @lguzzi. Anyway please rebase to have just a single commit (also add the requested workflows).
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47113/43322 |
Pull request #47113 was updated. @Martin-Grunewald, @antoniovilela, @cmsbuild, @davidlange6, @fabiocos, @mandrenguyen, @mmusich, @rappoccio can you please check and sign again. |
d17f0fb
to
64612ec
Compare
I left out the part written by JME because I cannot validate it. If you think it is appropriate I can add the merged track collection producer, then JME can add what is left for puppi in a separate PR. |
It will be a nice topic for discussion at the next HLT Upgrade meeting! |
+1 Size: This PR adds an extra 68KB to repository
Comparison SummarySummary:
|
@rovere @VourMa I gather we should put this on hold until it's clarified what's the way forward at the first upgrade meeting? |
FYI @cms-sw/tracking-pog-l2 |
In general, for @cms-sw/tracking-pog-l2 it would be easier to indeed have the |
5511128
to
1eee0c3
Compare
thank you, I moved the tracking region definition to HLTHighPtTripletStepSeedingSequence and added some LST workflows. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47113/43331
|
Pull request #47113 was updated. @AdrianoDee, @Martin-Grunewald, @Moanwar, @antoniovilela, @cmsbuild, @davidlange6, @DickyChant, @fabiocos, @mandrenguyen, @miquork, @mmusich, @rappoccio, @srimanob, @subirsarkar can you please check and sign again. |
1eee0c3
to
fd4a8d9
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47113/43332
|
Pull request #47113 was updated. @AdrianoDee, @Martin-Grunewald, @Moanwar, @antoniovilela, @cmsbuild, @davidlange6, @DickyChant, @fabiocos, @mandrenguyen, @miquork, @mmusich, @rappoccio, @srimanob, @subirsarkar can you please check and sign again. |
PR description:
This PR adds vertex trimming to the phase2 hlt menu with a proc. modifier. This modifier allows to select and build only initial step seeds and high-pt triplet step doublets compatible with a subset of pixel vertices.
Preliminary studies of this configuration are available here for the legacy iterations.
PR validation:
This PR has been validated comparing the results of the customisation function used so far with the proc. modifier approach. Comparison plots show no difference.
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
not needed