[ATLIDTRKCP-188] Move ID tracking hole search to pattern recognition step
This MR implmenents the ability to run the hole search directly following the pattern recognition stage, avoiding the costly additional hole search run later during the reconstruction.
The changes and their motivation are documented in more detail at https://indico.cern.ch/event/968904/contributions/4080296/attachments/2131590/3589975/2020-10-28-IDSW_Holes_Updated.pdf).
In summary, this MR does the following:
- Introduce new outcome flags to the
IBoundaryCheckTool
. - Update the
InDetBoundaryCheckTool
's logic to work with these flags. - Adapt the hole search tool to understand the additional flags and treat them correctly
- Slightly tune the
SiTrajectoryElement_xk
, in order to allow for larger multiple scattering effects in the forward pixel region - Teach the
SiTrajectory_xk
to perform the required hole & dead module counting at the end of track finding. Note that during the track finding itelf, we keep using the old hole definition of the pattern, which is better suited to the early phase where uncertainties are still large. - Add an additional hole cut based on the resulting count to filter the tracks early
- Allow the combi track finder event data to store the results of the pattern hole search
- Teach the SiSpSeededTrackFinder to write the holes into the track summary
- Make the dense ambi not forget said track summary after a refit
- Add a flag in the InDetFlags to steer the entire procedure and use it in the config
The MR was updated following feedback received at IDSW on Monday this week.
The performance impact of this MR is non-trivial:
- We expect to significantly speed up track reconstruction, by around 15% compared to master. The main effect is on the ambiguity solver, which speeds up by a factor 2
- Fakes are increased, as is the number of total tracks. The fake rate is still well below the one we had with cut level 16, and 70% below release 21. The increase is smaller when only looking at 'TightPrimary' tracks
- The efficiency, in particular at low pt and high eta, is slightly improved.
Changes are under final validation before this MR can move forward.
Adding @sswatman @zschilla @npetters and @sroe + @christos. Also adding @elsing, @asalzbur, @gavrilen.
Merge request reports
Activity
- Resolved by Maximilian Emanuel Goblirsch-Kolb
added 8 commits
- b6178b71 - add a flag to use holes from the pattern
- 92a39b3b - add forward-copy of track summary in refit for DenseAmbiProcessor
- 7adb0310 - teach the SiTrajectory to do a hole count usable by the trackSummary
- 43ec33c4 - make the boundary status enum available within the Si trajectory element
- d93af66b - allow the combi track finder data to store a track to hole association map
- b9a2660d - make Si Sp seeded track finder use the track to hole association map
- 89dddd86 - make the combi track finder populate the track to hole map
- abc529ae - propagate config of holes-from-pattern to job config
Toggle commit listThis merge request affects 8 packages:
- InnerDetector/InDetExample/InDetRecExample
- InnerDetector/InDetRecAlgs/SiSPSeededTrackFinder
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData
- InnerDetector/InDetRecTools/InDetBoundaryCheckTool
- InnerDetector/InDetRecTools/InDetTrackHoleSearch
- InnerDetector/InDetRecTools/SiCombinatorialTrackFinderTool_xk
- Tracking/TrkTools/TrkAmbiguityProcessor
- Tracking/TrkTools/TrkToolInterfaces
Adding @goetz ,@amorley ,@oda ,@gavrilen ,@sroe ,@jsandesa as watchers
added InnerDetector Tracking master review-pending-level-1 labels
removed review-pending-level-1 label
❎ CI Result FAILURE (hash abc529ae)Athena AthSimulation AthGeneration AnalysisBase externals ✅ ✅ ✅ ✅ cmake ✅ ✅ ✅ ✅ make ⚠ ✅ ✅ ✅ required tests ⭕ ✅ ✅ ✅ optional tests ✅ ✅ ✅ ✅ Full details available on this CI monitor view
⚠ Athena: number of compilation errors 0, warnings 1
✅ AthSimulation: number of compilation errors 0, warnings 0
✅ AthGeneration: number of compilation errors 0, warnings 0
✅ AnalysisBase: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22373]added Run2-DataReco-output-changed label
This merge request affects 20 files:
- InnerDetector/InDetExample/InDetRecExample/python/InDetJobProperties.py
- InnerDetector/InDetExample/InDetRecExample/share/ConfiguredNewTrackingSiPattern.py
- InnerDetector/InDetExample/InDetRecExample/share/InDetRecLoadTools.py
- InnerDetector/InDetRecAlgs/SiSPSeededTrackFinder/SiSPSeededTrackFinder/SiSPSeededTrackFinder.h
- InnerDetector/InDetRecAlgs/SiSPSeededTrackFinder/share/SiSPSeededTracksStandalone.py
- InnerDetector/InDetRecAlgs/SiSPSeededTrackFinder/src/SiSPSeededTrackFinder.cxx
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/SiSPSeededTrackFinderData/SiCombinatorialTrackFinderData_xk.h
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/SiSPSeededTrackFinderData/SiTools_xk.h
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/SiSPSeededTrackFinderData/SiTrajectoryElement_xk.h
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/SiSPSeededTrackFinderData/SiTrajectoryElement_xk.icc
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/SiSPSeededTrackFinderData/SiTrajectory_xk.h
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/src/SiCombinatorialTrackFinderData_xk.cxx
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/src/SiTrajectoryElement_xk.cxx
- InnerDetector/InDetRecEvent/SiSPSeededTrackFinderData/src/SiTrajectory_xk.cxx
- InnerDetector/InDetRecTools/InDetBoundaryCheckTool/src/InDetBoundaryCheckTool.cxx
- InnerDetector/InDetRecTools/InDetTrackHoleSearch/src/InDetTrackHoleSearchTool.cxx
- InnerDetector/InDetRecTools/SiCombinatorialTrackFinderTool_xk/SiCombinatorialTrackFinderTool_xk/SiCombinatorialTrackFinder_xk.h
- InnerDetector/InDetRecTools/SiCombinatorialTrackFinderTool_xk/src/SiCombinatorialTrackFinder_xk.cxx
- Tracking/TrkTools/TrkAmbiguityProcessor/src/DenseEnvironmentsAmbiguityProcessorTool.cxx
- Tracking/TrkTools/TrkToolInterfaces/TrkToolInterfaces/IBoundaryCheckTool.h
Adding @goetz ,@amorley ,@oda ,@gavrilen ,@sroe ,@jsandesa as watchers
added review-pending-level-1 label
removed review-pending-level-1 label
- Resolved by Maximilian Emanuel Goblirsch-Kolb
@jojungge, I have the feeling something important is missing on this MR.
added 1 commit
- 8e21b927 - a commented line a day makes angry reviewers look away
- Resolved by Maximilian Emanuel Goblirsch-Kolb
❎ CI Result FAILURE (hash b31b49eb)Athena AthSimulation AthGeneration AnalysisBase externals ✅ ✅ ✅ ✅ cmake ✅ ✅ ✅ ✅ make ✅ ✅ ✅ ✅ required tests ⭕ ✅ ✅ ✅ optional tests ✅ ✅ ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
✅ AthGeneration: number of compilation errors 0, warnings 0
✅ AnalysisBase: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22610]
mentioned in merge request !37574 (closed)
added changes-trigger-counts label
added 307 commits
-
563de42e...084d7f36 - 301 commits from branch
atlas:master
- 4f86426c - Merge remote-tracking branch 'upstream/master' into implementHolesFromPattern
- 74362165 - Merge tag 'nightly/master/2020-10-28T2101' into implementHolesFromPattern
- b17b91fc - trigger ref up
- bb47afb2 - q221 up
- 4ed367bc - q431 up
- 18ca8d45 - Merge branch 'implementHolesFromPattern' of...
Toggle commit list-
563de42e...084d7f36 - 301 commits from branch
added Tools Trigger review-pending-level-1 labels
Un-WIPing. This can aleady be reviewed, but it will need one more update of the digest ref-files once !37704 (merged) has been merged, as both MRs update these.
Please only merge after !37704 (merged) to allow me to clean up the expected conflict.
Adding @amete to this for information, as we hopefully will see a change in SPOT once merged.
added urgent label and removed Tools Trigger review-pending-level-1 labels
mentioned in merge request !37704 (merged)
added Tools Trigger review-pending-level-1 labels
Hi @goblirsc do we expect changes in the reconstruction output? There are changes (a couple of %) in the trigger output where it should should run the existing code for the moment? cheers Jiri
- Resolved by Maximilian Emanuel Goblirsch-Kolb
Hi, yes, we expect a change to the trigger output, as a slightly more tolerant radiation length assumption is used within
SiTrajectoryElement_xk
to recover a bit of low-pt, forward efficiency. If you prefer not to have this, please let me know - it should be fairly easy to make this exclusive to the offline reco!
❎ CI Result FAILURE (hash 18ca8d45)Athena AthSimulation AthGeneration AnalysisBase externals ✅ ✅ ✅ ✅ cmake ✅ ✅ ✅ ✅ make ✅ ✅ ✅ ✅ required tests ⭕ ✅ ✅ ✅ optional tests ✅ ✅ ✅ ✅ Full details available on this CI monitor view
✅ Athena: number of compilation errors 0, warnings 0
✅ AthSimulation: number of compilation errors 0, warnings 0
✅ AthGeneration: number of compilation errors 0, warnings 0
✅ AnalysisBase: number of compilation errors 0, warnings 0
📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22629]added review-pending-level-2 label and removed review-pending-level-1 label
- Resolved by Tadej Novak
@goblirsc, I see no issues with the code so this could be merged as it is, but if you want to wait on the other MR, then fine.
Starting a thread so it's not merged by accident.
Tadej (L2)
added review-user-action-required label and removed review-pending-level-2 label
added review-approved label and removed review-user-action-required label
mentioned in commit ff469bab
added sweep:ignore label
added Egamma label
mentioned in merge request !37756 (merged)
mentioned in merge request !37772 (merged)
mentioned in merge request !37792 (merged)