Skip to content
Snippets Groups Projects

Update MuonSegmentRegionRecoveryTool to use new region selector tools

Merged Savanna Shaw requested to merge sshaw/athena:segmentrecovery-regionselector into master

Migrating the MuonSegmentRegionRecoveryTool to use the new region selector tools instead of the region selector service:

  • Extend the new JO configuration for the region selector tools to include the rest of the muon technologies
  • Update the segment region recovery tool to use the region selector tools instead of the old service
  • Update the configuration for the segment region recovery (both old and new style JOs)
  • We were always configuring the MuonSegmentRegionRecoveryTool due to the 'getPublicTool' call in MooreTools.py. This caused some issues with scheduling the region selector cond alg out of place in the legacy trigger tests. In the end this 'getPublicTool' call is not needed, since the tools are all configured in the relevent places, so have removed it.

Fixes ATLASRECTS-5377.

Edited by Savanna Shaw

Merge request reports

Pipeline #1730981 passed

Pipeline passed for 2dfeb3e7 on sshaw:segmentrecovery-regionselector

Approval is optional

Merged by Edward MoyseEdward Moyse 4 years ago (Jun 23, 2020 6:48pm UTC)

Merge details

  • Changes merged into with c93d3f74.
  • Deleted 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
  • This merge request affects 5 packages:

    • DetectorDescription/RegionSelector
    • MuonSpectrometer/MuonConfig
    • MuonSpectrometer/MuonReconstruction/MuonRecExample
    • MuonSpectrometer/MuonReconstruction/MuonTrackMakers/MuonTrackMakerTools/MuonTrackFinderTools
    • Reconstruction/MuonIdentification/MuonCombinedRecExample

    Adding @sutt ,@goetz ,@rosati ,@wleight ,@nkoehler ,@rbianchi as watchers

  • Hi Many thanks Savanna, would it be possible to use the naming convention "regSelTool_MDT_Cfg()" for instance, for more consistency with the other standard configuration ?

    Also for the Cfg() functions themselves, would it be possible to replace most of the internal code with a function, say _regSelToolCfg(), and then simply have the specific Cfg() functions call this ? This way it would reduce the duplication in the code and would be tidier in case we need to add additional functions for the other subsystems.

    Cheers Mark

  • Savanna Shaw added 1 commit

    added 1 commit

    • 93b46351 - Reduce duplicate code for RegSelTools config, and update names tool config function names

    Compare with previous version

  • Savanna Shaw changed the description

    changed the description

  • Author Developer

    Hi Mark,

    thanks for the suggestions! Both have been implemented.

    Cheers,

    Savanna

  • CI Result FAILURE (hash 0c5d20bf)

    Athena AthSimulation AnalysisBase AthGeneration
    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
    AnalysisBase: number of compilation errors 0, warnings 0
    AthGeneration: number of compilation errors 0, warnings 0
    📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 15779]

  • This merge request affects 5 packages:

    • DetectorDescription/RegionSelector
    • MuonSpectrometer/MuonConfig
    • MuonSpectrometer/MuonReconstruction/MuonRecExample
    • MuonSpectrometer/MuonReconstruction/MuonTrackMakers/MuonTrackMakerTools/MuonTrackFinderTools
    • Reconstruction/MuonIdentification/MuonCombinedRecExample

    Adding @sutt ,@goetz ,@rosati ,@wleight ,@nkoehler ,@rbianchi as watchers

  • Hi, the failed CI test appears to be simply because of a typo in the sTGC name for makeRegSelTool_sTGC ...

    ImportError: cannot import name makeRegSelTool_STGC

    since they they use tha lower case "s".

    Cheers Mark

  • Many thanks Savanna,

    Cheers Mark

  • Savanna Shaw added 1 commit

    added 1 commit

    • 2dfeb3e7 - fix typo when configuring sTGC region selector tool

    Compare with previous version

  • This merge request affects 5 packages:

    • DetectorDescription/RegionSelector
    • MuonSpectrometer/MuonConfig
    • MuonSpectrometer/MuonReconstruction/MuonRecExample
    • MuonSpectrometer/MuonReconstruction/MuonTrackMakers/MuonTrackMakerTools/MuonTrackFinderTools
    • Reconstruction/MuonIdentification/MuonCombinedRecExample

    Adding @sutt ,@goetz ,@rosati ,@wleight ,@nkoehler ,@rbianchi as watchers

  • CI Result FAILURE (hash 93b46351)

    Athena AthSimulation AnalysisBase AthGeneration
    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
    AnalysisBase: number of compilation errors 0, warnings 0
    AthGeneration: number of compilation errors 0, warnings 0
    📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 15793]

  • This is the second pipeline that has failed, with the code changes, but not the fix for CI test, which is in the pipeline that is still running so this latestround of CI tests should be ignored, and we wait for the third pipeline where the fix is included.

    Cheers Mark

  • CI Result SUCCESS (hash 2dfeb3e7)

    Athena AthSimulation AnalysisBase AthGeneration
    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
    AnalysisBase: number of compilation errors 0, warnings 0
    AthGeneration: number of compilation errors 0, warnings 0
    📝 For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 15795]

  • Mark Sutton approved this merge request

    approved this merge request

  • added urgent label

  • Hi, would someone be able to approve and merge this please ? There are other requests waiting to go on top of the changes from this one.

    Cheers Mark

  • Mark Sutton mentioned in merge request !33889 (merged)

    mentioned in merge request !33889 (merged)

  • Mark Sutton mentioned in merge request !33663 (merged)

    mentioned in merge request !33663 (merged)

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading