Skip to content
Snippets Groups Projects

[ATLIDTRKCP-188] Move ID tracking hole search to pattern recognition step

Merged [ATLIDTRKCP-188] Move ID tracking hole search to pattern recognition step
All threads resolved!
All threads resolved!

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.

Edited by Maximilian Emanuel Goblirsch-Kolb

Merge request reports

Pipeline #2050084 failed

Pipeline failed for 18ca8d45 on goblirsc:implementHolesFromPattern

Merged by Frank WinklmeierFrank Winklmeier 4 years ago (Oct 29, 2020 3:39pm UTC)

Merge details

  • Changes merged into with ff469bab.
  • Did not delete the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 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

    Compare with previous version

  • Maximilian Emanuel Goblirsch-Kolb changed the description

    changed the description

  • Maximilian Emanuel Goblirsch-Kolb changed title from WIP: More serious implementation of holes from pattern to WIP: [ATLIDTRKCP-188] More serious implementation of holes from pattern

    changed title from WIP: More serious implementation of holes from pattern to WIP: [ATLIDTRKCP-188] More serious implementation of holes from pattern

  • Will do one run of the CI to see if anything beyond the digest (expected) dies...

  • Maximilian Emanuel Goblirsch-Kolb unmarked as a Work In Progress

    unmarked as a Work In Progress

  • WARNING: big files (>100K) are found in the changeset

    📝 116K in file InnerDetector/InDetExample/InDetRecExample/python/InDetJobProperties.py

  • This 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

  • Maximilian Emanuel Goblirsch-Kolb marked as a Work In Progress

    marked as a Work In Progress

  • 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 2 commits

    • 8b4fe396 - fix compiler warning
    • 7c716410 - reorganise to use old intersection within pattern and switch to new logic...

    Compare with previous version

  • Maximilian Emanuel Goblirsch-Kolb changed the description

    changed the description

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • added 1 commit

    • b31b49eb - turn holes from pattern on by default

    Compare with previous version

  • Maximilian Emanuel Goblirsch-Kolb unmarked as a Work In Progress

    unmarked as a Work In Progress

  • Maximilian Emanuel Goblirsch-Kolb changed title from WIP: [ATLIDTRKCP-188] More serious implementation of holes from pattern to [ATLIDTRKCP-188] Move ID tracking hole search to pattern recognition step

    changed title from WIP: [ATLIDTRKCP-188] More serious implementation of holes from pattern to [ATLIDTRKCP-188] Move ID tracking hole search to pattern recognition step

  • Doing one more run of the CI overnight - hopefully the standalone CI test will now work, and we will only have the trigger and digest tests fail.

  • WARNING: big files (>100K) are found in the changeset

    📝 116K in file InnerDetector/InDetExample/InDetRecExample/python/InDetJobProperties.py

  • 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

  • Maximilian Emanuel Goblirsch-Kolb marked as a Work In Progress

    marked as a Work In Progress

  • Maximilian Emanuel Goblirsch-Kolb changed the description

    changed the description

  • added 1 commit

    • 8e21b927 - a commented line a day makes angry reviewers look away

    Compare with previous version

  • added 1 commit

    • 563de42e - auto is not well-liked in ATLAS

    Compare with previous version

  • mentioned in merge request !37574 (closed)

  • 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...

    Compare with previous version

  • Maximilian Emanuel Goblirsch-Kolb unmarked as a Work In Progress

    unmarked as a Work In Progress

  • WARNING: big files (>100K) are found in the changeset

    📝 116K in file InnerDetector/InDetExample/InDetRecExample/python/InDetJobProperties.py

    📝 216K in file Trigger/TrigValidation/TrigAnalysisTest/share/ref_RDOtoRDOTrig_v1Dev_build.ref

  • This merge request affects 23 files. Since this is a long list, I will not print it here.

    Adding @goetz ,@amorley ,@amete ,@okumura ,@oda ,@gavrilen ,@sroe ,@jsandesa ,@jpanduro as watchers

  • 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.

  • Also adding urgent label as this MR is required for the tracking feature freeze on Friday.

  • resolved all threads

  • mentioned in merge request !37704 (merged)

  • resolved all threads

  • 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 all threads

  • 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]

  • Overlay tests are failing for many MR's, investigation is ongoing. (L1)

  • Tadej Novak resolved all threads

    resolved all threads

  • mentioned in commit ff469bab

  • Mentioning @turra , @dboerner , @jmaurer .

    Effect on e/gamma I think is inside the expected envelope. Higher eff image

    and higher cpu

    image

    So bit more efficient and bit more fakes .

  • mentioned in merge request !37756 (merged)

  • mentioned in merge request !37772 (merged)

  • mentioned in merge request !37792 (merged)

  • Please register or sign in to reply
    Loading