Skip to content
Snippets Groups Projects

Tracking geometry conditions alg

This adds a TrackingGeometry Conditions Algorithm which can be used IN ADDITION to the TrackingGeometrySvc. It also contains a "Test Algorithm" which actually just shows how to access the new TG but doesn't do anything with it. It serves as an example how to migrate all clients of the TrackingGeometry to the conditions algorithm. Not all TG Builders (Muon, Calo) are constructed with conditions data, and therefore don't have an IOVRange. I didn't change how they are built.

If you want to look at the changes of copied files, it is easier to look at the files without the cond postfix here: rlangenb/athena!2 (diffs)

As some interfaces changed, I copied the classes used to build the TG into new files adding "Cond" to their names as postfix. These files should be a temporary addition, until everything is migrated from the TrackingGeometrySvc to the TrackingGeometryCondAlg.

I made some interfaces const and didn't copy the file where I thought it wouldn't do damage, the CI will tell me if I forgot to adapt an implementation.

The input file used in the test alg configuration has multiple IOVs but I'm not sure if the TG actually changes. The TG is invalidated on IOV change though and rebuilt. This particular test file is both on cvmfs and on my public afs, loading the file from afs was ~30min (!) faster than from cvmfs.

Changes for https://its.cern.ch/jira/browse/ATLASRECTS-4732

Edited by Robert Johannes Langenberg

Merge request reports

Merged by Adam Edward BartonAdam Edward Barton 4 years ago (Jun 19, 2020 2:41pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • I made one comment on what I meant (in the meeting) on if we want to avoid const_cast.

    Another is const Trk::GlueVolumesDescriptor& enclosedDetGlueVolumes = tvol->glueVolumesDescriptor(); which is in a blob (so will put this here) MuonTrackingGeometryBuilderCond.cxx

    Again the tvol is const so you go through the non safe version..

    const GlueVolumesDescriptor& glueVolumesDescriptor();
    const GlueVolumesDescriptor& glueVolumesDescriptor ATLAS_NOT_THREAD_SAFE() const;

    Again you might know that the volume is really not const . So you might know that the behaviour is well defined etc and you can even annotate things as such. And at the end it might not matter as you will recreate the geometry and no client actually should ever being going near to these setter.

    But I just do not get how patterns like

    const Trk::TrackingVolume* foo = new Trk::TrackingVolume ....

    foo->[something that is a setter, but we were hidding it because everything was mutable and now is const_cast but we do not want to say lets use the non-const overload ...] Fundamentally help with something ....

    And why we would want to keep pretending it is const and then const_cast/or keep internals mutable and try to reason on access patterns?

    Ideally the less we pretend things are const, only to modify them in the next few lines the more ATLAS_NOT_THREAD_SAFE methods we can remove and keep only the safe versions.

    True at the end will be some const_cast when we do the so called sign i.e even in Acts which is a much more cleaned version of Tracking Geomety are there https://github.com/acts-project/acts/blob/69659f90bacdec6446d89c7955bc0d9a5e64e7c2/Core/src/Geometry/Layer.cpp#L47

    Still I would imagine we do not want to make their usage more than it is needed or if we go this way we prb should be a bit more explicit on why is fine in places?

    If you are to change interfaces maybe there is a chance here to go towards this direction maybe not?

    Edited by Christos Anastopoulos
  • Since this MR adds a new algorithm, passing to L2 shifters. Large amount of files were copied and changed by the developer. memory is allocated dynamically at several places.

    L1 shifter

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