From 6f1ec6edc3de8d43c3c6fe5d1183fdae979a3136 Mon Sep 17 00:00:00 2001 From: Markus Frank <Markus.Frank@cern.ch> Date: Wed, 4 Apr 2018 22:10:20 +0200 Subject: [PATCH] Fix bug in DDDB conditions loading. Add Gaudi-like conditions service to DDDB --- DDCond/include/DDCond/ConditionsCleanup.h | 16 ++++--- DDCond/include/DDCond/ConditionsIOVPool.h | 4 ++ DDCond/include/DDCond/ConditionsManager.h | 4 ++ .../include/DDCond/ConditionsManagerObject.h | 3 ++ DDCond/include/DDCond/Type1/Manager_Type1.h | 4 ++ DDCond/src/ConditionsCleanup.cpp | 27 ++++++++++-- DDCond/src/ConditionsIOVPool.cpp | 21 +++++++++- DDCond/src/ConditionsManager.cpp | 5 +++ DDCond/src/Type1/Manager_Type1.cpp | 14 +++++++ DDCore/src/AlignmentsCalculator.cpp | 2 - DDDB/include/Detector/IDetService.h | 12 +++--- DDDB/src/plugins/DDDBDerivedCondTest.cpp | 42 ++++++++++++------- DDDB/src/plugins/DeVeloServiceTest.cpp | 3 +- DDDB/src/plugins/DeVeloTest.cpp | 5 ++- DDDB/src/plugins/DetService.cpp | 2 +- DDDB/src/plugins/DetService.h | 3 +- examples/DDDB/CMakeLists.txt | 2 +- 17 files changed, 129 insertions(+), 40 deletions(-) diff --git a/DDCond/include/DDCond/ConditionsCleanup.h b/DDCond/include/DDCond/ConditionsCleanup.h index b3439319a..38b9b52cb 100644 --- a/DDCond/include/DDCond/ConditionsCleanup.h +++ b/DDCond/include/DDCond/ConditionsCleanup.h @@ -14,7 +14,7 @@ #define DDCOND_CONDITIONSCLEANUP_H // Framework include files -#include "DDCond/ConditionsManager.h" +#include "DDCond/ConditionsPool.h" /// Namespace for the AIDA detector description toolkit namespace dd4hep { @@ -39,8 +39,10 @@ namespace dd4hep { virtual ~ConditionsCleanup() = default; /// Assignment operator ConditionsCleanup& operator=(const ConditionsCleanup& c) = default; - /// Cleanup operation - virtual void operator()(ConditionsManager mgr) const = 0; + /// Request cleanup operation of IOV POOL + virtual bool operator()(const ConditionsIOVPool& iov_pool) const; + /// Request cleanup operation of regular conditiions pool + virtual bool operator()(const ConditionsPool& pool) const; }; /// Base class to handle conditions cleanups @@ -60,10 +62,12 @@ namespace dd4hep { virtual ~ConditionsFullCleanup() = default; /// Assignment operator ConditionsFullCleanup& operator=(const ConditionsFullCleanup& c) = default; - /// Cleanup operation - virtual void operator()(ConditionsManager mgr) const override; + /// Request cleanup operation of IOV POOL + virtual bool operator()(const ConditionsIOVPool& iov_pool) const override; + /// Request cleanup operation of regular conditiions pool + virtual bool operator()(const ConditionsPool& pool) const override; }; - } /* End namespace cond */ + } /* End namespace cond */ } /* End namespace dd4hep */ #endif /* DDCOND_CONDITIONSCLEANUP_H */ diff --git a/DDCond/include/DDCond/ConditionsIOVPool.h b/DDCond/include/DDCond/ConditionsIOVPool.h index 46e033758..4c0d49158 100644 --- a/DDCond/include/DDCond/ConditionsIOVPool.h +++ b/DDCond/include/DDCond/ConditionsIOVPool.h @@ -66,6 +66,10 @@ namespace dd4hep { /// Remove all key based pools with an age beyon the minimum age. /** @return Number of conditions cleaned up and removed. */ int clean(int max_age); + + /// Invoke cache cleanup with user defined policy + /** @return pair<Number of pools cleared, Number of conditions cleaned up and removed> */ + int clean(const ConditionsCleanup& cleaner); }; } /* End namespace cond */ diff --git a/DDCond/include/DDCond/ConditionsManager.h b/DDCond/include/DDCond/ConditionsManager.h index 7c57dc1b0..899dd8156 100644 --- a/DDCond/include/DDCond/ConditionsManager.h +++ b/DDCond/include/DDCond/ConditionsManager.h @@ -31,6 +31,7 @@ namespace dd4hep { class UserPool; class ConditionsPool; class ConditionsSlice; + class ConditionsCleanup; class ConditionsIOVPool; class ConditionsDataLoader; class ConditionsManagerObject; @@ -156,6 +157,9 @@ namespace dd4hep { */ void pushUpdates() const; + /// Invoke cache cleanup with user defined policy + void clean(const ConditionsCleanup& cleaner) const; + /// Clean conditions, which are above the age limit. void clean(const IOVType* typ, int max_age) const; diff --git a/DDCond/include/DDCond/ConditionsManagerObject.h b/DDCond/include/DDCond/ConditionsManagerObject.h index 769d0e067..1a7a74352 100644 --- a/DDCond/include/DDCond/ConditionsManagerObject.h +++ b/DDCond/include/DDCond/ConditionsManagerObject.h @@ -168,6 +168,9 @@ namespace dd4hep { /** @return Number of conditions cleaned/removed from the IOV pool of the given type */ virtual int clean(const IOVType* typ, int max_age) = 0; + /// Invoke cache cleanup with user defined policy + virtual std::pair<int,int> clean(const ConditionsCleanup& cleaner) = 0; + /// Full cleanup of all managed conditions. /** @return pair<Number of pools cleared, Number of conditions cleaned up and removed> */ virtual std::pair<int,int> clear() = 0; diff --git a/DDCond/include/DDCond/Type1/Manager_Type1.h b/DDCond/include/DDCond/Type1/Manager_Type1.h index 015f72b4d..75c5cbe21 100644 --- a/DDCond/include/DDCond/Type1/Manager_Type1.h +++ b/DDCond/include/DDCond/Type1/Manager_Type1.h @@ -141,6 +141,10 @@ namespace dd4hep { /** @return Number of conditions cleaned/removed from the IOV pool of the given type */ int clean(const IOVType* typ, int max_age) final; + /// Invoke cache cleanup with user defined policy + /** @return pair<Number of pools cleared, Number of conditions cleaned up and removed> */ + std::pair<int,int> clean(const ConditionsCleanup& cleaner) final; + /// Full cleanup of all managed conditions. /** @return pair<Number of pools cleared, Number of conditions cleaned up and removed> */ std::pair<int,int> clear() final; diff --git a/DDCond/src/ConditionsCleanup.cpp b/DDCond/src/ConditionsCleanup.cpp index 5deffdec7..76c3d9349 100644 --- a/DDCond/src/ConditionsCleanup.cpp +++ b/DDCond/src/ConditionsCleanup.cpp @@ -1,5 +1,5 @@ //========================================================================== -// AIDA Detector description implementation +// AIDA Detector description implementation //-------------------------------------------------------------------------- // Copyright (C) Organisation europeenne pour la Recherche nucleaire (CERN) // All rights reserved. @@ -16,7 +16,26 @@ using namespace dd4hep::cond; -/// Cleanup operation -void ConditionsFullCleanup::operator()(ConditionsManager mgr) const { - mgr.clear(); +/// Request cleanup operation of IOV POOL +bool ConditionsCleanup::operator()(const ConditionsIOVPool & /* iov_pool */) const +{ + return false; +} + +/// Request cleanup operation of regular conditiions pool +bool ConditionsCleanup::operator()(const ConditionsPool & /* iov_pool */) const +{ + return true; +} + +/// Request cleanup operation of IOV POOL +bool ConditionsFullCleanup::operator()(const ConditionsIOVPool & /* iov_pool */) const +{ + return true; +} + +/// Request cleanup operation of regular conditiions pool +bool ConditionsFullCleanup::operator()(const ConditionsPool & /* iov_pool */) const +{ + return true; } diff --git a/DDCond/src/ConditionsIOVPool.cpp b/DDCond/src/ConditionsIOVPool.cpp index 890a57eae..09f36f196 100644 --- a/DDCond/src/ConditionsIOVPool.cpp +++ b/DDCond/src/ConditionsIOVPool.cpp @@ -15,9 +15,11 @@ #include "DD4hep/Printout.h" #include "DD4hep/InstanceCount.h" #include "DDCond/ConditionsIOVPool.h" -#include "DD4hep/detail/ConditionsInterna.h" +#include "DDCond/ConditionsCleanup.h" #include "DDCond/ConditionsDataLoader.h" +#include "DD4hep/detail/ConditionsInterna.h" + using namespace dd4hep; using namespace dd4hep::cond; @@ -66,6 +68,23 @@ size_t ConditionsIOVPool::selectRange(Condition::key_type key, const IOV& req_va return result.size() - len; } +/// Invoke cache cleanup with user defined policy +int ConditionsIOVPool::clean(const ConditionsCleanup& cleaner) { + Elements rest; + int count = 0; + for( const auto& e : elements ) { + const ConditionsPool* p = e.second.get(); + if ( cleaner (*p) ) { + count += e.second->size(); + e.second->print("Remove"); + continue; + } + rest.insert(e); + } + elements = std::move(rest); + return count; +} + /// Remove all key based pools with an age beyon the minimum age int ConditionsIOVPool::clean(int max_age) { Elements rest; diff --git a/DDCond/src/ConditionsManager.cpp b/DDCond/src/ConditionsManager.cpp index cf33735e0..250c75ae4 100644 --- a/DDCond/src/ConditionsManager.cpp +++ b/DDCond/src/ConditionsManager.cpp @@ -245,6 +245,11 @@ void ConditionsManager::clean(const IOVType* typ, int max_age) const { access()->clean(typ, max_age); } +/// Invoke cache cleanup with user defined policy +void ConditionsManager::clean(const ConditionsCleanup& cleaner) const { + access()->clean(cleaner); +} + /// Full cleanup of all managed conditions. void ConditionsManager::clear() const { access()->clear(); diff --git a/DDCond/src/Type1/Manager_Type1.cpp b/DDCond/src/Type1/Manager_Type1.cpp index 4ee6c0ab4..0b57ba0a8 100644 --- a/DDCond/src/Type1/Manager_Type1.cpp +++ b/DDCond/src/Type1/Manager_Type1.cpp @@ -29,6 +29,7 @@ #include "DDCond/ConditionsPool.h" #include "DDCond/ConditionsEntry.h" +#include "DDCond/ConditionsCleanup.h" #include "DDCond/ConditionsManager.h" #include "DDCond/ConditionsIOVPool.h" #include "DDCond/ConditionsDataLoader.h" @@ -311,6 +312,19 @@ int Manager_Type1::clean(const IOVType* typ, int max_age) { return count; } +/// Invoke cache cleanup with user defined policy +pair<int,int> Manager_Type1::clean(const ConditionsCleanup& cleaner) { + pair<int,int> count(0,0); + for( TypedConditionPool::iterator i=m_rawPool.begin(); i != m_rawPool.end(); ++i) { + ConditionsIOVPool* p = *i; + if ( p && cleaner(*p) ) { + ++count.first; + count.second += p->clean(cleaner); + } + } + return count; +} + /// Full cleanup of all managed conditions. pair<int,int> Manager_Type1::clear() { pair<int,int> count(0,0); diff --git a/DDCore/src/AlignmentsCalculator.cpp b/DDCore/src/AlignmentsCalculator.cpp index d60e3a3c5..910092009 100644 --- a/DDCore/src/AlignmentsCalculator.cpp +++ b/DDCore/src/AlignmentsCalculator.cpp @@ -213,7 +213,6 @@ Result AlignmentsCalculator::compute(const std::map<DetElement, Delta>& deltas, ConditionsMap& alignments) const { Result result; - Calculator obj; Calculator::Context context(alignments); // This is a tricky one. We absolutely need the detector elements ordered // by their depth aka. the distance to /world. @@ -235,7 +234,6 @@ Result AlignmentsCalculator::compute(const std::map<DetElement, const Delta*>& d ConditionsMap& alignments) const { Result result; - Calculator obj; Calculator::Context context(alignments); // This is a tricky one. We absolutely need the detector elements ordered // by their depth aka. the distance to /world. diff --git a/DDDB/include/Detector/IDetService.h b/DDDB/include/Detector/IDetService.h index 3954e1b06..48e453d41 100644 --- a/DDDB/include/Detector/IDetService.h +++ b/DDDB/include/Detector/IDetService.h @@ -39,12 +39,12 @@ namespace gaudi { typedef dd4hep::cond::ConditionsManager ConditionsManager; /// Other useful type definitions and type short-cuts typedef dd4hep::cond::ConditionUpdateUserContext Context; - typedef dd4hep::cond::ConditionDependency Dependency; - typedef dd4hep::cond::ConditionsCleanup Cleanup; - typedef dd4hep::DetElement DetElement; - typedef dd4hep::Condition Condition; - typedef dd4hep::IOV::Key_first_type EventStamp; - typedef dd4hep::IOVType IOVType; + typedef dd4hep::cond::ConditionDependency Dependency; + typedef dd4hep::cond::ConditionsCleanup Cleanup; + typedef dd4hep::DetElement DetElement; + typedef dd4hep::Condition Condition; + typedef dd4hep::IOV::Key_first_type EventStamp; + typedef dd4hep::IOVType IOVType; typedef std::shared_ptr<dd4hep::cond::ConditionsContent> Content; typedef std::shared_ptr<dd4hep::cond::ConditionsSlice> Slice; diff --git a/DDDB/src/plugins/DDDBDerivedCondTest.cpp b/DDDB/src/plugins/DDDBDerivedCondTest.cpp index 57daa06ee..cc005c16a 100644 --- a/DDDB/src/plugins/DDDBDerivedCondTest.cpp +++ b/DDDB/src/plugins/DDDBDerivedCondTest.cpp @@ -80,11 +80,16 @@ namespace { } virtual ~ConditionUpdate1() { } /// Interface to client Callback in order to update the condition - virtual Condition operator()(const ConditionKey& key, ConditionUpdateContext& ctxt) override final { + virtual Condition operator()(const ConditionKey& key, ConditionUpdateContext& ) override final { printout(context.level,"ConditionUpdate1","++ Building dependent condition: %s",key.name.c_str()); Condition target(key.name,"Alignment"); + target.bind<AlignmentData>(); + return target; + } + /// Interface to client Callback in order to update the condition + void resolve(Condition target, ConditionUpdateContext& ctxt) override final { try { - AlignmentData& data = target.bind<AlignmentData>(); + AlignmentData& data = target.get<AlignmentData>(); Condition cond0 = ctxt.condition(ctxt.key(0)); const Delta& delta = cond0.get<Delta>(); data.delta = delta; @@ -94,9 +99,8 @@ namespace { catch(const exception& exc) { ++context.numFail1; printout(ERROR,"ConditionUpdate2","++ Failed to build condition %s: %s", - key.name.c_str(), exc.what()); + ctxt.key(0).name.c_str(), exc.what()); } - return target; } }; /// Specialized conditions update callback for alignments @@ -115,11 +119,16 @@ namespace { } virtual ~ConditionUpdate2() { } /// Interface to client Callback in order to update the condition - virtual Condition operator()(const ConditionKey& key, ConditionUpdateContext& ctxt) override final { + virtual Condition operator()(const ConditionKey& key, ConditionUpdateContext&) override final { printout(context.level,"ConditionUpdate2","++ Building dependent condition: %s",key.name.c_str()); Condition target(key.name,"Alignment"); + target.bind<AlignmentData>(); + return target; + } + /// Interface to client Callback in order to update the condition + void resolve(Condition target, ConditionUpdateContext& ctxt) override final { try { - AlignmentData& data = target.bind<AlignmentData>(); + AlignmentData& data = target.get<AlignmentData>(); Condition cond0 = ctxt.condition(ctxt.key(0)); const Delta& delta0 = cond0.get<Delta>(); const AlignmentData& data1 = ctxt.get<AlignmentData>(ctxt.key(1)); @@ -131,9 +140,8 @@ namespace { catch(const exception& exc) { ++context.numFail2; printout(ERROR,"ConditionUpdate2","++ Failed to build condition %s: %s", - key.name.c_str(), exc.what()); + ctxt.key(0).name.c_str(), exc.what()); } - return target; } }; /// Specialized conditions update callback for alignments @@ -152,15 +160,20 @@ namespace { } virtual ~ConditionUpdate3() { } /// Interface to client Callback in order to update the condition - virtual Condition operator()(const ConditionKey& key, ConditionUpdateContext& ctxt) override final { + virtual Condition operator()(const ConditionKey& key, ConditionUpdateContext&) override final { printout(context.level,"ConditionUpdate3","++ Building dependent condition: %s",key.name.c_str()); Condition target(key.name,"Alignment"); + target.bind<AlignmentData>(); + return target; + } + /// Interface to client Callback in order to update the condition + void resolve(Condition target, ConditionUpdateContext& ctxt) override final { try { - AlignmentData& data = target.bind<AlignmentData>(); - Condition cond0 = ctxt.condition(ctxt.key(0)); + AlignmentData& data = target.get<AlignmentData>(); + Condition cond0 = ctxt.condition(ctxt.key(0)); const Delta& delta0 = cond0.get<Delta>(); - const AlignmentData& data1 = ctxt.get<AlignmentData>(1); - const AlignmentData& data2 = ctxt.get<AlignmentData>(2); + const AlignmentData& data1 = ctxt.get<AlignmentData>(ctxt.key(1)); + const AlignmentData& data2 = ctxt.get<AlignmentData>(ctxt.key(2)); data.delta = delta0; data.delta = data1.delta; data.delta = data2.delta; @@ -170,9 +183,8 @@ namespace { catch(const exception& exc) { ++context.numFail3; printout(ERROR,"ConditionUpdate3","++ Failed to build condition %s: %s", - key.name.c_str(), exc.what()); + ctxt.key(0).name.c_str(), exc.what()); } - return target; } }; diff --git a/DDDB/src/plugins/DeVeloServiceTest.cpp b/DDDB/src/plugins/DeVeloServiceTest.cpp index 1c7dee4d5..50a8ca37c 100644 --- a/DDDB/src/plugins/DeVeloServiceTest.cpp +++ b/DDDB/src/plugins/DeVeloServiceTest.cpp @@ -167,7 +167,6 @@ namespace { /// __________________________________________________________________________________ long dump() { shared_ptr<dd4hep::cond::ConditionsSlice> slice; - IDetService::ConditionsManager manager = m_service->manager(); const IDetService::IOVType* iov_typ = m_service->iovType("epoch"); printout(INFO,"ConditionsManager","+++ Starting conditions dump loop"); @@ -202,10 +201,12 @@ namespace { } } m_service->cleanup(dd4hep::cond::ConditionsFullCleanup()); + printout(dd4hep::ALWAYS,"ServiceTest","Test finished...."); return 1; } }; + dd4hep::setPrintFormat("%-18s %5s %s"); for(int i=0; i<argc; ++i) { if ( ::strcmp(argv[i],"-print")==0 ) { s_PrintLevel = dd4hep::printLevel(argv[++i]); diff --git a/DDDB/src/plugins/DeVeloTest.cpp b/DDDB/src/plugins/DeVeloTest.cpp index b86668bf9..2514cb3fc 100644 --- a/DDDB/src/plugins/DeVeloTest.cpp +++ b/DDDB/src/plugins/DeVeloTest.cpp @@ -27,6 +27,7 @@ #include "DDCond/ConditionsSlice.h" #include "DDCond/ConditionsIOVPool.h" +#include "DDCond/ConditionsCleanup.h" #include "DDCond/ConditionsManager.h" #include "DDDB/DDDBHelper.h" @@ -200,11 +201,13 @@ namespace { cout << "Exception(This is GOOD!): " << e.what() << endl; } } - m_manager.clear(); + m_manager.clean(dd4hep::cond::ConditionsFullCleanup()); + printout(dd4hep::ALWAYS,"ServiceTest","Test finished...."); return 1; } }; + dd4hep::setPrintFormat("%-18s %5s %s"); for(int i=0; i<argc; ++i) { if ( ::strcmp(argv[i],"-print")==0 ) { s_PrintLevel = dd4hep::printLevel(argv[++i]); diff --git a/DDDB/src/plugins/DetService.cpp b/DDDB/src/plugins/DetService.cpp index 9c08f6755..0eed20715 100644 --- a/DDDB/src/plugins/DetService.cpp +++ b/DDDB/src/plugins/DetService.cpp @@ -314,5 +314,5 @@ DetService::Slice DetService::project(const string& content, /// Invoke slice cleanup void DetService::cleanup(const Cleanup &cleaner) { - cleaner(m_manager); + m_manager.clean(cleaner); } diff --git a/DDDB/src/plugins/DetService.h b/DDDB/src/plugins/DetService.h index e2cc70fbb..e43f62df5 100644 --- a/DDDB/src/plugins/DetService.h +++ b/DDDB/src/plugins/DetService.h @@ -131,8 +131,7 @@ namespace gaudi { const std::string &address) override; /// Add a condition functor for a derived condition to the content - virtual void addContent(Content &content, - Dependency *call); + virtual void addContent(Content &content, Dependency *call) override; /// Close content creation context virtual void closeContent(Content &content) override; diff --git a/examples/DDDB/CMakeLists.txt b/examples/DDDB/CMakeLists.txt index c84ec0141..9f82e15bf 100644 --- a/examples/DDDB/CMakeLists.txt +++ b/examples/DDDB/CMakeLists.txt @@ -158,7 +158,7 @@ if (DD4HEP_USE_XERCESC) -config DD4hep_ConditionsManagerInstaller -plugin DDDB_DerivedAlignmentsTest -print DEBUG -turns 1 -access 3 DEPENDS DDDB_extract_LONGTEST - REGEX_PASS "\\+ DDDB: AlignmentManager: 9352 conditions \\(S:0,L:9352,C:0,M:0\\) alignments: \\(A:6200,M:0\\) for IOV:epoch\\(0\\)" + REGEX_PASS "\\+ DDDB: AlignmentManager: 9352 conditions \\(S:9352,L:0,C:0,M:0\\) alignments: \\(A:6200,M:0\\) for IOV:epoch\\(0\\)" REGEX_FAIL "EXCEPTION;Exception" ) # -- GitLab