[NGT] Extension of CA Pixel Tracking to Phase 2 Outer Tracker barrel#48921
[NGT] Extension of CA Pixel Tracking to Phase 2 Outer Tracker barrel#48921cmsbuild merged 40 commits intocms-sw:masterfrom
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48921/46075
|
|
A new Pull Request was created by @Parsifal-2045 for master. It involves the following packages:
@AdrianoDee, @Dr15Jones, @Martin-Grunewald, @Moanwar, @antoniovagnerini, @bsunanda, @civanch, @cmsbuild, @ctarricone, @davidlange6, @DickyChant, @fabiocos, @ftenchini, @fwyzard, @gabrielmscampos, @jfernan2, @kpedro88, @makortel, @mandrenguyen, @mdhildreth, @miquork, @mmusich, @nothingface0, @rseidita, @srimanob, @subirsarkar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
type ngt |
|
test parameters:
|
|
allow @Parsifal-2045 test rights |
|
@cmsbuild, please test |
Parsifal-2045
changed the title
|
Pull request #48921 was updated. @AdrianoDee, @DickyChant, @Dr15Jones, @Martin-Grunewald, @Moanwar, @antoniovagnerini, @bsunanda, @civanch, @cmsbuild, @ctarricone, @davidlange6, @elusian, @fabiocos, @ftenchini, @gabrielmscampos, @jfernan2, @kpedro88, @makortel, @mandrenguyen, @mdhildreth, @miquork, @mmasciov, @mmusich, @nothingface0, @rseidita, @srimanob, @subirsarkar can you please check and sign again. |
After a little misunderstanding with the bot, the history should now be clean :) |
|
@cmsbuild, please test |
|
Thanks, indeed and explicit comparison shows no differences. |
|
+1 |
|
+1 Size: This PR adds an extra 220KB to repository Comparison SummarySummary:
AMD_MI300X Comparison SummarySummary:
AMD_W7900 Comparison SummarySummary:
NVIDIA_H100 Comparison SummarySummary:
NVIDIA_L40S Comparison SummarySummary:
NVIDIA_T4 Comparison SummarySummary:
|
|
@cms-sw/db-l2 @cms-sw/geometry-l2 @cms-sw/pdmv-l2 @cms-sw/tracking-pog-l2 @cms-sw/upgrade-l2 it would be great to get this into |
| [ 28, 29, False, 1100, -1200, 1200, -10000, 10000, 10000, -50.0, 50.0, 0.85], | ||
| # [ 28, 30, False, 2000, -40, 40, -10000, 10000, 10000, -10000, 10000, 0.85], | ||
| [ 29, 30, False, 1250, -1200, 1200, -10000, 10000, 10000, -40.0, 40.0, 0.85], | ||
| ] |
There was a problem hiding this comment.
Couldn’t all these numbers just be stored in a CSV, JSON, or XML file instead?
There was a problem hiding this comment.
For better or worse, CMS decided to use python for its configurations in 2008, so we use python - not CSV, JSON or XML.
There was a problem hiding this comment.
On a more concrete note: not using python would mean increased code complexity (you'd need to parse JSON etc) and reduced configurability (you couldn't change the parameters in a configuration fragment, customisation function, or Era-based modifier).
There was a problem hiding this comment.
Haha, true — CMS tradition stands strong! I just thought of giving those poor constants a short vacation outside the Python file, maybe in a lighter format, but still within the same configuration flow. Anyway, it’s fine by me overall.
There was a problem hiding this comment.
I just thought of giving those poor constants a short vacation outside the Python file, maybe in a lighter format, but still within the same configuration flow.
This is beyond the point.
In current HLT development model, these configuration parameters can be changed within a same family of HLT menus starting from the same release template. If we start storing this outside of python we're going to need a change of release each time we want to change these (I would say this is not optimal at best).
There was a problem hiding this comment.
Thanks for the notes and the nice explanation. I was just thinking out loud, but apparently it wasn’t optimal at all.
There was a problem hiding this comment.
On a general level (not suggesting anything for this particular case) I just want to point out that large ParameterSets have a cost in storage because these are stored in the output files as part of the provenance. As usual, what exactly "large" means can be debated, and while a few "large" PSets can be accommodated, "too many" "large" PSets would become costly.
| else: | ||
| setattr(geometry, "phiCuts", cms.vint32( | ||
| 522, 730, 730, 522, 626, 626, 522, 522, 626, 626, 626, 522, 522, 522, 522, 522, 522, 522, 522)) | ||
|
|
There was a problem hiding this comment.
Is this really a DeltaPhi cut? Isn’t the number a bit too large for such a cut?
There was a problem hiding this comment.
This is because the phi stored in the hit SoA is actually a translation from float to short via these functions..
| delattr(geometry, "maxR") | ||
| else: | ||
| maxR = cms.vdouble(20, 9, 9, 20, 7, 7, 5, 5, 20, 6, 6, 5, 5, 20, 20, 9, 9, 9, 9) | ||
| if not hasattr(geometry, "maxDR"): |
There was a problem hiding this comment.
The same applies here; the numbers seem too large for a DeltaR value, unless it represents something different in this context.
There was a problem hiding this comment.
Yes, this is a real deltaR. The magnitude is driven by the gaps between the layers (and the fact that the doublets are "tilted" in RZ). It might be slightly high for some pairs (?), but these cuts have proven to be efficient.
There was a problem hiding this comment.
I think the confusion is between deltaR as in radial coordinate (in cm, as used here) vs deltaR as in phi/pseudorapidity cones (like in jets or particle associators).
| // Layers params | ||
| const std::vector<double> caThetaCuts_; | ||
| const std::vector<double> caDCACuts_; | ||
| const std::vector<int> isBarrel_; |
There was a problem hiding this comment.
Defined but not used? Or maybe I just missed it.
There was a problem hiding this comment.
I think this is a valid comment. Do you think we could do that as a follow-up clean-up (while we also provide the workflows requested by Andrea)?
|
+pdmv |
|
+Upgrade |
|
+dqm |
|
+1 |
|
+hlt |
|
+1 |
|
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. @ftenchini, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
|
🎉 |
|
This PR is a suspect for assertion failures in IBs, see #49266 |
20 participants
PR description:
This PR was co-developed by: @AdrianoDee @JanGerritSchulz @mmusich @rovere
The Phase 2 CA Extension for
PixelTracksenables the usage of recHits found in the first 3 layers of the Outer Tracker barrel in addition to the pixel recHits in the CA algorithm. As part of this work, the regular CA has also been updated and its parameters re-tuned for Phase 2. In particular, two more connections have been added in the very forward region bringing the total number of layer pairs to 57; a few more cuts have also been added and others have been re-tuned. The newly optimised, non-extended CA is proposed as the new default pixel tracking configuration, producing tracks with 4+ hits.The CA extension provides a more radical improvement, bringing the total number of layer pairs to 71, taking advantage of 3 more layers in the barrel, while still producing tracks with 4+ hits. It is not enabled by default, but can be tested using in workflows
*.7511,*.771,*.774, and*.775, which are also part of the IB matrix or by enabling thephase2CAExtensionprocModifier.Full talks with discussions on physics performance as well as the overall impact on general tracks have been recently given at the HLT Upgrade and Tracking POG. These slides also contain links to a large set of plots and comparisons.
Main developments
To support the extension, two new plugins have been developed:
Phase2OTRecHitsSoAConverterconverts therecHitson the P modules of the Outer Tracker barrel detector into SoA format using the same layout and following the same ordering logic as what is already in use in the pixelSiPixelRecHitExtendedAlpakamerges the existingtrackingRecHitsSoACollectionfor pixel hits with the newly converted one so that each column is extended with the hits from the OT, meaning that the CA interface with the input collection does not need to be changedOther modules have been modified to support the extension, including:
PixelTrackProducerFromSoAAlpaka, responsible for converting thePixelTracksin SoA format to legacy. It can now be configured to include the OT recHits in the legacy tracks or discard themTrackerTraits,phase2OT, has been introduce to be able to differentiate parameters, classes and functions, between regular and extended CAtrackingLSTprocModifier, the OTrecHitsare removed fromPixelTracksfed in input toLSTas to avoid a large increase in duplicatesinitialStephigh-purity selection is applied to extendedPixelTracksto further reduce their fake rate. Note that this is a temporary solution until a proper DNN will be applied directly to thePixelTracksin SoA format in a follow-up PR.Both the regular and extended CA have been optimised and new cuts have been introduced to improve fake rejection such as:
inner/outermax/minz(r) for layer pairs in the barrel (endcap)chi2cut based on the number of hits of the trackThe meaning of the
ngtScoutingprocModifier has been changed as follows:ngtScouting, promotes PixelTracks as general tracks directly without any other pattern recognition or fitting stepngtScouting,phase2CAExtensionusesextendedPixelTracks as general tracksngtScouting,phase2CAExtension,trackingLSTmerges extendedPixelTrackswithLSTT5s and uses the resulting collection as general tracksA few workflows have been added to test the extension and have been included in the IB matrix
*.7511: HLT phase-2 timing menu, with PixelTracks CA Extension*.774: HLT phase-2 NGT Scouting menu Alpaka variant, with PixelTracks CA Extension as GeneralTracks*.775: HLT phase-2 NGT Scouting menu Alpaka variant, with Pixeltracks CA Extension + LST T5s as GeneralTracksPhysics performance
Run 3
Since we expect no changes in Run 3 performance, we verified that the Run 3 CA performs exactly the same using the following recipe:
and comparing the DQM outputs.
Phase 2
Performance measured on a TTbar D110 PU Run4 RelVal sample (EDM input):
A detailed report on physics performance can be found in the talks linked above, here a few results are reported for completeness. Firstly, a comparison between legacy PixelTracks, new alpaka-based default, and extended CA (all plots
The impact on general tracks is more nuanced, here we only show a few configurations which were highlighted during the talks linked above and in the following discussions (all plots). Note, that the first 3 configurations run 2 tracking iterations (
initialStep+highPtTriplets), while the last configurationsingleIterCAExtensionLSTruns a single tracking iteration whereLSTis seeded by extendedPixelTracksonly.Timing
Measurements run on 2x AMD EPYC 9534 with 4x L40 GPUs. Configuration: 16 jobs with 16 threads/16 streams each, sample of 1000 TTbar 200PU events.
We have tested about 20 different configurations: the most interesting comparisons can be found in the talks linked above while all the results are here. We only show a comparison between legacy, the proposed new default and the promising
singleIterCAExtLSTconfiguration for the recordssingleIterCAExtLSTMemory usage
GPU memory usage have been measured while running the timing measurements using the output from
nvidia-smi, thus the setup is exactly the same.The non-extend optimised CA proposed as the new baseline shows a memory consumption comparable with what was already in release after #47611. The extension, including more layer pair connections and doubling the maximum number of doublets and tracks produced shows a more noticeable memory increase of about 34%
PR validation:
This PR has been tested locally running both
addOnTests.pyandrunTheMatrix.py -l limited -i all --ibeos, both commands resulting in all tests passing. The newly introduced workflows have been tested with:Update 25/09
We have updated the track selection settings for the default Pixel Tracking in Phase-2 (with Patatrack, without extension) by changing the$\chi^2$ / ndof requirement (depends on number of hits per track). It was previously set identical to the extended Pixel Tracking, which led to efficiency losses in the barrel due to shorter tracks. The change in efficiency for the new default before and after the commit is shown below. More validation plots can be found here.
The small efficiency losses at$|\eta|$ > 4 are due to an added requirement for quadruplets to not skip layers. This part of the track selection is planned to be replaced soon by a DNN to improve the track selection further.