diff --git a/CMakeLists.txt b/CMakeLists.txt index cd21b78ebfd19f3a7084d8c95de7f808945e471d..a3f05fb15eb662cd4093901f6051272172ba0f1b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -243,7 +243,7 @@ add_detector_plugin(DetectorTestPlugins Detector/TestScope/scope_cond.cpp Core/tests/src/test_load_scope.cpp ) -target_link_libraries(DetectorTestPlugins GitCondDB::GitCondDB ${FS_LIB} DetectorLib) +target_link_libraries(DetectorTestPlugins GitCondDB::GitCondDB TestUtils ${FS_LIB} DetectorLib) #-------------------------------------------------------------------------- diff --git a/Core/include/Core/ConditionIdentifier.h b/Core/include/Core/ConditionIdentifier.h index dc5195de971dee19149d133aa33a35856a3d4e32..be653bc71d48499d77f8f553e7183ebe7d1acdde 100644 --- a/Core/include/Core/ConditionIdentifier.h +++ b/Core/include/Core/ConditionIdentifier.h @@ -14,8 +14,6 @@ //========================================================================== #pragma once -#include "Core/ConditionsRepository.h" - #include "DD4hep/Conditions.h" #include "DD4hep/DetElement.h" #include "DDCond/ConditionsContent.h" diff --git a/Core/include/Core/ConditionsRepository.h b/Core/include/Core/ConditionsRepository.h index 19b00f8cfd1ed72dcbfa0e50b8ca3aab552862d7..87c6e28b2ac34e2efb0b05ad8571c91526bdff6a 100644 --- a/Core/include/Core/ConditionsRepository.h +++ b/Core/include/Core/ConditionsRepository.h @@ -18,9 +18,34 @@ #include "DDCond/ConditionsContent.h" +#include <boost/multi_index/hashed_index.hpp> +#include <boost/multi_index/key_extractors.hpp> +#include <boost/multi_index/member.hpp> +#include <boost/multi_index/ordered_index.hpp> +#include <boost/multi_index_container.hpp> + +namespace { + namespace bmi = boost::multi_index; +} + namespace LHCb::Detector { - class ConditionIdentifier; + // Tag structs to refer to the three indices of the + // ConditionsRepository::Locations multi-index container + struct by_location_hash {}; + struct by_filename {}; + struct by_condition_key {}; + + /// Utility function to extract the combined location hash (filename + /// and condition name) from a ConditionIdentifier + dd4hep::Condition::key_type location_key( const ConditionIdentifier* entry ); + + /// Utility function to extract the filename hash from a const ConditionIdentifier + inline std::string condition_filename( const ConditionIdentifier* entry ) { return entry->sys_id; }; + + /// Utility function to extract the condition key from a const ConditionIdentifier + inline dd4hep::Condition::key_type condition_key( const ConditionIdentifier* entry ) { return entry->hash; }; + struct RequestSelector; /** @@ -32,7 +57,28 @@ namespace LHCb::Detector { friend struct RequestSelector; public: - std::map<dd4hep::Condition::key_type, ConditionIdentifier*> locations; + // A multi-index container that allows (fast) lookup in three + // ways. the value is either ConditionIdentifier* or + // const ConditionIdentifier*. + template <bool const_pointer = false> + using Locations = boost::multi_index_container< + std::conditional_t<const_pointer, const ConditionIdentifier*, ConditionIdentifier*>, + bmi::indexed_by< + // Analogously to an `std::map` with key the combined hash + // of the filename and the condition name + bmi::ordered_unique< + bmi::tag<LHCb::Detector::by_location_hash>, + bmi::global_fun<const ConditionIdentifier*, dd4hep::Condition::key_type, &location_key>>, + // Analogously to an `std::multimap` with key the hash of the filename + bmi::ordered_non_unique<bmi::tag<LHCb::Detector::by_filename>, + bmi::global_fun<const ConditionIdentifier*, std::string, &condition_filename>>, + // Analogously to an `std::unordered_map` with key the + // combined hash of the detector element and the condition + // name + bmi::hashed_unique< + bmi::tag<LHCb::Detector::by_condition_key>, + bmi::global_fun<const ConditionIdentifier*, dd4hep::Condition::key_type, &condition_key>>>>; + Locations<> locations; private: /// Default assignment operator diff --git a/Core/src/ConditionsGitReader.cpp b/Core/src/ConditionsGitReader.cpp index 27a5d0b936c5fb76a092d1d4cc69c6cdfa4fdda9..996a1241ab6e0c92e8491757df5e46254bdae38e 100644 --- a/Core/src/ConditionsGitReader.cpp +++ b/Core/src/ConditionsGitReader.cpp @@ -95,7 +95,13 @@ int LHCb::Detector::ConditionsGitReader::getObject( const std::string& system_id // Now loading the condition // m_condDB->logger()->level = GitCondDB::Logger::Level::Debug; - auto [data, iov] = m_condDB->get( {m_dbtag, path, time_point} ); + std::string data{}; + GitCondDB::CondDB::IOV iov{}; + + try { + std::tie( data, iov ) = m_condDB->get( {m_dbtag, path, time_point} ); + } catch ( std::runtime_error const& e ) { return 0; } + if ( !m_limitedIovPaths.empty() && m_limitedIovPaths.find( path ) != m_limitedIovPaths.end() ) { if ( iov.since != time_point ) { throw std::runtime_error( diff --git a/Core/src/ConditionsLoader.cpp b/Core/src/ConditionsLoader.cpp index a82745164d3f5008c380d4fccb5cf8cb9c38364c..de17b0d591df8d342b4e1a7fb4e936d5ef5cb16b 100644 --- a/Core/src/ConditionsLoader.cpp +++ b/Core/src/ConditionsLoader.cpp @@ -17,6 +17,7 @@ #include "Core/ConditionsLoader.h" #include "Core/ConditionsReaderContext.h" #include "Core/ConditionsRepository.h" +#include "Core/Keys.h" // Other dd4hep includes #include "Core/UpgradeTags.h" @@ -31,6 +32,7 @@ #include "TTimeStamp.h" #include <Core/yaml_converters.h> +#include <boost/range/iterator_range_core.hpp> #include <exception> #include <set> #include <sstream> @@ -38,6 +40,31 @@ #include <utility> #include <vector> +#include <range/v3/view/all.hpp> +#include <range/v3/view/transform.hpp> +#include <range/v3/view/unique.hpp> + +namespace { + std::tuple<bool, YAML::Node> empty_condition( const LHCb::Detector::ConditionIdentifier* cond ) { + const std::array<dd4hep::Condition::key_type, 2> alignment_keys = {LHCb::Detector::Keys::deltaKey, + LHCb::Detector::Keys::alignmentKey}; + + dd4hep::ConditionKey::KeyMaker key( cond->hash ); + + bool is_alignment = std::any_of( alignment_keys.begin(), alignment_keys.end(), + [&key]( auto hash ) { return key.values.item_key == hash; } ); + + auto const* payload = is_alignment ? "NotFound: !alignment\n position: [0.0 * mm, 0.0 * mm, 0.0 * mm]\n " + "rotation: [0.0 * rad, 0.0 * rad, 0.0 * rad]\n" + : ""; + auto node = YAML::Load( payload ); + return {is_alignment, is_alignment ? node["NotFound"] : node}; + } + + namespace rv = ranges::views; + namespace ra = ranges::actions; +} // namespace + /// Standard constructor, initializes variables LHCb::Detector::ConditionsLoader::ConditionsLoader( dd4hep::Detector& description, dd4hep::cond::ConditionsManager mgr, const std::string& nam ) @@ -90,13 +117,6 @@ size_t LHCb::Detector::ConditionsLoader::load_many( const dd4hep::IOV& req_iov, // via "SYSTEM" keyword (needed only for XML parsing but done here instead of inside the loop) dd4hep::xml::UriContextReader xml_entity_resolver( m_reader.get(), &ctxt ); - // First collect all required URIs which need loading. - // Since one file contains many conditions, we have - // to create a unique set - - // container to keep track of the URLs already visited - std::set<std::string_view> processed_urls; - // FIXME using a stable buffer for storing the condition docs to be parsed might look like an optimization, // but in the case of GitCondDB (or the ConditionsFileReader) it does not really help as each request // creates a new string that then has to be moved into this buffer @@ -106,56 +126,70 @@ size_t LHCb::Detector::ConditionsLoader::load_many( const dd4hep::IOV& req_iov, // dd4hep::setPrintLevel( dd4hep::DEBUG ); // I have the impression ConditionsIdentifier is not a very good name, but I stick by it for the moment - struct ToDoItem { - ToDoItem( const dd4hep::cond::ConditionsLoadInfo* load_info ) - : cond_id{reinterpret_cast<const ConditionIdentifier*>( load_info->ptr() )} {} - bool done = false; - const ConditionIdentifier* cond_id = nullptr; - }; + ConditionsRepository::Locations<true> all_conditions{}; + std::for_each( conditions_to_load.begin(), conditions_to_load.end(), [&all_conditions]( auto entry ) { + auto* load_info = entry.second->ptr(); + if ( !load_info ) { + dd4hep::printout( dd4hep::INFO, "ConditionsLoader", "++ CANNOT update condition: [%16llX] [No load info]", + entry.first ); + } else { + all_conditions.insert( reinterpret_cast<const ConditionIdentifier*>( load_info ) ); + } + } ); - // we convert the list of conditions to load into a map so that we can easily - // flag as already loaded the conditions we get from each URL that are not - // what we requested initially. So, if we ask for conditions A and B, and - // when processing A we load B as it comes from the same URL, then we flag B - // as done so that we can skip the iteration. - std::map<dd4hep::Condition::key_type, ToDoItem> todo_list{conditions_to_load.begin(), conditions_to_load.end()}; + const auto& conditions_by_location_hash = all_conditions.get<by_location_hash>(); + const auto& conditions_by_filename = all_conditions.get<by_filename>(); + + // Extract the range of unique filenames from the conditions + auto unique_filenames = conditions_by_filename | rv::transform( &ConditionIdentifier::sys_id ) | rv::unique; + + auto block_register = [this]( dd4hep::IOV::Key iov_key, const std::vector<dd4hep::Condition>& entity_conditions ) { + dd4hep::cond::ConditionsPool* pool = manager().registerIOV( *m_iovType, iov_key ); + return manager().blockRegister( *pool, entity_conditions ); + }; try { - for ( auto [current_key, item] : todo_list ) { // O[N] - const auto cond_id = item.cond_id; - // FIXME this early skip for conditions already loaded from a URL does not convince me - if ( item.done ) { continue; } - if ( !cond_id ) { - dd4hep::printout( dd4hep::INFO, "ConditionsLoader", "++ CANNOT update condition: [%16llX] [No load info]", - current_key ); - continue; - } + for ( auto filename : unique_filenames ) { - // set::insert returns a pair where the second tells if the insertion - // took place (true) or the entry was already present (false) - if ( !processed_urls.insert( cond_id->sys_id ).second ) { - // no insertion - continue; - } + auto register_condition = [&loaded]( std::vector<dd4hep::Condition>& entity_conditions, + const ConditionIdentifier* cond_ident, const std::string& cond_name, + const YAML::Node& cond_data, const std::string& doc ) { + dd4hep::Condition c = LHCb::YAMLConverters::make_condition( cond_name, cond_data ); + if ( !c.isValid() ) { + // Will eventually have to raise an exception + dd4hep::printout( dd4hep::ERROR, "ConditionsLoader", "++ Failed to load condition %s from %s", + cond_name.c_str(), doc.c_str() ); + } else { + const auto cond_key = cond_ident->hash; + + c->hash = cond_key; + // pass the empty condition to the caller + loaded[cond_key] = c; + + // record the condition for bulk registration to the manager + entity_conditions.push_back( c ); + } + }; // ready, set, chrono started! TTimeStamp start; // Load from the git cond db here XXX - // FIXME: if the interface make sense, we should do something in case of failure - if ( m_reader->load( cond_id->sys_id, &ctxt, buffer ) ) { - auto ext = std::string_view{cond_id->sys_id}; + // If the file can't be loaded, create empty conditions for all + // of the conditions expected in this file + if ( m_reader->load( filename, &ctxt, buffer ) ) { + auto ext = std::string_view{filename}; ext.remove_prefix( ext.find_last_of( '.' ) ); if ( ext == ".yml" || ext == ".yaml" ) { - dd4hep::ConditionKey::KeyMaker kmaker( cond_id->sys_id_hash, 0 ); - const auto* repo = cond_id->repository; + // Use the filename hash as the upper bits of the hash + dd4hep::ConditionKey::KeyMaker kmaker( dd4hep::detail::hash32( filename ), 0 ); // condition YAML files are expected to contain a mapping <name> -> <condition data> - auto data = YAML::Load( ( m_useOverlay ) ? m_overlay.get_or_default( cond_id->sys_id, buffer ) : buffer ); + auto data = YAML::Load( ( m_useOverlay ) ? m_overlay.get_or_default( filename, buffer ) : buffer ); // Checking whether we have conditions to override in that same file (i.e. o.path) // in that case we directly update the YAML node for ( auto const& o : m_conditionsOverride ) { - if ( o.path == cond_id->sys_id ) { + if ( o.path == filename ) { dd4hep::printout( dd4hep::DEBUG, "ConditionsLoader", "++ Overriding %s:%s", o.path.c_str(), o.name.c_str() ); data[o.name] = o.value; @@ -164,65 +198,53 @@ size_t LHCb::Detector::ConditionsLoader::load_many( const dd4hep::IOV& req_iov, // prepare the store for the conditions we will register to the conditions manager // ... it would be nice if we had an actual type here and not a handle - std::vector<dd4hep::Condition> entity_conditons; - entity_conditons.reserve( data.size() ); + std::vector<dd4hep::Condition> entity_conditions; + entity_conditions.reserve( data.size() ); // loop over all conditions in the YAML document for ( const auto& cond_data : data ) { const std::string cond_name = cond_data.first.as<std::string>(); - // see if the condition is in the list of locations(?) + // Check if the condition has been requested. + // Only the upper 32 bits of kmaker.hash have been set + // above, set the lower bits here kmaker.values.item_key = dd4hep::detail::hash32( cond_name ); - auto cond_ident = repo->locations.find( kmaker.hash ); // O[log(N)] - if ( cond_ident == repo->locations.end() ) { - dd4hep::printout( dd4hep::ERROR, "ConditionsLoader", "++ Got stray condition %s from %s", - cond_name.c_str(), cond_id->sys_id.c_str() ); - continue; - } - dd4hep::Condition c = LHCb::YAMLConverters::make_condition( cond_name, cond_data.second ); - if ( !c.isValid() ) { - // Will eventually have to raise an exception - dd4hep::printout( dd4hep::ERROR, "ConditionsLoader", "++ Failed to load condition %s from %s", - cond_name.c_str(), cond_id->sys_id.c_str() ); - continue; + auto cond_ident = conditions_by_location_hash.find( kmaker.hash ); // O[log(N)] + if ( cond_ident != conditions_by_location_hash.end() ) { + register_condition( entity_conditions, *cond_ident, cond_name, cond_data.second, filename ); + } else { + dd4hep::printout( dd4hep::WARNING, "ConditionsLoader", "++ Got stray condition %s from %s", + cond_name.c_str(), filename.c_str() ); } + } - // FIXME: is this not the same as kmaker.hash? - const auto cond_key = cond_ident->second->hash; - c->hash = cond_key; - // pass the condition to the caller - loaded[cond_key] = c; - // record the condition for bulk registration to the manager - entity_conditons.push_back( c ); - // if this is the condition we were looking for, - // flag the ToDoItem as done and move to the next condition in the YAML object - if ( cond_key == current_key ) { - item.done = true; - continue; + // Check if there are conditions in this file that have been + // requested, but have not yet been loaded. If so, create + // empty conditions for those too. + for ( const auto* cond_ident : + boost::make_iterator_range( conditions_by_filename.equal_range( filename ) ) ) { + auto loaded_it = loaded.find( cond_ident->hash ); + if ( loaded_it == loaded.end() ) { + auto [is_alignment, cond] = empty_condition( cond_ident ); + register_condition( entity_conditions, cond_ident, "NotFound", cond, filename ); + dd4hep::printout( is_alignment ? dd4hep::WARNING : dd4hep::INFO, "ConditionsLoader", + "++ Created empty%s condition %s in %s", is_alignment ? " alignment" : "", + cond_ident->object.c_str(), cond_ident->sys_id.c_str() ); } - // otherwise see if we are supposed to load it, - // in which case the pending work item can be marked as already dine - if ( auto cond_itr = todo_list.find( cond_key ); cond_itr != todo_list.end() ) { - cond_itr->second.done = true; - continue; - } - // this warning means that we found in the YAML file a condition we were not requesting - // FIXME I do not see why this is a problem - dd4hep::printout( dd4hep::WARNING, "ConditionsLoader", "++ Got stray but valid condition %s from %s", - cond_name.c_str(), cond_id->sys_id.c_str() ); } - // now that we converted the whole file we make a pool for the IOV of the YAML file - // and we register the conditions we found + // now that we converted the whole file and created empty conditions for those that are missing, + // we make a pool for the IOV of the YAML file and we register the conditions; the IOV for missing + // conditions should be the same as for those that were found, as they may only appear at the end of + // the IOV // FIXME why one pool per YAML file? should it not be one pool for all conditions? (given an event time) dd4hep::IOV::Key iov_key( ctxt.valid_since, ctxt.valid_until ); // FIXME where does the IOVType comes from? why it is bound the the ConditionsLoader instance? - dd4hep::cond::ConditionsPool* pool = manager().registerIOV( *m_iovType, iov_key ); - size_t ret = manager().blockRegister( *pool, entity_conditons ); + size_t ret = block_register( iov_key, entity_conditions ); // FIXME it looks like blockRegister returns the number of conditions registered and we have to check // that it's what we expect, but clearly we do not know what to do with that information - if ( ret != entity_conditons.size() ) { + if ( ret != entity_conditions.size() ) { // Error! } } @@ -232,10 +254,30 @@ size_t LHCb::Detector::ConditionsLoader::load_many( const dd4hep::IOV& req_iov, std::string valid_end = ctxt.valid_until == std::numeric_limits<std::int64_t>::max() ? std::string{"inf "} : std::to_string( ctxt.valid_until ); - dd4hep::printout( - dd4hep::INFO, "ConditionsLoader", "++ Added %4ld conditions from %-48s to pool [%6s-%6s] [%7.3f sec]", - loaded.size() - loaded_len, cond_id->sys_id.c_str(), std::to_string( ctxt.valid_since ).c_str(), - valid_end.c_str(), stop.AsDouble() - start.AsDouble() ); + dd4hep::printout( dd4hep::INFO, "ConditionsLoader", + "++ Added %4ld conditions from %-48s to pool [%6s-%6s] [%7.3f sec]", + loaded.size() - loaded_len, filename.c_str(), std::to_string( ctxt.valid_since ).c_str(), + valid_end.c_str(), stop.AsDouble() - start.AsDouble() ); + loaded_len = loaded.size(); + } else { + // File containing Conditions not found, create empty + // conditions for all requested conditions that are in this + // file. + std::vector<dd4hep::Condition> entity_conditions; + entity_conditions.reserve( conditions_by_filename.count( filename ) ); + + for ( const auto* cond_ident : boost::make_iterator_range( conditions_by_filename.equal_range( filename ) ) ) { + auto [is_alignment, cond] = empty_condition( cond_ident ); + register_condition( entity_conditions, cond_ident, "NotFound", cond, filename ); + } + + // If the file is not in the DB, it's never going to appear, + // so register empty conditions with an infinite IOV + dd4hep::IOV::Key iov_key( 0, std::numeric_limits<std::int64_t>::max() ); + block_register( iov_key, entity_conditions ); + + dd4hep::printout( dd4hep::INFO, "ConditionsLoader", "++ Added %4ld null conds for %-48s to pool [%6s-%6s]", + loaded.size() - loaded_len, filename.c_str(), std::to_string( 0 ).c_str(), "inf " ); loaded_len = loaded.size(); } if ( print_results ) { diff --git a/Core/src/ConditionsRepository.cpp b/Core/src/ConditionsRepository.cpp index f5722b7cfded7984a62ca611d739e94e2381fc48..6e06712d319cfb6fd594d4325008c21b8f4fc5fd 100644 --- a/Core/src/ConditionsRepository.cpp +++ b/Core/src/ConditionsRepository.cpp @@ -18,11 +18,16 @@ //========================================================================== // Framework includes -#include "Core/ConditionIdentifier.h" +#include "Core/ConditionsRepository.h" #include "DDCond/ConditionsContent.h" #include <iostream> +dd4hep::Condition::key_type LHCb::Detector::location_key( const ConditionIdentifier* entry ) { + dd4hep::ConditionKey::KeyMaker loc( entry->sys_id_hash, entry->object_hash ); + return loc.hash; +} + // C/C++ include files template <> std::string dd4hep::cond::ConditionsContent::LoadInfo<LHCb::Detector::ConditionIdentifier>::toString() const { @@ -38,17 +43,16 @@ LHCb::Detector::ConditionsRepository::addLocation( dd4hep::DetElement de, dd4hep const std::string& sys_id, const std::string& object ) { ConditionIdentifier ident; dd4hep::ConditionKey::KeyMaker km( de.key(), item ); - ident.hash = km.hash; - ident.detector = de; - ident.object = object; - ident.sys_id = sys_id; - ident.sys_id_hash = dd4hep::detail::hash32( sys_id ); - ident.object_hash = dd4hep::detail::hash32( object ); - ident.repository = this; - auto* info = new LoadInfo<ConditionIdentifier>( ident ); - auto ret = addLocationInfo( km.hash, info ); - dd4hep::ConditionKey::KeyMaker loc( ident.sys_id_hash, ident.object_hash ); - locations.insert( std::make_pair( loc.hash, &info->info ) ); + ident.hash = km.hash; + ident.detector = de; + ident.object = object; + ident.sys_id = sys_id; + ident.sys_id_hash = dd4hep::detail::hash32( sys_id ); + ident.object_hash = dd4hep::detail::hash32( object ); + ident.repository = this; + auto* info = new LoadInfo<ConditionIdentifier>( ident ); + auto ret = addLocationInfo( km.hash, info ); + locations.insert( &info->info ); return ret; } @@ -57,17 +61,16 @@ LHCb::Detector::ConditionsRepository::addGlobal( dd4hep::DetElement de, dd4hep:: const std::string& sys_id, const std::string& object ) { ConditionIdentifier ident; dd4hep::ConditionKey::KeyMaker km( 0, item ); - ident.hash = km.hash; - ident.detector = de; - ident.object = object; - ident.sys_id = sys_id; - ident.sys_id_hash = dd4hep::detail::hash32( sys_id ); - ident.object_hash = dd4hep::detail::hash32( object ); - ident.repository = this; - auto* info = new LoadInfo<ConditionIdentifier>( ident ); - auto ret = addLocationInfo( km.hash, info ); - dd4hep::ConditionKey::KeyMaker loc( ident.sys_id_hash, ident.object_hash ); - locations.insert( std::make_pair( loc.hash, &info->info ) ); + ident.hash = km.hash; + ident.detector = de; + ident.object = object; + ident.sys_id = sys_id; + ident.sys_id_hash = dd4hep::detail::hash32( sys_id ); + ident.object_hash = dd4hep::detail::hash32( object ); + ident.repository = this; + auto* info = new LoadInfo<ConditionIdentifier>( ident ); + auto ret = addLocationInfo( km.hash, info ); + locations.insert( &info->info ); return ret; } diff --git a/Core/src/DetectorDataService.cpp b/Core/src/DetectorDataService.cpp index 6810a69f91c6508273301ad2e6ed81a9602a0b58..3b308f6488906f6d747cebe88b00f9d64fe13666 100644 --- a/Core/src/DetectorDataService.cpp +++ b/Core/src/DetectorDataService.cpp @@ -133,7 +133,6 @@ void LHCb::Detector::DetectorDataService::initialize( const nlohmann::json& conf // make sure the detector_names option is in sync auto& opts = *m_description.extension<Options>(); opts["detector_names"] = m_detectorNames; - if ( config.contains( "without_tell40links" ) ) { opts["without_tell40links"] = config.at( "without_tell40links" ); } // Now setup DeLHCb // BEWARE: HAS TO BE DONE BEFORE THE NEXT STEP THAT GATHERS ALL THE CONDITIONS @@ -182,11 +181,10 @@ void LHCb::Detector::DetectorDataService::initialize( const nlohmann::json& conf auto cond_key = LHCb::Detector::ConditionKey( m_description.detector( std::string{deName} ), std::string{condKey} ); // Check in the conditions mappings which file this corresponds to - if ( auto el = std::find_if( begin( m_all_conditions->locations ), end( m_all_conditions->locations ), - [cond_key]( const auto& item ) { return item.second->hash == cond_key; } ); - el != end( m_all_conditions->locations ) ) { + const auto& conditions_by_key = m_all_conditions->locations.get<by_condition_key>(); + if ( auto el = conditions_by_key.find( cond_key ); el != end( conditions_by_key ) ) { overrides.emplace_back( - LHCb::Detector::ConditionOverrideEntry{el->second->sys_id, el->second->object, YAML::Load( o.second )} ); + LHCb::Detector::ConditionOverrideEntry{( *el )->sys_id, ( *el )->object, YAML::Load( o.second )} ); } else { throw std::invalid_argument{"cannot find " + std::string{condKey} + " in " + std::string{deName} + "'s ConditionsRepository"}; @@ -324,11 +322,10 @@ void LHCb::Detector::DetectorDataService::update_condition( const dd4hep::DetEle void LHCb::Detector::DetectorDataService::update_condition( const dd4hep::DetElement& de, const std::string& cond, const YAML::Node& data ) { - auto key = LHCb::Detector::ConditionKey( de, cond ); - if ( auto el = std::find_if( begin( m_all_conditions->locations ), end( m_all_conditions->locations ), - [key]( const auto& item ) { return item.second->hash == key; } ); - el != end( m_all_conditions->locations ) ) { - update_condition( el->second->sys_id, el->second->object, data ); + auto key = LHCb::Detector::ConditionKey( de, cond ); + const auto& conditions_by_key = m_all_conditions->locations.get<by_condition_key>(); + if ( auto el = conditions_by_key.find( key ); el != end( conditions_by_key ) ) { + update_condition( ( *el )->sys_id, ( *el )->object, data ); } else { throw std::invalid_argument{"cannot find " + cond + " in " + de.name() + "'s ConditionsRepository"}; } diff --git a/Core/tests/src/test_DetectorDataService.cpp b/Core/tests/src/test_DetectorDataService.cpp index 088614b69921a6ae061813458c6eaf9c45dd26d2..c24df16b1332488e95a7150f38c34a13a0c5efc9 100644 --- a/Core/tests/src/test_DetectorDataService.cpp +++ b/Core/tests/src/test_DetectorDataService.cpp @@ -19,13 +19,26 @@ #include "Detector/Magnet/DeMagnet.h" #include "Detector/VP/DeVP.h" +#include <Detector/Test/Fixture.h> #include <cstring> #include <exception> #include <nlohmann/json.hpp> +#include <regex> #include <string> #include <vector> +#include <range/v3/action/split.hpp> +#include <range/v3/range/primitives.hpp> +#include <range/v3/view/all.hpp> +#include <range/v3/view/c_str.hpp> +#include <range/v3/view/filter.hpp> + +namespace { + namespace ra = ranges::actions; + namespace rv = ranges::views; +} // namespace + static long test_DetectorDataService( dd4hep::Detector& description, int argc, char** argv ) { bool help = false; @@ -442,11 +455,31 @@ static long test_VP_motion( dd4hep::Detector& description, int argc, char** argv return position; }; + // Use a fixture to capture DD4hep output so we can check if + // warnings about missing alignment conditions were printed + Detector::Test::Fixture f{true}; + // Getting the first position where motion should be 0 auto position100 = get_vpleft_pos( 100 ); + std::cout << "run 100 - VP Left position: " << position100 << '\n'; + auto dd4hep_output = f.printout.str(); + + // Check if warnings were printed about missing alignment conditions + auto lines = ra::split( dd4hep_output, ranges::views::c_str( "\n" ) ); + std::regex missing_align{R"del(WARN\s+.+empty alignment condition)del"}; + std::smatch result; + auto warnings = lines | rv::filter( [&missing_align, &result]( auto line ) { + return std::regex_search( line, result, missing_align ); + } ); + if ( ranges::empty( warnings ) ) { + std::cout << "No warnings printed about missing alignment conditions; expected 2" << std::endl; + ::exit( EINVAL ); + } + auto position1000 = get_vpleft_pos( 1000 ); + std::cout << "run 1000 - VP Left position: " << position1000 << '\n'; auto diff = position1000 - position100; diff --git a/Core/tests/src/test_demuon.cpp b/Core/tests/src/test_demuon.cpp index e39db717cff17edf0d2d6ccad2b9b6d5ad88e4c9..8c005d6d6623caba85eb0b9406e593b16ee648cd 100644 --- a/Core/tests/src/test_demuon.cpp +++ b/Core/tests/src/test_demuon.cpp @@ -114,7 +114,7 @@ void _iterate_muon( const LHCb::Detector::DeMuon& muon ) { // test listOfPhysChannels and related code const std::array<LHCb::Detector::DeMuonGasGap, 4> test_all_gaps = det_ch.gaps(); unsigned int i = 0; - double zin, zout; + double zin = 0., zout = 0.; for ( const auto& igap : test_all_gaps ) { i++; @@ -369,7 +369,7 @@ static long test_load_demuon( dd4hep::Detector& description, int argc, char** ar // test listOfPhysChannels and related code const std::array<LHCb::Detector::DeMuonGasGap, 4> test_all_gaps = det_ch.gaps(); unsigned int i = 0; - double zin, zout; + double zin = 0., zout = 0.; for ( const auto& igap : test_all_gaps ) { i++; diff --git a/Core/tests/src/test_scope.cpp b/Core/tests/src/test_scope.cpp index 2bcfd626f3bd70d00270cf6d87800840ad653d69..d576b59d63ca10ece37b7773cf30c1beaa78d44c 100644 --- a/Core/tests/src/test_scope.cpp +++ b/Core/tests/src/test_scope.cpp @@ -32,7 +32,7 @@ TEST_CASE( "scope_test" ) { description.fromXML( compact_path ); std::vector<std::string> detector_list{"/world", "scope"}; LHCb::Detector::DetectorDataService dds( description, detector_list ); - dds.initialize( nlohmann::json{{"repository", conditions_path}, {"without_tell40links", true}} ); + dds.initialize( nlohmann::json{{"repository", conditions_path}} ); auto slice = dds.get_slice( 100 ); dd4hep::DetElement det = description.detector( "/world/scope/side1/sensor0" ); diff --git a/Detector/LHCb/include/Detector/LHCb/DeLHCb.h b/Detector/LHCb/include/Detector/LHCb/DeLHCb.h index 25a9f0756ac1422ab8f6bee27b34a346d17ac693..e5d9c1b7eb5465a0de755dab66d3c38b7c85327d 100644 --- a/Detector/LHCb/include/Detector/LHCb/DeLHCb.h +++ b/Detector/LHCb/include/Detector/LHCb/DeLHCb.h @@ -24,7 +24,7 @@ namespace LHCb::Detector { namespace detail { struct DeLHCbObject : DeIOVObject { - DeLHCbObject( const dd4hep::DetElement& de, dd4hep::cond::ConditionUpdateContext& ctxt, bool withTell40Links ); + DeLHCbObject( const dd4hep::DetElement& de, dd4hep::cond::ConditionUpdateContext& ctxt ); void applyToAllChildren( const std::function<void( DeIOVElement<DeIOVObject> )>& ) const override; std::vector<DeIOV> children; Tell40Links m_tell40links; diff --git a/Detector/LHCb/src/DeLHCb.cpp b/Detector/LHCb/src/DeLHCb.cpp index 83700a2c6ab0df14f04908ddd92b07e5b9919560..8fdd9390c262ffb0d39b86f8b4269073fb59e5e2 100644 --- a/Detector/LHCb/src/DeLHCb.cpp +++ b/Detector/LHCb/src/DeLHCb.cpp @@ -18,9 +18,11 @@ #include <stdexcept> LHCb::Detector::detail::DeLHCbObject::DeLHCbObject( const dd4hep::DetElement& de, - dd4hep::cond::ConditionUpdateContext& ctxt, bool withTell40Links ) + dd4hep::cond::ConditionUpdateContext& ctxt ) : DeIOVObject( de, ctxt, 9000000, false ) { - if ( withTell40Links ) { m_tell40links = ctxt.condition( hash_key( de, "Tell40Links" ) ).get<nlohmann::json>(); } + + auto links = ctxt.condition( hash_key( de, "Tell40Links" ) ).get<nlohmann::json>(); + if ( !links.is_null() ) { m_tell40links = links; } } void LHCb::Detector::detail::DeLHCbObject::applyToAllChildren( @@ -45,15 +47,13 @@ namespace { std::vector<std::string> children_names; // Contains the DeIOVs for all the children - bool withTell40Links{true}; - virtual dd4hep::Condition operator()( const dd4hep::ConditionKey& key, dd4hep::cond::ConditionUpdateContext& context ) override final { if ( key.item_key() == LHCb::Detector::Keys::deKey ) { // This is the pointer we need to return at the end of the callback auto lhcbdet = dd4hep::Detector::getInstance().world(); // Creating the Detector element with the desired field - auto delhcbobj = new LHCb::Detector::detail::DeLHCbObject( lhcbdet, context, withTell40Links ); + auto delhcbobj = new LHCb::Detector::detail::DeLHCbObject( lhcbdet, context ); // Now looking up the children DeIOVs for ( const auto& name : children_names ) { LHCb::Detector::DeIOV deiov = LHCb::Detector::detail::get_DeIOV_from_context( context, name ); @@ -96,14 +96,10 @@ void LHCb::Detector::setup_DeLHCb_callback( dd4hep::Detector& description ) { depbuilder.add( hash_key( description.detector( name ), LHCb::Detector::Keys::deKey ) ); } - iovUpdate->withTell40Links = - opts && !( opts->contains( "without_tell40links" ) && opts->at( "without_tell40links" ).get<bool>() ); - if ( iovUpdate->withTell40Links ) { - ( *requests ) - ->addLocation( de, LHCb::Detector::item_key( "Tell40Links" ), "Conditions/LHCb/Online/Tell40Links.yml", - "Tell40Links" ); - depbuilder.add( hash_key( de, "Tell40Links" ) ); - } + ( *requests ) + ->addLocation( de, LHCb::Detector::item_key( "Tell40Links" ), "Conditions/LHCb/Online/Tell40Links.yml", + "Tell40Links" ); + depbuilder.add( hash_key( de, "Tell40Links" ) ); ( *requests )->addDependency( depbuilder.release() ); } diff --git a/TestUtils/CMakeLists.txt b/TestUtils/CMakeLists.txt index 55d35844278f9beafc05974dad9939c063830b9f..9ba5119de883bf0c46314a0cdc34ca32d9ec50fe 100644 --- a/TestUtils/CMakeLists.txt +++ b/TestUtils/CMakeLists.txt @@ -10,10 +10,12 @@ ############################################################################### # Library with helper classes to simplify writing tests -add_library(TestUtils INTERFACE) -target_include_directories(TestUtils INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/include) +add_library(TestUtils SHARED src/Fixture.cpp) +target_include_directories(TestUtils PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include) target_link_libraries(TestUtils - INTERFACE - ROOT::Core - DD4hep::DDCore + PRIVATE + fmt::fmt + PUBLIC + ROOT::Core + DD4hep::DDCore ) diff --git a/TestUtils/include/Detector/Test/Fixture.h b/TestUtils/include/Detector/Test/Fixture.h index ba629a7f067a60c7fbdfff7bde65f9c25680b938..f2a7ab4986506cb1a1f78ef2cbf2cce4513ba46b 100644 --- a/TestUtils/include/Detector/Test/Fixture.h +++ b/TestUtils/include/Detector/Test/Fixture.h @@ -17,11 +17,18 @@ #include <string_view> namespace Detector::Test { + + size_t test_printer( void* fixture, dd4hep::PrintLevel, const char* src, const char* text ); + struct Fixture { // Instantiate the global dd4hep instance and silence printouts - Fixture() : originalLevels{gErrorIgnoreLevel, dd4hep::printLevel()} { + Fixture( bool capture_printout = false ) : originalLevels{gErrorIgnoreLevel, dd4hep::printLevel()} { + auto keep_output = std::getenv( "KEEP_OUTPUT" ); - if ( !keep_output || std::string_view( keep_output ) == "" ) { + + if ( capture_printout ) { + dd4hep::setPrinter( this, &test_printer ); + } else if ( !keep_output || std::string_view( keep_output ) == "" ) { // Set the error level to ignore all messages // - from DD4hep dd4hep::setPrintLevel( static_cast<dd4hep::PrintLevel>( dd4hep::ALWAYS + 1 ) ); @@ -35,6 +42,8 @@ namespace Detector::Test { // Restore the error level gErrorIgnoreLevel = originalLevels.gErrorIgnoreLevel; dd4hep::setPrintLevel( originalLevels.printLevel ); + // Restore the default printer + dd4hep::setPrinter( nullptr, nullptr ); // destroy the global dd4hep instance dd4hep::Detector::destroyInstance(); } @@ -44,6 +53,9 @@ namespace Detector::Test { dd4hep::PrintLevel printLevel; } originalLevels; + std::mutex output_mutex; + std::stringstream printout; + dd4hep::Detector* descriptionPtr{nullptr}; // helpers to access the detector description diff --git a/TestUtils/src/Fixture.cpp b/TestUtils/src/Fixture.cpp new file mode 100644 index 0000000000000000000000000000000000000000..f2b84d2c977df03c629f03ac49f2e56f8944db04 --- /dev/null +++ b/TestUtils/src/Fixture.cpp @@ -0,0 +1,50 @@ +/*****************************************************************************\ +* (c) Copyright 2022 CERN for the benefit of the LHCb Collaboration * +* * +* This software is distributed under the terms of the GNU General Public * +* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * +* * +* In applying this licence, CERN does not waive the privileges and immunities * +* granted to it by virtue of its status as an Intergovernmental Organization * +* or submit itself to any jurisdiction. * +\*****************************************************************************/ +#include <Detector/Test/Fixture.h> +#include <fmt/format.h> + +namespace { + + // Copied from DD4hep, because it's not available outside it... + const char* print_level( dd4hep::PrintLevel lvl ) { + switch ( lvl ) { + case dd4hep::NOLOG: + return "NOLOG"; + case dd4hep::VERBOSE: + return "VERB "; + case dd4hep::DEBUG: + return "DEBUG"; + case dd4hep::INFO: + return "INFO "; + case dd4hep::WARNING: + return "WARN "; + case dd4hep::ERROR: + return "ERROR"; + case dd4hep::FATAL: + return "FATAL"; + case dd4hep::ALWAYS: + return " "; + default: + if ( lvl > dd4hep::ALWAYS ) return print_level( dd4hep::ALWAYS ); + return print_level( dd4hep::NOLOG ); + } + } +} // namespace + +size_t Detector::Test::test_printer( void* fixture, dd4hep::PrintLevel lvl, const char* src, const char* text ) { + Fixture* f = reinterpret_cast<Fixture*>( fixture ); + std::lock_guard<std::mutex> lock( f->output_mutex ); + // std::string s = std::string{src} + " " + text; + std::string s = fmt::format( "{:-<16} {} {}", src, print_level( lvl ), text ); + f->printout << s << "\n"; + std::cout << s << "\n"; + return s.size() + 1; +} diff --git a/tests/ConditionsIOV/Conditions/VP/Alignment/Global.yml/v1 b/tests/ConditionsIOV/Conditions/VP/Alignment/Global.yml/v1 index 32c5f2898c02b483860217bb32b7aa657653ac3e..515d5dbe97b22c1ce3e4bd99c5e16eaf73f33ea2 100644 --- a/tests/ConditionsIOV/Conditions/VP/Alignment/Global.yml/v1 +++ b/tests/ConditionsIOV/Conditions/VP/Alignment/Global.yml/v1 @@ -1,6 +1,2 @@ VPSystem: !alignment position: [1.0 * mm, 0.1 * cm, 1000 * um] -VPLeft: !alignment - position: [0.0 * mm, 0.0 * mm, 0.0 * mm] -VPRight: !alignment - position: [0.0 * mm, 0.0 * mm, 0.0 * mm] diff --git a/tests/ConditionsIOV/Conditions/VP/Alignment/Global.yml/v2 b/tests/ConditionsIOV/Conditions/VP/Alignment/Global.yml/v2 index fcbf9690f01ebcc5a2ed3da9f5755dd805a86ba3..287dec0b346d88a263cb6f7c8fa6ced3f61d766b 100644 --- a/tests/ConditionsIOV/Conditions/VP/Alignment/Global.yml/v2 +++ b/tests/ConditionsIOV/Conditions/VP/Alignment/Global.yml/v2 @@ -1,6 +1,4 @@ VPSystem: !alignment position: [1.0 * mm, 1.0 * mm, 1.0 * mm] -VPLeft: !alignment - position: [0.0 * mm, 0.0 * mm, 0.0 * mm] VPRight: !alignment position: [5.0 * mm, 5.0 * mm, 5.0 * mm] diff --git a/tests/ConditionsIOV/Conditions/VP/Motion.yml/0 b/tests/ConditionsIOV/Conditions/VP/Motion.yml/0 index 8d48475d2fb370f88741e5630432dd91170708a3..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 --- a/tests/ConditionsIOV/Conditions/VP/Motion.yml/0 +++ b/tests/ConditionsIOV/Conditions/VP/Motion.yml/0 @@ -1,4 +0,0 @@ -MotionVPLeft: !alignment - position: [0.0 * mm, 0.0 * mm, 0.0 * mm] -MotionVPRight: !alignment - position: [0.0 * mm, 0.0 * mm, 0.0 * mm] diff --git a/tests/ConditionsIOV/Conditions/VP/Motion.yml/400 b/tests/ConditionsIOV/Conditions/VP/Motion.yml/400 index 8d48475d2fb370f88741e5630432dd91170708a3..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 --- a/tests/ConditionsIOV/Conditions/VP/Motion.yml/400 +++ b/tests/ConditionsIOV/Conditions/VP/Motion.yml/400 @@ -1,4 +0,0 @@ -MotionVPLeft: !alignment - position: [0.0 * mm, 0.0 * mm, 0.0 * mm] -MotionVPRight: !alignment - position: [0.0 * mm, 0.0 * mm, 0.0 * mm]