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
Merge request reports
Activity
This merge request affects 8 packages:
- Calorimeter/CaloTrackingGeometry
- InnerDetector/InDetDetDescr/InDetTrackingGeometry
- MuonSpectrometer/MuonDetDescr/MuonTrackingGeometry
- Tracking/TrkConditions/TrackingGeometryCondAlg
- Tracking/TrkConditions/TrkCondTest
- Tracking/TrkDetDescr/TrkDetDescrInterfaces
- Tracking/TrkDetDescr/TrkDetDescrTools
- Tracking/TrkDetDescr/TrkGeometry
Adding @goetz ,@amorley ,@wleight ,@pavol ,@sroe ,@nkoehler ,@rosati as watchers
CI Result FAILURE Athena AthSimulation AnalysisBase AthGeneration externals cmake N/A N/A N/A N/A make N/A N/A N/A N/A required tests N/A N/A N/A N/A optional tests N/A N/A N/A N/A Due to problems in externals build or cmake configuration the job is stopped, results are not available on the ATLAS CI monitor 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 13124] This merge request affects 8 packages:
- Calorimeter/CaloTrackingGeometry
- InnerDetector/InDetDetDescr/InDetTrackingGeometry
- MuonSpectrometer/MuonDetDescr/MuonTrackingGeometry
- Tracking/TrkConditions/TrackingGeometryCondAlg
- Tracking/TrkConditions/TrkCondTest
- Tracking/TrkDetDescr/TrkDetDescrInterfaces
- Tracking/TrkDetDescr/TrkDetDescrTools
- Tracking/TrkDetDescr/TrkGeometry
Adding @goetz ,@amorley ,@wleight ,@pavol ,@sroe ,@nkoehler ,@rosati as watchers
let me also add @stavrop who did the migration of
MuonReadoutGeometry
. When I did a quick grep forMuonDetectorManager
insideMuonTrackingGeometry
I think that the package is still using the nominal (no mis-alignment)MuonDetectorManager
and not the conditions objects (one per alignment change). Is this correct @stavrop? Was there a specific reason why the CondAlg is not used in this package?Thanks, Nico
CI Result FAILURE (hash e1442ceb) 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 13180] - Resolved by Christos Anastopoulos
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.cxxAgain 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- Resolved by Robert Johannes Langenberg
added review-pending-level-2 label and removed review-pending-level-1 label
- Resolved by Robert Johannes Langenberg
- Resolved by Robert Johannes Langenberg
- Resolved by Robert Johannes Langenberg