diff --git a/src/core/Corryvreckan.cpp b/src/core/Corryvreckan.cpp index 515f1aaa3844ceceadc8a466f11e6e3af2a7fcd7..3507da5887185ce86539806b2fdaddeff177515b 100644 --- a/src/core/Corryvreckan.cpp +++ b/src/core/Corryvreckan.cpp @@ -109,7 +109,7 @@ void Corryvreckan::load() { // Put welcome message and set version LOG(STATUS) << "Welcome to Corryvreckan " << CORRYVRECKAN_PROJECT_VERSION; - global_config.set("version", CORRYVRECKAN_PROJECT_VERSION); + global_config.set("version", CORRYVRECKAN_PROJECT_VERSION, true); // Get output directory std::string directory = gSystem->pwd(); diff --git a/src/core/config/ConfigManager.cpp b/src/core/config/ConfigManager.cpp index 6cfae0feb854632674273774187cc073d476abda..d5137f4b8fedde1fec2ab07747c44853cbe5e5cf 100644 --- a/src/core/config/ConfigManager.cpp +++ b/src/core/config/ConfigManager.cpp @@ -101,7 +101,7 @@ bool ConfigManager::loadModuleOptions(const std::vector& options) { bool optionsApplied = false; // Parse the options - for(auto& option : options) { + for(const auto& option : options) { module_option_parser_.parseOption(option); } @@ -128,7 +128,7 @@ bool ConfigManager::loadDetectorOptions(const std::vector& options) OptionParser detector_option_parser; // Parse the options - for(auto& option : options) { + for(const auto& option : options) { detector_option_parser.parseOption(option); } @@ -157,7 +157,6 @@ std::list& ConfigManager::getDetectorConfigurations() { if(detector_configs_.empty()) { parse_detectors(); } - return detector_configs_; } diff --git a/src/core/config/ConfigManager.hpp b/src/core/config/ConfigManager.hpp index a9e24743b4499d798b12dfaf2c7ebec71c40f2fe..a8d8d29f1957b24293d5c6a62ec83d4093134632 100644 --- a/src/core/config/ConfigManager.hpp +++ b/src/core/config/ConfigManager.hpp @@ -114,14 +114,13 @@ namespace corryvreckan { std::set global_names_{}; std::set ignore_names_{}; - // Helper function to parse detectors file - void parse_detectors(); - OptionParser module_option_parser_; std::list module_configs_; Configuration global_config_; + // Helper function for delayed parsing of detectors file + void parse_detectors(); std::list detector_configs_; std::list instance_configs_; diff --git a/src/core/config/ConfigReader.cpp b/src/core/config/ConfigReader.cpp index b71cf61e7a33163f8b07be9120be0a4dffa993b2..4cb5a9b52b1d3f2f9b8e03a2d6f07cac573a3b96 100644 --- a/src/core/config/ConfigReader.cpp +++ b/src/core/config/ConfigReader.cpp @@ -230,7 +230,7 @@ std::vector ConfigReader::getConfigurations(std::string name) con } std::vector result; - for(auto& iter : conf_map_.at(name)) { + for(const auto& iter : conf_map_.at(name)) { result.push_back(*iter); } return result; diff --git a/src/core/config/Configuration.cpp b/src/core/config/Configuration.cpp index 80f3568dc673c6483b2d1c2156fc8e0d6db4b600..2b8f6891dddf0f20fe6c571d776e6c7fc95ab972 100644 --- a/src/core/config/Configuration.cpp +++ b/src/core/config/Configuration.cpp @@ -20,6 +20,25 @@ using namespace corryvreckan; +Configuration::AccessMarker::AccessMarker(const Configuration::AccessMarker& rhs) { + for(const auto& key : rhs.markers_) { + registerMarker(key.first); + markers_.at(key.first).store(key.second.load()); + } +} + +Configuration::AccessMarker& Configuration::AccessMarker::operator=(const Configuration::AccessMarker& rhs) { + for(const auto& key : rhs.markers_) { + registerMarker(key.first); + markers_.at(key.first).store(key.second.load()); + } + return *this; +} + +void Configuration::AccessMarker::registerMarker(const std::string& key) { + markers_.emplace(std::piecewise_construct, std::forward_as_tuple(key), std::forward_as_tuple()); +} + Configuration::Configuration(std::string name, std::string path) : name_(std::move(name)), path_(std::move(path)) {} bool Configuration::has(const std::string& key) const { @@ -32,7 +51,7 @@ unsigned int Configuration::count(std::initializer_list keys) const } unsigned int found = 0; - for(auto& key : keys) { + for(const auto& key : keys) { if(has(key)) { found++; } @@ -50,6 +69,7 @@ std::string Configuration::getFilePath() const { std::string Configuration::getText(const std::string& key) const { try { // NOTE: returning literally including "" + used_keys_.markUsed(key); return config_.at(key); } catch(std::out_of_range& e) { throw MissingKeyError(key, getName()); @@ -117,10 +137,11 @@ std::string Configuration::path_to_absolute(std::string path, bool canonicalize_ void Configuration::setText(const std::string& key, const std::string& val) { config_[key] = val; + used_keys_.registerMarker(key); } /** - * The alias is only used if new key does not exist but old key does + * The alias is only used if new key does not exist but old key does. The old key is automatically marked as used. */ void Configuration::setAlias(const std::string& new_key, const std::string& old_key, bool warn) { if(!has(old_key) || has(new_key)) { @@ -128,6 +149,8 @@ void Configuration::setAlias(const std::string& new_key, const std::string& old_ } try { config_[new_key] = config_.at(old_key); + used_keys_.registerMarker(new_key); + used_keys_.markUsed(old_key); } catch(std::out_of_range& e) { throw MissingKeyError(old_key, getName()); } @@ -153,11 +176,11 @@ void Configuration::merge(const Configuration& other) { } } -std::vector> Configuration::getAll() { +std::vector> Configuration::getAll() const { std::vector> result; // Loop over all configuration keys - for(auto& key_value : config_) { + for(const auto& key_value : config_) { // Skip internal keys starting with an underscore if(!key_value.first.empty() && key_value.first.front() == '_') { continue; @@ -169,6 +192,20 @@ std::vector> Configuration::getAll() { return result; } +std::vector Configuration::getUnusedKeys() const { + std::vector result; + + // Loop over all configuration keys, excluding internal ones + for(const auto& key_value : getAll()) { + // Add those to result that have not been accessed: + if(!used_keys_.isUsed(key_value.first)) { + result.emplace_back(key_value.first); + } + } + + return result; +} + /** * String is recursively parsed for all pair of [ and ] brackets. All parts between single or double quotation marks are * skipped. diff --git a/src/core/config/Configuration.hpp b/src/core/config/Configuration.hpp index 629ff6af5e0252c5071b3c889620ebfddc0ab52c..f3ec6ef44e941e6a40e3c402002405c0903e207d 100644 --- a/src/core/config/Configuration.hpp +++ b/src/core/config/Configuration.hpp @@ -10,6 +10,7 @@ #ifndef CORRYVRECKAN_CONFIGURATION_H #define CORRYVRECKAN_CONFIGURATION_H +#include #include #include #include @@ -31,6 +32,58 @@ namespace corryvreckan { * the library of \ref StringConversions. */ class Configuration { + + /** + * @brief Helper class to keep track of key access + * + * This class holds all configuration keys in a map together with an atomic boolean marking whether they have been + * accessed already. This allows to find out whick keys have not been accessed at all. This wrapper allows to use + * atomics for non-locking access but requires to register all keys beforehand. + */ + class AccessMarker { + public: + /** + * Default constructor + */ + AccessMarker() = default; + + /** + * @brief Explicit copy constructor to allow copying of the map keys + */ + AccessMarker(const AccessMarker& rhs); + + /** + * @brief Explicit copy assignment operator to allow copying of the map keys + */ + AccessMarker& operator=(const AccessMarker& rhs); + + /** + * @brief Method to register a key for a new access marker + * @param key Key of the marker + * @warning This operation is not thread-safe + */ + void registerMarker(const std::string& key); + + /** + * @brief Method to mark existing marker as accessed/used. + * @param key Key of the marker + * @note This is an atomic operation and thread-safe. + * @throws std::out_of_range if the key has not been registered beforehand + */ + void markUsed(const std::string& key) { markers_.at(key).store(true); }; + + /** + * @brief Method to retrieve access status of an existing marker. + * @param key Key of the marker + * @note This is an atomic operation and thread-safe. + * @throws std::out_of_range if the key has not been registered beforehand + */ + bool isUsed(const std::string& key) { return markers_.at(key).load(); } + + private: + std::map> markers_; + }; + public: /** * @brief Construct a configuration object @@ -153,8 +206,9 @@ namespace corryvreckan { * @brief Set value for a key in a given type * @param key Key to set value of * @param val Value to assign to the key + * @param mark_used Flag whether key should be marked as "used" directly */ - template void set(const std::string& key, const T& val); + template void set(const std::string& key, const T& val, bool mark_used = false); /** * @brief Store value for a key in a given type, including units @@ -168,9 +222,10 @@ namespace corryvreckan { * @brief Set list of values for a key in a given type * @param key Key to set values of * @param val List of values to assign to the key + * @param mark_used Flag whether key should be marked as "used" directly */ // TODO [doc] Provide second template parameter to specify the vector type to return it in - template void setArray(const std::string& key, const std::vector& val); + template void setArray(const std::string& key, const std::vector& val, bool mark_used = false); /** * @brief Set matrix of values for a key in a given type @@ -211,6 +266,7 @@ namespace corryvreckan { * @param new_key New alias to be created * @param old_key Key the alias is created for * @param warn Optionally print a warning message to notify of deprecation + * @note This marks the old key as "used" automatically */ void setAlias(const std::string& new_key, const std::string& old_key, bool warn = false); @@ -245,7 +301,15 @@ namespace corryvreckan { * @return List of all key value pairs */ // FIXME Better name for this function - std::vector> getAll(); + std::vector> getAll() const; + + /** + * @brief Obtain all keys which have not been accessed yet + * + * This method returns all keys from the configuration object which have not yet been accessed, Default values as + * well as aliases are marked as used automatically and are therefore never returned. + */ + std::vector getUnusedKeys() const; private: /** @@ -275,6 +339,7 @@ namespace corryvreckan { using ConfigMap = std::map; ConfigMap config_; + mutable AccessMarker used_keys_; }; } // namespace corryvreckan diff --git a/src/core/config/Configuration.tpp b/src/core/config/Configuration.tpp index df4494b138f8b721b609f7006f563e22bdc29ba8..ce7d0e8765eb3d3bee549b5aa6816eeff9c60958 100644 --- a/src/core/config/Configuration.tpp +++ b/src/core/config/Configuration.tpp @@ -16,6 +16,7 @@ namespace corryvreckan { template T Configuration::get(const std::string& key) const { try { auto node = parse_value(config_.at(key)); + used_keys_.markUsed(key); try { return corryvreckan::from_string(node->value); } catch(std::invalid_argument& e) { @@ -48,6 +49,7 @@ namespace corryvreckan { template std::vector Configuration::getArray(const std::string& key) const { try { std::string str = config_.at(key); + used_keys_.markUsed(key); std::vector array; auto node = parse_value(str); @@ -86,6 +88,7 @@ namespace corryvreckan { template Matrix Configuration::getMatrix(const std::string& key) const { try { std::string str = config_.at(key); + used_keys_.markUsed(key); Matrix matrix; auto node = parse_value(str); @@ -126,8 +129,12 @@ namespace corryvreckan { return def; } - template void Configuration::set(const std::string& key, const T& val) { + template void Configuration::set(const std::string& key, const T& val, bool mark_used) { config_[key] = corryvreckan::to_string(val); + used_keys_.registerMarker(key); + if(mark_used) { + used_keys_.markUsed(key); + } } template @@ -141,9 +148,10 @@ namespace corryvreckan { } ret_str.pop_back(); config_[key] = ret_str; + used_keys_.registerMarker(key); } - template void Configuration::setArray(const std::string& key, const std::vector& val) { + template void Configuration::setArray(const std::string& key, const std::vector& val, bool mark_used) { // NOTE: not the most elegant way to support arrays std::string str; for(T el : val) { @@ -152,6 +160,10 @@ namespace corryvreckan { } str.pop_back(); config_[key] = str; + used_keys_.registerMarker(key); + if(mark_used) { + used_keys_.markUsed(key); + } } template void Configuration::setMatrix(const std::string& key, const Matrix& val) { diff --git a/src/core/config/OptionParser.hpp b/src/core/config/OptionParser.hpp index 4cdbdb4af71a3864f3344ba83dec0f75f84b6e22..63f02d4be607519118842070d66a3c737c29002a 100644 --- a/src/core/config/OptionParser.hpp +++ b/src/core/config/OptionParser.hpp @@ -48,8 +48,8 @@ namespace corryvreckan { /** * @brief Use default move behaviour */ - OptionParser(OptionParser&&) noexcept = default; - OptionParser& operator=(OptionParser&&) noexcept = default; + OptionParser(OptionParser&&) noexcept = default; // NOLINT + OptionParser& operator=(OptionParser&&) = default; /// @} /** diff --git a/src/core/config/exceptions.cpp b/src/core/config/exceptions.cpp index 86c4267c985626ea346c8f9bc6d6261296ce3452..24e8e0a0ffcd8db686a4c9921b1a208c466aadca 100644 --- a/src/core/config/exceptions.cpp +++ b/src/core/config/exceptions.cpp @@ -32,7 +32,7 @@ InvalidCombinationError::InvalidCombinationError(const Configuration& config, section_str = "in global section"; } error_message_ = "Combination of keys "; - for(auto& key : keys) { + for(const auto& key : keys) { if(!config.has(key)) { continue; } diff --git a/src/core/detector/PixelDetector.cpp b/src/core/detector/PixelDetector.cpp index d83cab6af2a87b06c6b4cb7db01d6c23a8661b9e..b4cf7df8a63936d391db8c18faf5ebbaccd97818 100644 --- a/src/core/detector/PixelDetector.cpp +++ b/src/core/detector/PixelDetector.cpp @@ -186,10 +186,10 @@ void PixelDetector::configure_detector(Configuration& config) const { config.set("number_of_pixels", m_nPixels); // Size of the pixels - config.set("pixel_pitch", m_pitch, {"um"}); + config.set("pixel_pitch", m_pitch, {{"um"}}); // Intrinsic resolution: - config.set("spatial_resolution", m_spatial_resolution, {"um"}); + config.set("spatial_resolution", m_spatial_resolution, {{"um"}}); // Pixel mask file: if(!m_maskfile.empty()) { @@ -207,7 +207,7 @@ void PixelDetector::configure_detector(Configuration& config) const { void PixelDetector::configure_pos_and_orientation(Configuration& config) const { config.set("position", m_displacement, {"um", "mm"}); config.set("orientation_mode", m_orientation_mode); - config.set("orientation", m_orientation, {"deg"}); + config.set("orientation", m_orientation, {{"deg"}}); } // Function to get global intercept with a track diff --git a/src/core/module/ModuleManager.cpp b/src/core/module/ModuleManager.cpp index 3df3cd43226de6f9c722a3e8a5a95fdfa2d45f0e..029ccea3515f637f7e46f85d3138155779d908da 100644 --- a/src/core/module/ModuleManager.cpp +++ b/src/core/module/ModuleManager.cpp @@ -750,6 +750,34 @@ void ModuleManager::finalizeAll() { LOG(STATUS) << "Wrote updated detector configuration to " << path; } + // Check for unused configuration keys: + auto unused_keys = global_config.getUnusedKeys(); + if(!unused_keys.empty()) { + std::stringstream st; + st << "Unused configuration keys in global section:"; + for(auto& key : unused_keys) { + st << std::endl << key; + } + LOG(WARNING) << st.str(); + } + for(auto& config : conf_manager_->getInstanceConfigurations()) { + auto unique_name = config.getName(); + auto identifier = config.get("identifier"); + if(!identifier.empty()) { + unique_name += ":"; + unique_name += identifier; + } + auto cfg_unused_keys = config.getUnusedKeys(); + if(!cfg_unused_keys.empty()) { + std::stringstream st; + st << "Unused configuration keys in section " << unique_name << ":"; + for(auto& key : cfg_unused_keys) { + st << std::endl << key; + } + LOG(WARNING) << st.str(); + } + } + // Check the timing for all events timing(); }