New HLT tracking baseline as default#231
Conversation
|
run-ci: all |
|
The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.
The full set of validation and comparison plots can be found here. Here is a timing comparison: |
|
There was a problem while building and running with CMSSW. The logs can be found here. |
I have trouble understanding where the problem: comes from. I can see that the procModifier is there in the master (as it should) and has been deleted in this PR (as it should). |
|
The standalone plots are missing because didn't fully deploy the new CI yet because I'm figuring out what's the issue with standalone GPU. You can still use the old syntax for now. |
|
/run all |
|
Static checks with the "old" CI failed with: I would trust the "new" CI static checks. I ran the |
|
The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.
The full set of validation and comparison plots can be found here. Here is a timing comparison: |
|
There was a problem while building and running with CMSSW. The logs can be found here. |
@ariostas Any idea about this one? Things compiled normally locally, and it seems to me like I don't have an instance of |
|
Oh it's because the CI doesn't add the |
Oh, OK. Maybe not needed for now (not sure how complicated its implementation is) but useful for the long run. Thanks! |
Does it mean that if a PR changes some code, it will not be checked out by the CI unless that code is in a specific set of packages? Or is it a comment about dependencies? |
There was a problem hiding this comment.
I'm not sure CAExtension in hltPhase2PixelTracksCAExtension should remain in the default pre-selection track collection name.
The choice of having or not HighPurity on the pixel track collection is unclear, but perhaps there is a benefit for the downstream users to see it as hltPhase2PixelTracks (similar to hltGeneralTracks, which actually pass HP).
OTOH, perhaps I'm confused on the choice of inputs (like PV reco or scouting output: shouldn't they all consume hltPhase2PixelTracks).
These comments are beyond the LST topic review. So, I'd ask for feedback from the POG.
Configuration/ProcessModifiers/python/phase2LegacyTracking_cff.py
Outdated
Show resolved
Hide resolved
HLTrigger/Configuration/python/HLT_75e33/modules/hltPhase2PixelTracks_cfi.py
Show resolved
Hide resolved
No, it used to just check out a select list of packages. Now it should check out what's needed and you can also do it manually with |
|
run-ci: all |
|
GitHub actions is having issues (https://www.githubstatus.com/incidents/xwn6hjps36ty), so I'll test on the self-hosted runner. |
|
run-ci: [standalone, cmssw] |
|
There was a problem while building and running in standalone mode. The logs can be found here. |
|
The PR was built and ran successfully with CMSSW. Here are some plots. OOTB All Tracks
The full set of validation and comparison plots can be found here. |
|
As a heads-up, after discussing with @mmasciov, it was agreed to add also a legacy configuration but with Patatrack quads instead of the legacy ones. I will work on this today and then it should be good to go right after. |
|
run-ci: standalone |
|
The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.
The full set of validation and comparison plots can be found here. Here is a timing comparison: |
|
Sorry for the CI spam. I'm done now. (I'll fix the repeated timings and test somewhere else) |
The legacy configuration with Patatrack quads can be enabled by using the procModifier combination |
HLTrigger/Configuration/python/HLT_75e33/eventsetup/hltESPMkFit_cfi.py
Outdated
Show resolved
Hide resolved
Configuration/ProcessModifiers/python/hltPhase2LegacyTrackingPatatrackQuads_cff.py
Outdated
Show resolved
Hide resolved
5d07346 to
1cb2ecc
Compare
|
Thanks, @slava77, submitting to cms-sw! |
| @@ -0,0 +1,4 @@ | |||
| import FWCore.ParameterSet.Config as cms | |||
|
|
|||
| # This modifiers reverts the Tracking in the HLT Phase-2 to the Legacy algorithms from Run-2 | |||
There was a problem hiding this comment.
Note to self, if more changes are needed:
| # This modifiers reverts the Tracking in the HLT Phase-2 to the Legacy algorithms from Run-2 | |
| # This modifier reverts the Tracking in the HLT Phase-2 to the Legacy algorithms from Run-2 |
| @@ -0,0 +1,6 @@ | |||
| import FWCore.ParameterSet.Config as cms | |||
|
|
|||
| # This modifiers reverts the Tracking in the HLT Phase-2 to the Legacy algorithms from Run-2 | |||
There was a problem hiding this comment.
Note to self, if more changes are needed:
| # This modifiers reverts the Tracking in the HLT Phase-2 to the Legacy algorithms from Run-2 | |
| # This modifier reverts the Tracking in the HLT Phase-2 to the Legacy algorithms from Run-2 |
| _HLTTrackingSequenceLegacy = cms.Sequence(HLTItLocalRecoSequence+ | ||
| HLTOtLocalRecoSequence+ | ||
| hltTrackerClusterCheck+ | ||
| HLTPhase2PixelTracksAndVerticesSequence+ | ||
| HLTInitialStepSequence+ | ||
| HLTHighPtTripletStepSequence+ | ||
| hltGeneralTracks) |
There was a problem hiding this comment.
Note to self, fix indentation, if more changes are needed.
81b2882 to
e72f9c1
Compare
| HLTLocalTrackerSequence = cms.Sequence( | ||
| hltPhase2SiPixelClustersSoA | ||
| + hltPhase2SiPixelRecHitsSoA | ||
| + hltSiPhase2Clusters |
There was a problem hiding this comment.
cross-post from #235 (comment)
it would be more practical to split the offloadable sequences in the tracking sequence definition files and pick them up here. The approach with re-definition doesn't look well maintainable
There was a problem hiding this comment.
Do you think that this is an issue with this PR, or it pre-existed in the master:
https://github.com/cms-sw/cmssw/blob/master/HLTrigger/Configuration/python/HLT_75e33/paths/DST_HeterogeneousReco_cfi.py
If the latter is the case (to which I would agree), I would propose to leave it out of this PR and include it in a separate one, after having coordinated with Marco offline. It may require more thinking and redesigning to be accommodated as a side note in another PR, in my opinion.
92b5204 to
46ec72c
Compare
…tatrack + LST, mkFit) as Phase 2 HLT default Make phase2CAExtension the default for HLT tracking Make singleIterPatatrack the default for HLT tracking Make mkFit the default for HLT tracking Make LST seeding the default for HLT tracking Addition of legacy HLT tracking workflows
Co-authored-by: Marco Musich <marco.musich@cern.ch>
46ec72c to
247705b
Compare













As per title.
Validations:
hltPhase2LegacyTrackingprocModifier, equivalent to the previous legacy) = two iterations, legacy quad + triplet seeding, CKF building:https://uaf-10.t2.ucsd.edu/~evourlio/SDL/CMSSWPR231/oldBaseline_legacyPixels_noCAExt_noSingleIter_noMkFit_noLST/plots_hlt.html
hltPhase2LegacyTrackingPatatrackQuadsprocModifier, equivalent to the current default) = two iterations, legacy quad + triplet seeding, CKF building:https://uaf-10.t2.ucsd.edu/~evourlio/SDL/CMSSWPR231/oldBaseline_noCAExt_noSingleIter_noMkFit_noLST/plots_hlt.html
https://uaf-10.t2.ucsd.edu/~evourlio/SDL/CMSSWPR231/newBaseline_noCAExt_noSingleIter_noMkFit_noLST/plots_hlt.html
trackingLST= single iteration, CA extension, LST building:https://uaf-10.t2.ucsd.edu/~evourlio/SDL/CMSSWPR231/trackingLST_noCAExt_noSingleIter_noMkFit_noLST/plots_hlt.html
trackingMkFitFit= single iteration, CA extension + LST seeding, mkFit building, mkFit fitting:https://uaf-10.t2.ucsd.edu/~evourlio/SDL/CMSSWPR231/trackingMkFitFit_noCAExt_noSingleIter_noMkFit_noLST/plots_hlt.html
ngtScouting:https://uaf-10.t2.ucsd.edu/~evourlio/SDL/CMSSWPR231/ngtScouting_noCAExt_noSingleIter_noMkFit_noLST/plots_hlt.html
ngtScouting,trackingLST:https://uaf-10.t2.ucsd.edu/~evourlio/SDL/CMSSWPR231/ngtScouting_trackingLST_noCAExt_noSingleIter_noMkFit_noLST/plots_hlt.html
Small cleanup done:
hltTrackingMkFitInitialStepprocModifier