Skip to content
Snippets Groups Projects

re-working CaloClusterCorrectionCommon to retrieve CaloDetDescrManager from CondStore

All threads resolved!

An attempt to retrieve the CaloDetDesrcManager from the ConditionStore in CaloClusterCorrectionCommon. So far, this object was retreived in a somewhat hidden way and cached inside CaloClusterCorrectionCommon::DDHelper. The proposed way forward:

  • Add a ReadCondHandleKey<CaloDetDescrManager> to the base-class CaloClusterCorrection
  • Retrieve the CaloDetDescrManager inside CaloClusterCorrectionCommon::makeCorrection. Caveat: The retrieve with the implict EventContext-lookup happens more frequently than necessary and is potentially slow.
  • Pass a pointer to the CaloDetDescrManager through the call-chain of CaloClusterCorrectionCommon::DDHelper
  • The dummy DetDescrElements (for the 'missing' strip at eta=0) are still built when the DDHelper using whatever version of the CaloDetDescrManager is valid at this point in time (and put in a CachedUniquePtr) At least in theory, subsequent calls to the DDHelper may use a version of the CaloDetDescrManager with updated alignment. I believe in practice this is irrelevant, though it's not nice.

I'd like to hear @ssnyder's opinion ...

One unit-test (CaloScaleCluster_test.py) is still failing b/c its reference values come from an un-aligned geometry and I don't know how to obtain an unalinged CaloDetDescrManager instance with the new cond-algos. @tsulaia?

Edited by Walter Lampl

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Walter Lampl added 1 commit

    added 1 commit

    • 73cfdc7e - CaloClusterCorrectionCommon: retrieve with context

    Compare with previous version

  • Walter Lampl resolved all threads

    resolved all threads

  • Sounds ok to me.

    At some point, i guess it would be nice to fix ddman so that the hacks in DDHelper aren't needed --- but that's out of scope for this work.

    One unit-test (CaloScaleCluster_test.py) is still failing b/c its reference values come from an un-aligned geometry and I don't know how to obtain an unalinged CaloDetDescrManager instance with the new cond-algos. @tsulaia?

    That should probably be an option for CaloAlignCondAlg. Note that in today's nightly, this was failing:

    athena.py -c 'EvtMax=3' G4AtlasTests/test_AtlasG4_ParticleGun_electrons.py

    because G4TestAlg now wants to get dd from the cond store, but the CaloAlignCondAlg is disabled for simulation jobs. It should probably instead be configured to produce an unaligned dd.

  • Vakhtang Tsulaia mentioned in merge request !47033 (merged)

    mentioned in merge request !47033 (merged)

  • That should probably be an option for CaloAlignCondAlg.

    Please have a look at !47033 (merged) . If we pass empty keys to read condition handles of CaloAlignCondAlg, then it will build an unaligned Calo Det manager and write it into Cond Store with infinite validity range.

    After that it will still be required to fix ref for the CaloScaleCluster_test because the CaloAlignCondAlg will show up in the log, and few other printouts will be reordered, but hopefully that's OK.

    As for the failures in G4TestAlg, they have been fixed by switching LArHitsTestTool to reading CaloDetDescrManager from the Detector Store. See !46989 (merged). As a wrote in the description of that MR, in the long term, though, we need to revisit the CaloDetDescrManager usage patterns in Simulation in order to determine whether it is really needed to construct it by the converter at initialization.

  • Thanks @tsulaia. Once !47033 (merged) is accepted, I'll un-draft this one.

  • Walter Lampl added 167 commits

    added 167 commits

    • 73cfdc7e...ea9e410e - 165 commits from branch atlas:master
    • 2088b907 - Merge remote-tracking branch 'upstream/master' into CaloClusterCorrection_DetDescr_CondStore
    • 96af9ca1 - update reference file CaloScaleCluster_test.ref, since we have now more CondAlgs in the Log-file

    Compare with previous version

  • Walter Lampl marked this merge request as ready

    marked this merge request as ready

  • This merge request affects 1 package:

    • Calorimeter/CaloClusterCorrection

    This merge request affects 5 files:

    • Calorimeter/CaloClusterCorrection/CaloClusterCorrection/CaloClusterCorrection.h
    • Calorimeter/CaloClusterCorrection/CaloClusterCorrection/CaloClusterCorrectionCommon.h
    • Calorimeter/CaloClusterCorrection/share/CaloScaleCluster_test.ref
    • Calorimeter/CaloClusterCorrection/src/CaloClusterCorrection.cxx
    • Calorimeter/CaloClusterCorrection/src/CaloClusterCorrectionCommon.cxx

    Adding @pavol as watcher

  • :white_check_mark: CI Result SUCCESS (hash 96af9ca1)

    Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon
    externals :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    required tests :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    optional tests :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:

    Full details available on this CI monitor view
    :white_check_mark: Athena: number of compilation errors 0, warnings 0
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    :white_check_mark: AthGeneration: number of compilation errors 0, warnings 0
    :white_check_mark: AnalysisBase: number of compilation errors 0, warnings 0
    :white_check_mark: AthAnalysis: number of compilation errors 0, warnings 0
    :white_check_mark: DetCommon: number of compilation errors 0, warnings 0
    :pencil: For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 40081]

  • Walter Lampl mentioned in commit 1aba44fb

    mentioned in commit 1aba44fb

  • merged

  • Please register or sign in to reply
    Loading