New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of the decisions made at the Conditions mini-workshop #159

Merged
merged 16 commits into from Jun 8, 2017

Conversation

Projects
None yet
3 participants
@MarkusFrankATcernch
Contributor

MarkusFrankATcernch commented Jun 8, 2017

BEGINRELEASENOTES

Implementation of the decisions made at the Conditions mini-workshop

Access mechanisms of DD4hep conditions for utilities

Access to conditions is solely supported using the interface class DDCore/ConditionsMap.

  • All utilities must use this interface.
  • Any concrete implementation using conditions/alignment utilities must implement this interface
  • Basic implementation using STL map, multimap and unordered_map are provided.
  • A special no-op implementation of this interface shall be provided to access "default" alignment conditions. This implementation shall fall-back internally to the DetElement::nominal() alignment.
    Known clients: VolumeManager (hence: DDG4, DDRec, etc.)

Though this sounds like a trivial change, the consequences concern the entire conditions
and alignment handling. This interface decouples entirely the core part of DD4hep
from the conditions cache handling and the alignment handling.

Based on this interface most utilities used to handle conditions, detectors scans
to visit DetElement related condition sets, alignment and conditions printers etc.

For details, please see:

DDCore/include/DD4hep/AlignmentsPrinter.h
DDCore/include/DD4hep/AlignmentsProcessor.h
DDCore/include/DD4hep/ConditionsPrinter.h
DDCore/include/DD4hep/ConditionsProcessor.h
DDCore/include/DD4hep/DetectorProcessor.h

Naming conventions for detector conditions

  • Condition are logically attached to DetElements

    • Condition names are: DetElement.path()+"#"+condition-name
      Example: /world/LHCb/DownstreamRegion/Muon/M5/M5ASide/R3ASide/Cham046#alignment
  • Condition keys are a int64 compound of two int32:

   union {
     int64 key;
     struct {
       int32 item_key;
       int32 det_key;     // Needs to be the high word to have a properly ordered map
     } values;
   };
   det_key  = hash32(DetElement.path())
   item_key = hash32(condition-name)

Condition keys must be unique throughout the detector description.

  • Alignment conditions naming conventions:

    • Alignment-delta conditions are called alignment_delta.
    • Fully qualified alignment conditions are called alignment.
      DD4hep provided alignment utilities rely on this convention.
  • Other conditions can be named freely.

Important Notice

The Alignment conditions naming conventions are already used by several utilities involving alignments. If you plan to use these, do not freely ignore these recommendations. When the naming conventions are ignored, these utilities shall not work.

Updates to DDCond

DDCond implements a working conditions cache following the design criteria sketched above. The conditionsSlice object implements (though by forwarding to the ConditionsUserPool) a ConditionsMap interface.

The DD4hep_ConditionsMapUserPool plugin implements in a very efficient way this interface using an ordered map. Using the above described key definition, this implementation allows very efficient scans of conditions/alignments etc. of individual detector elements, since conditions which belong to the same detector element are contiguous.

Alignment handling/computations

Using the conditions maps, the computation of (mis-)alignment data from deltas
is no longer bound to the conditions mechanisms.

A special utility called AlignmentsCalculator is put in place (see DDCore/include/DD4hep/AlignmentsCalculator.h) to facilitate the computation of a coherent set of alignments given a set of delta-parameters. This mechanism is much simpler, easier to understand and far less code intensive than the previously designed callback mechanism where alignments are obtained using conditions derivation.

Update of the existing examples

The example sets in DDDB, examples/Conditions, examples/AlignDet, examples/DDDB were updated according to the changed mechanism of accessing conditions. Here we can see the real benefits of the new approach: keeping same functionality, the examples became way off simpler. Simply count the number of lines of code.

ENDRELEASENOTES

@petricm

This comment has been minimized.

Member

petricm commented Jun 8, 2017

Can you please fix this on GCC 7

[99/521] Building CXX object DDCore/CMakeFiles/DDCore.dir/src/ConditionsInterna.cpp.o
../DDCore/src/ConditionsInterna.cpp: In static member function 'static size_t DD4hep::Conditions::Interna::ConditionObject::offset()':
../DDCore/src/ConditionsInterna.cpp:50:72: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
   size_t off = ((char*)&o->data.grammar) - ((char*)o) + sizeof(o->data.grammar);

and this on mac

[272/521] Building CXX object DDG4/CMakeFiles/DDG4.dir/src/Geant4PhysicsConstructor.cpp.o
../DDG4/src/Geant4PhysicsConstructor.cpp:41:14: warning: instantiation of variable 'G4VUPLSplitter<G4VPCData>::offset' required here, but no definition is available [-Wundefined-var-template]
      iter = aParticleIterator;
             ^
/cvmfs/clicdp.cern.ch/software/Geant4/10.02.p02/x86_64-mac1012-clang80-opt/include/Geant4/G4VPhysicsConstructor.hh:119:48: note: expanded from macro 'aParticleIterator'
#define aParticleIterator ((subInstanceManager.offset[g4vpcInstanceID])._aParticleIterator)
                                               ^
/cvmfs/clicdp.cern.ch/software/Geant4/10.02.p02/x86_64-mac1012-clang80-opt/include/Geant4/G4VUPLSplitter.hh:195:38: note: forward declaration of template entity is here
    G4RUN_DLL G4ThreadLocalStatic T* offset; //Pointer to first instance of an array
                                     ^
../DDG4/src/Geant4PhysicsConstructor.cpp:41:14: note: add an explicit instantiation declaration to suppress this warning if 'G4VUPLSplitter<G4VPCData>::offset' is explicitly instantiated in another translation unit
      iter = aParticleIterator;
             ^
/cvmfs/clicdp.cern.ch/software/Geant4/10.02.p02/x86_64-mac1012-clang80-opt/include/Geant4/G4VPhysicsConstructor.hh:119:48: note: expanded from macro 'aParticleIterator'
#define aParticleIterator ((subInstanceManager.offset[g4vpcInstanceID])._aParticleIterator)
@MarkusFrankATcernch

This comment has been minimized.

Contributor

MarkusFrankATcernch commented Jun 8, 2017

-- mac errors cannot be fixed. This is in Geant4.
-- gcc errors on the next pull request. This one is too beig to be pending for long. The chance of conflicts is too large.

@andresailer

This comment has been minimized.

Member

andresailer commented Jun 8, 2017

Please don't use the "update branch" button, this makes an ugly "merge" commit

@MarkusFrankATcernch

This comment has been minimized.

Contributor

MarkusFrankATcernch commented Jun 8, 2017

I was forced to do so, because you did this commit before. How can I avoid it?

@andresailer

This comment has been minimized.

Member

andresailer commented Jun 8, 2017

"Rebase and merge" was still possible, no need to click the button at all.
But this merge commit should disappear with the rebase, at least it did in my test.

@MarkusFrankATcernch MarkusFrankATcernch merged commit c7d1807 into AIDASoft:master Jun 8, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment