diff --git a/src/core/config/ConfigManager.cpp b/src/core/config/ConfigManager.cpp index ba7db7465dabba729294a08b53a218660b638607..69b1ce1f0fdde2a52e0b5c8cb1cc27878e4777b1 100644 --- a/src/core/config/ConfigManager.cpp +++ b/src/core/config/ConfigManager.cpp @@ -163,26 +163,25 @@ std::list& ConfigManager::getDetectorConfigurations() { /** * @warning A previously stored configuration is directly invalidated if the same unique name is used again * - * An instance configuration is a specialized configuration for a particular module instance. If an unique name already - * exists the previous record is deleted and a new configuration record corresponding to the replaced instance is added. + * An instance configuration is a specialized configuration for a particular module instance. If a ModuleIdentifier already + * exists an error is thrown. */ Configuration& ConfigManager::addInstanceConfiguration(const ModuleIdentifier& identifier, const Configuration& config) { - std::string unique_name = identifier.getUniqueName(); // Check uniqueness - if(instance_name_to_config_.find(unique_name) != instance_name_to_config_.end()) { - instance_configs_.erase(instance_name_to_config_[unique_name]); + if(instance_identifier_to_config_.find(identifier) != instance_identifier_to_config_.end()) { + throw ModuleIdentifierAlreadyAddedError(identifier); } // Add configuration instance_configs_.push_back(config); Configuration& ret_config = instance_configs_.back(); - instance_name_to_config_[unique_name] = --instance_configs_.end(); + instance_identifier_to_config_[identifier] = --instance_configs_.end(); // Add identifier key to config ret_config.set("identifier", identifier.getIdentifier()); // Apply instance options - module_option_parser_.applyOptions(unique_name, ret_config); + module_option_parser_.applyOptions(identifier.getUniqueName(), ret_config); return ret_config; } @@ -193,3 +192,18 @@ Configuration& ConfigManager::addInstanceConfiguration(const ModuleIdentifier& i std::list& ConfigManager::getInstanceConfigurations() { return instance_configs_; } + +/** + * An instance configuration might be dropped when not used (e.g. it is overwritten by another module instance afterwards) + * We need to remove it from the instance configuration list to ensure dumping the config actually dumps only the instance + * configurations that were used. + */ +void ConfigManager::dropInstanceConfiguration(const ModuleIdentifier& identifier) { + // Remove config from instance configs and from instance identifier map + if(instance_identifier_to_config_.find(identifier) != instance_identifier_to_config_.end()) { + instance_configs_.erase(instance_identifier_to_config_[identifier]); + instance_identifier_to_config_.erase(identifier); + } else { + throw ModuleIdentifierNotFoundError(identifier); + } +} diff --git a/src/core/config/ConfigManager.hpp b/src/core/config/ConfigManager.hpp index be985ac94368722a9997bc52b0849ba7f338e128..6458a8c3c25b29dd4c9cc79158b860e4a5238123 100644 --- a/src/core/config/ConfigManager.hpp +++ b/src/core/config/ConfigManager.hpp @@ -90,6 +90,12 @@ namespace corryvreckan { */ std::list& getInstanceConfigurations(); + /** + * @brief Drops an instance configuration from instance configuration storage + * @param identifier Identifier of the module instance to drop + */ + void dropInstanceConfiguration(const ModuleIdentifier& identifier); + /** * @brief Load module options and directly apply them to the global configuration and the module configurations * @param options List of options to load and apply @@ -125,7 +131,7 @@ namespace corryvreckan { std::list detector_configs_; std::list instance_configs_; - std::map::iterator> instance_name_to_config_; + std::map::iterator> instance_identifier_to_config_; }; } // namespace corryvreckan diff --git a/src/core/config/exceptions.cpp b/src/core/config/exceptions.cpp index 348b852111ce51481ecdc10a1c9cce4592b6bb6a..3b25ff39f82103777fd5b86f7c6dae94fa90bcb1 100644 --- a/src/core/config/exceptions.cpp +++ b/src/core/config/exceptions.cpp @@ -10,6 +10,7 @@ #include "core/config/exceptions.h" #include "Configuration.hpp" +#include "core/module/ModuleIdentifier.hpp" using namespace corryvreckan; @@ -43,3 +44,13 @@ InvalidCombinationError::InvalidCombinationError(const Configuration& config, error_message_ += ": " + reason; } } + +ModuleIdentifierNotFoundError::ModuleIdentifierNotFoundError(const ModuleIdentifier& identifier) { + error_message_ = "Module Identifier " + identifier.getUniqueName() + ":" + std::to_string(identifier.getPriority()) + + " not found in the module identifier list"; +} + +ModuleIdentifierAlreadyAddedError::ModuleIdentifierAlreadyAddedError(const ModuleIdentifier& identifier) { + error_message_ = "Module Identifier " + identifier.getUniqueName() + ":" + std::to_string(identifier.getPriority()) + + " already added to the module identifier list"; +} diff --git a/src/core/config/exceptions.h b/src/core/config/exceptions.h index 615bc16d5c002ca4b6ce3df022503d66c4034aab..76b5119fec655f6843742510549a49340056ad2a 100644 --- a/src/core/config/exceptions.h +++ b/src/core/config/exceptions.h @@ -168,6 +168,24 @@ namespace corryvreckan { std::initializer_list keys, const std::string& reason = ""); }; + + class ModuleIdentifier; + /** + * @ingroup Exceptions + * @brief Indicates that a given ModuleIdentifier was not found in the module identifier list + */ + class ModuleIdentifierNotFoundError : public LogicError { + public: + explicit ModuleIdentifierNotFoundError(const ModuleIdentifier& identifier); + }; + /** + * @ingroup Exceptions + * @brief Indicates that a given ModuleIdentifier is already in the module identifier list + */ + class ModuleIdentifierAlreadyAddedError : public LogicError { + public: + explicit ModuleIdentifierAlreadyAddedError(const ModuleIdentifier& identifier); + }; } // namespace corryvreckan #endif /* CORRYVRECKAN_CONFIG_EXCEPTIONS_H */ diff --git a/src/core/module/ModuleIdentifier.hpp b/src/core/module/ModuleIdentifier.hpp index 3cd7f7ab4dc85d3c1d79f354762738c7c5670580..0ba9e083e7818e4df8aff99a49cec41f9dd3991a 100644 --- a/src/core/module/ModuleIdentifier.hpp +++ b/src/core/module/ModuleIdentifier.hpp @@ -11,7 +11,6 @@ #define CORRYVRECKAN_MODULE_IDENTIFIER_H #include -#include #include #include @@ -74,14 +73,30 @@ namespace corryvreckan { /** * @brief Operators for comparing identifiers * - * Identifiers are only compared on their unique name, identifiers are not distinguished on priorities + * Identifiers are compared on their unique name and their priorities */ - bool operator==(const ModuleIdentifier& other) const { return getUniqueName() == other.getUniqueName(); } - bool operator!=(const ModuleIdentifier& other) const { return getUniqueName() != other.getUniqueName(); } - bool operator<(const ModuleIdentifier& other) const { return getUniqueName() < other.getUniqueName(); } - bool operator<=(const ModuleIdentifier& other) const { return getUniqueName() <= other.getUniqueName(); } - bool operator>(const ModuleIdentifier& other) const { return getUniqueName() > other.getUniqueName(); } - bool operator>=(const ModuleIdentifier& other) const { return getUniqueName() >= other.getUniqueName(); } + bool operator==(const ModuleIdentifier& other) const { + return getUniqueName() == other.getUniqueName() && getPriority() == other.getPriority(); + } + bool operator!=(const ModuleIdentifier& other) const { + return getUniqueName() != other.getUniqueName() || getPriority() != other.getPriority(); + } + bool operator<(const ModuleIdentifier& other) const { + return getUniqueName() != other.getUniqueName() ? getUniqueName() < other.getUniqueName() + : getPriority() < other.getPriority(); + } + bool operator<=(const ModuleIdentifier& other) const { + return getUniqueName() != other.getUniqueName() ? getUniqueName() <= other.getUniqueName() + : getPriority() <= other.getPriority(); + } + bool operator>(const ModuleIdentifier& other) const { + return getUniqueName() != other.getUniqueName() ? getUniqueName() > other.getUniqueName() + : getPriority() > other.getPriority(); + } + bool operator>=(const ModuleIdentifier& other) const { + return getUniqueName() != other.getUniqueName() ? getUniqueName() >= other.getUniqueName() + : getPriority() >= other.getPriority(); + } /// @} private: diff --git a/src/core/module/ModuleManager.cpp b/src/core/module/ModuleManager.cpp index 37a80ee283d2fe481d6e8c427b56b2efa3680e67..141805bd6812fc8ddfa203f5b67234b8455369a7 100644 --- a/src/core/module/ModuleManager.cpp +++ b/src/core/module/ModuleManager.cpp @@ -258,7 +258,9 @@ void ModuleManager::load_modules() { ModuleIdentifier identifier = id_mod.first; // Check if the unique instantiation already exists - auto iter = id_to_module_.find(identifier); + auto iter = std::find_if(id_to_module_.begin(), id_to_module_.end(), [&identifier](const auto& mapv) { + return identifier.getUniqueName() == mapv.first.getUniqueName(); + }); if(iter != id_to_module_.end()) { // Unique name exists, check if its needs to be replaced if(identifier.getPriority() < iter->first.getPriority()) { @@ -266,6 +268,9 @@ void ModuleManager::load_modules() { LOG(TRACE) << "Replacing model instance " << iter->first.getUniqueName() << " with instance with higher priority."; + // Drop configuration from replaced module + conf_manager_->dropInstanceConfiguration(iter->first); + module_execution_time_.erase(iter->second->get()); iter->second = m_modules.erase(iter->second); iter = id_to_module_.erase(iter); @@ -274,7 +279,9 @@ void ModuleManager::load_modules() { if(identifier.getPriority() == iter->first.getPriority()) { throw AmbiguousInstantiationError(config.getName()); } - // Priority is lower, do not add this module to the run list + // Priority is lower, do not add this module to the run list, drop config + conf_manager_->dropInstanceConfiguration(identifier); + module_execution_time_.erase(id_mod.second); continue; } } @@ -292,6 +299,18 @@ void ModuleManager::load_modules() { LOG_PROGRESS(STATUS, "MOD_LOAD_LOOP") << "Loaded " << m_modules.size() << " module instances"; } +/** + * Calls config_manager->addInstanceConfiguration(identifier, config) while handling ModuleIdentifierAlreadyAddedError + */ +static inline Configuration& +add_instance_configuration(ConfigManager* config_manager, const ModuleIdentifier& identifier, const Configuration& config) { + try { + return config_manager->addInstanceConfiguration(identifier, config); + } catch(ModuleIdentifierAlreadyAddedError&) { + throw AmbiguousInstantiationError(identifier.getUniqueName()); + } +} + std::vector ModuleManager::get_type_vector(char* type_tokens) { std::vector types; @@ -339,7 +358,7 @@ std::pair ModuleManager::create_unique_module(void* l } // Create and add module instance config - Configuration& instance_config = conf_manager_->addInstanceConfiguration(identifier, config); + Configuration& instance_config = add_instance_configuration(conf_manager_, identifier, config); // Specialize instance configuration auto output_dir = instance_config.get("_global_dir"); @@ -487,7 +506,7 @@ std::vector> ModuleManager::create_detector LOG(DEBUG) << "Creating instantiation \"" << identifier.getUniqueName() << "\""; // Create and add module instance config - Configuration& instance_config = conf_manager_->addInstanceConfiguration(instance.second, config); + Configuration& instance_config = add_instance_configuration(conf_manager_, instance.second, config); // Add internal module config auto output_dir = instance_config.get("_global_dir");