From bd3565ac69f4459019680096ac9815b8458513a3 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 19 May 2026 16:53:54 +1200 Subject: [PATCH 01/36] Investigations into cahcing equivalent variables lookup. --- src/CMakeLists.txt | 1 + src/internaltypes.h | 26 +++- src/printer.cpp | 4 +- src/unionfind.h | 60 +++++++++ src/validator.cpp | 108 ++++++++++++++- tests/CMakeLists.txt | 1 + tests/investigations/investigations.cpp | 172 ++++++++++++++++++++++++ tests/investigations/tests.cmake | 18 +++ tests/test_utils.cpp | 10 +- tests/test_utils.h | 2 +- 10 files changed, 389 insertions(+), 13 deletions(-) create mode 100644 src/unionfind.h create mode 100644 tests/investigations/investigations.cpp create mode 100644 tests/investigations/tests.cmake diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 59caf0233b..bbc9cb281e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -149,6 +149,7 @@ set(GIT_HEADER_FILES ${CMAKE_CURRENT_SOURCE_DIR}/namespaces.h ${CMAKE_CURRENT_SOURCE_DIR}/parentedentity_p.h ${CMAKE_CURRENT_SOURCE_DIR}/reset_p.h + ${CMAKE_CURRENT_SOURCE_DIR}/unionfind.h ${CMAKE_CURRENT_SOURCE_DIR}/units_p.h ${CMAKE_CURRENT_SOURCE_DIR}/utilities.h ${CMAKE_CURRENT_SOURCE_DIR}/variable_p.h diff --git a/src/internaltypes.h b/src/internaltypes.h index e21e35ed67..dcb67919b9 100644 --- a/src/internaltypes.h +++ b/src/internaltypes.h @@ -42,13 +42,14 @@ using UniqueNames = std::set; /**< Type definition for a set of uni using NodeAttributeNamespaceInfo = std::vector>; /**< Type definition for attribute namespace information. */ // VariableMap +using VariableStdPair = std::pair; /**< Type definition for Variable pointer pair using standard libary. */ using VariableMap = std::vector; /**< Type definition for vector of VariablePair.*/ using VariableMapIterator = VariableMap::const_iterator; /**< Type definition of const iterator for vector of VariablePair.*/ // ComponentMap -using ComponentPair = std::pair; /**< Type definition for Component pointer pair.*/ -using ComponentMap = std::vector; /**< Type definition for vector of ComponentPair.*/ -using ComponentMapIterator = ComponentMap::const_iterator; /**< Type definition of const iterator for vector of ComponentPair.*/ +using ComponentStdPair = std::pair; /**< Type definition for Component pointer pair using standard library.*/ +using ComponentMap = std::vector; /**< Type definition for vector of ComponentStdPair.*/ +using ComponentMapIterator = ComponentMap::const_iterator; /**< Type definition of const iterator for vector of ComponentStdPair.*/ using VariablePtrs = std::vector; /**< Type definition for list of variables. */ @@ -79,6 +80,25 @@ using UnitsConstPtr = std::shared_ptr; /**< Type definition for sha using ConnectionMap = std::map; /**< Type definition for a connection map.*/ using NamePairList = std::vector; /**< Type definition for a list of a pair of names. */ +using ComponentRawPtrPair = std::pair; +using ConnectionIdMap = std::map; + +struct ComponentPair { + + bool operator==(const ComponentPair& other) const { + return c1 == other.c1 && c2 == other.c2; + } + + ComponentPtr c1; + ComponentPtr c2; +}; + +struct ComponentPairHash { + size_t operator()(const ComponentPair& p) const { + return std::hash()(p.c1) ^ std::hash()(p.c2); + } +}; + /** * @brief Class for defining an epoch in the history of a @ref Component or @ref Units. * diff --git a/src/printer.cpp b/src/printer.cpp index 6a39d3a5f5..d88bed4e1d 100644 --- a/src/printer.cpp +++ b/src/printer.cpp @@ -77,7 +77,7 @@ std::string printConnections(const ComponentMap &componentMap, const VariableMap for (auto iterPair = componentMap.begin(); iterPair < componentMap.end(); ++iterPair) { ComponentPtr currentComponent1 = iterPair->first; ComponentPtr currentComponent2 = iterPair->second; - ComponentPair currentComponentPair = std::make_pair(currentComponent1, currentComponent2); + ComponentStdPair currentComponentPair = std::make_pair(currentComponent1, currentComponent2); // Check whether this set of connections has already been serialised. bool pairFound = false; for (const auto &serialisedIterPair : serialisedComponentMap) { @@ -178,7 +178,7 @@ void buildMapsForComponentsVariables(const ComponentPtr &component, ComponentMap ComponentPtr component1 = owningComponent(variable); ComponentPtr component2 = owningComponent(equivalentVariable); // Also create a component map pair corresponding with the variable map pair. - ComponentPair componentPair = std::make_pair(component1, component2); + ComponentStdPair componentPair = std::make_pair(component1, component2); componentMap.push_back(componentPair); } } diff --git a/src/unionfind.h b/src/unionfind.h new file mode 100644 index 0000000000..f421fb3504 --- /dev/null +++ b/src/unionfind.h @@ -0,0 +1,60 @@ +/* +Copyright libCellML Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include + +template +class UnionFind { +public: + T find(const T& x) { + auto it = parent.find(x); + if (it == parent.end()) { + parent[x] = x; + rank[x] = 0; + return x; + } + + if (it->second != x) { + it->second = find(it->second); // Path compression + } + return it->second; + } + + void unite(const T& a, const T& b) { + T rootA = find(a); + T rootB = find(b); + + if (rootA == rootB) return; + + // Union by rank + if (rank[rootA] < rank[rootB]) { + parent[rootA] = rootB; + } else if (rank[rootA] > rank[rootB]) { + parent[rootB] = rootA; + } else { + parent[rootB] = rootA; + rank[rootA]++; + } + } + + bool connected(const T& a, const T& b) { + return find(a) == find(b); + } + +private: + std::unordered_map parent; + std::unordered_map rank; +}; diff --git a/src/validator.cpp b/src/validator.cpp index 55a1db1186..976740f515 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -35,6 +35,7 @@ limitations under the License. #include "issue_p.h" #include "logger_p.h" #include "namespaces.h" +#include "unionfind.h" #include "utilities.h" #include "xmldoc.h" #include "xmlutils.h" @@ -639,8 +640,9 @@ class Validator::ValidatorImpl: public LoggerImpl * @param component The component to check. * @param idMap The IdMap object to construct. * @param reportedConnections A set of connection identifiers to prevent duplicate reporting. + * @param connectionIds A map of connection identifiers to prevent duplicate reporting of connections. */ - void buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set &reportedConnections); + void buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set &reportedConnections, const ConnectionIdMap &connectionIds); /** @brief Utility function to add an item to the idMap. * @@ -2692,11 +2694,102 @@ void Validator::ValidatorImpl::addIdMapItem(const std::string &id, const std::st } } +void gatherComponents(const ComponentPtr &component, std::vector &allComponents) +{ + allComponents.push_back(component); + for (size_t c = 0; c < component->componentCount(); ++c) { + gatherComponents(component->component(c), allComponents); + } +} + IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) { + UnionFind uf; + + // Traverse all components and variables + for (size_t c = 0; c < model->componentCount(); ++c) { + auto component = model->component(c); + + for (size_t i = 0; i < component->variableCount(); ++i) { + auto v = component->variable(i); + + for (size_t e = 0; e < v->equivalentVariableCount(); ++e) { + auto equiv = v->equivalentVariable(e); + + if (equiv != nullptr) { + uf.unite(v, equiv); + } + } + } + } + + std::unordered_map> groups; + for (size_t c = 0; c < model->componentCount(); ++c) { + auto component = model->component(c); + + for (size_t i = 0; i < component->variableCount(); ++i) { + auto v = component->variable(i); + auto root = uf.find(v); + + groups[root].push_back(v); + } + } + + std::unordered_map, ComponentPairHash> connectionMap; + // using VarPair>, ComponentPairHash> connectionMap; + + // for (auto& [root, vars] : groups) { + // for (size_t i = 0; i < vars.size(); ++i) { + // for (size_t j = i + 1; j < vars.size(); ++j) { + // auto v1 = vars[i]; + // auto v2 = vars[j]; + + // auto c1 = owningComponent(v1); + // auto c2 = owningComponent(v2); + + // if (!c1 || !c2 || c1 == c2) continue; + + // // Normalize ordering + // ComponentPair key = (c1 < c2) + // ? ComponentPair{c1, c2} + // : ComponentPair{c2, c1}; + + // connectionMap[key].emplace_back(v1, v2); + // } + // } + // } + IdMap idMap; std::string info; std::set reportedConnections; + + std::vector allComponents; + for (size_t c = 0; c < model->componentCount(); ++c) { + gatherComponents(model->component(c), allComponents); + } + + ConnectionIdMap connectionIds; + for (const auto &comp : allComponents) { + for (size_t i = 0; i < comp->variableCount(); ++i) { + auto item = comp->variable(i); + for (size_t e = 0; e < item->equivalentVariableCount(); ++e) { + auto equiv = item->equivalentVariable(e); + auto equivParent = owningComponent(equiv); + if (equivParent != nullptr) { + // Normalize the key order (min pointer first, max pointer second) + auto key = comp.get() < equivParent.get() + ? std::make_pair(comp.get(), equivParent.get()) + : std::make_pair(equivParent.get(), comp.get()); + + // If we haven't processed this component connection yet, do it once + if (connectionIds.find(key) == connectionIds.end()) { + connectionIds[key] = ""; //Variable::equivalenceConnectionId(item, equiv); + } + } + } + } + } + // Model. if (!model->id().empty()) { info = " - model '" + model->name() + "'"; @@ -2748,12 +2841,12 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) // Start recursion through encapsulation hierarchy. for (size_t c = 0; c < model->componentCount(); ++c) { - buildComponentIdMap(model->component(c), idMap, reportedConnections); + buildComponentIdMap(model->component(c), idMap, reportedConnections, connectionIds); } return idMap; } -void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set &reportedConnections) +void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set &reportedConnections, const ConnectionIdMap &connectionIds) { std::string info; @@ -2807,7 +2900,12 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component addIdMapItem(mappingId, info, idMap); } // Connections. - auto connectionId = Variable::equivalenceConnectionId(item, equiv); + auto key = component.get() < equivParent.get() + ? std::make_pair(component.get(), equivParent.get()) + : std::make_pair(equivParent.get(), component.get()); + + auto connectionId = connectionIds.at(key); + // auto connectionId = Variable::equivalenceConnectionId(item, equiv); std::string connection = component->name() < equivParent->name() ? component->name() + equivParent->name() : equivParent->name() + component->name(); if ((s1 < s2) && !connectionId.empty() && (reportedConnections.count(connection) == 0)) { std::string connectionDescription = @@ -2879,7 +2977,7 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component // Child components. for (size_t c = 0; c < component->componentCount(); ++c) { - buildComponentIdMap(component->component(c), idMap, reportedConnections); + buildComponentIdMap(component->component(c), idMap, reportedConnections, connectionIds); } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1dbf26da87..b66e31cdc5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -47,6 +47,7 @@ include(units/tests.cmake) include(validator/tests.cmake) include(variable/tests.cmake) include(version/tests.cmake) +include(investigations/tests.cmake) set(TEST_EXPORTDEFINITIONS_H "${CMAKE_CURRENT_BINARY_DIR}/test_exportdefinitions.h") diff --git a/tests/investigations/investigations.cpp b/tests/investigations/investigations.cpp new file mode 100644 index 0000000000..d5490872b8 --- /dev/null +++ b/tests/investigations/investigations.cpp @@ -0,0 +1,172 @@ +/* +Copyright libCellML Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include "gtest/gtest.h" +#include + +#include + +#include "test_utils.h" + +const char* BENCHMARKING_MODEL_ROOT = std::getenv("BENCHMARKING_MODEL_ROOT"); + +TEST(Investigations, exponentialTimeConsumption11) +{ + const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/image_to_model.cellml"; + const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/"; + auto importer = libcellml::Importer::create(false); + + auto parser = libcellml::Parser::create(false); + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + Debug() << modelPath; + Debug() << parser->issue(0)->description(); + Debug() << originalModel->name(); + + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(3), importer->issueCount()); + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + auto analyser = libcellml::Analyser::create(); + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + printIssues(analyser); +} + +TEST(Investigations, DISABLED_exponentialTimeConsumption246) +{ + const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_246_vessels/image_to_model.cellml"; + const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_246_vessels/"; + auto importer = libcellml::Importer::create(false); + + auto parser = libcellml::Parser::create(false); + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + Debug() << parser->issue(0)->description(); + Debug() << originalModel->name(); + + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(3), importer->issueCount()); + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + auto analyser = libcellml::Analyser::create(); + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + printIssues(analyser); +} + +TEST(Investigations, DISABLED_exponentialTimeConsumption380) +{ + const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_380_vessels/image_to_model.cellml"; + const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_380_vessels/"; + auto importer = libcellml::Importer::create(false); + + auto parser = libcellml::Parser::create(false); + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + Debug() << modelPath; + Debug() << parser->issue(0)->description(); + Debug() << originalModel->name(); + + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(3), importer->issueCount()); + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + auto analyser = libcellml::Analyser::create(); + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + printIssues(analyser); +} + +TEST(Investigations, DISABLED_exponentialTimeConsumption524) +{ + const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/image_to_model.cellml"; + const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/"; + auto importer = libcellml::Importer::create(false); + + auto parser = libcellml::Parser::create(false); + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + Debug() << modelPath; + Debug() << parser->issue(0)->description(); + Debug() << originalModel->name(); + + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(3), importer->issueCount()); + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + auto analyser = libcellml::Analyser::create(); + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + printIssues(analyser); +} + + +TEST(Investigations, DISABLED_exponentialTimeConsumptionOthers) +{ + const std::vector vesselCounts = { 246, 380, 524 }; + + auto importer = libcellml::Importer::create(false); + auto parser = libcellml::Parser::create(false); + auto printer = libcellml::Printer::create(); + auto analyser = libcellml::Analyser::create(); + auto generator = libcellml::Generator::create(); + + for (int vesselCount : vesselCounts) { + SCOPED_TRACE("Vessel count: " + std::to_string(vesselCount)); + std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_" + std::to_string(vesselCount) + "_vessels/image_to_model.cellml"; + std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_" + std::to_string(vesselCount) + "_vessels/"; + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(0), importer->issueCount()); + for (size_t i = 0; i < importer->issueCount(); ++i) { + Debug() << "[" << i << "]: " << importer->issue(i)->description(); + } + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + + const auto implementationCode = generator->implementationCode(analyser->analyserModel()); + EXPECT_NE(size_t(0), implementationCode.length()); + } +} diff --git a/tests/investigations/tests.cmake b/tests/investigations/tests.cmake new file mode 100644 index 0000000000..1b01f81fa1 --- /dev/null +++ b/tests/investigations/tests.cmake @@ -0,0 +1,18 @@ + +# Set the test name, 'test_' will be prepended to the +# name set here +set(CURRENT_TEST investigations) +# Set a category name to enable running commands like: +# ctest -R +# which will run the tests matching this category-label. +# Can be left empty (or just not set) +set(${CURRENT_TEST}_CATEGORY misc) +list(APPEND LIBCELLML_TESTS ${CURRENT_TEST}) +# Using absolute path relative to this file +set(${CURRENT_TEST}_SRCS + ${CMAKE_CURRENT_LIST_DIR}/investigations.cpp +) +set(${CURRENT_TEST}_HDRS +) + + diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 21f57ed540..27fa619c3e 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -35,12 +35,18 @@ std::string resourcePath(const std::string &resourceRelativePath) return TESTS_RESOURCE_LOCATION + "/" + resourceRelativePath; } -std::string fileContents(const std::string &fileName) +std::string fileContents(const std::string &fileName, bool absoulte) { - std::ifstream file(resourcePath(fileName)); + std::ifstream file; + if (absoulte) { + file.open(fileName); + } else { + file.open(resourcePath(fileName)); + } std::stringstream buffer; buffer << file.rdbuf(); + file.close(); return buffer.str(); } diff --git a/tests/test_utils.h b/tests/test_utils.h index 1199138682..c9c6347e92 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -82,7 +82,7 @@ std::chrono::steady_clock::time_point TEST_EXPORT timeNow(); int TEST_EXPORT elapsedTime(const std::chrono::steady_clock::time_point &startTime); std::string TEST_EXPORT resourcePath(const std::string &resourceRelativePath = ""); -std::string TEST_EXPORT fileContents(const std::string &fileName); +std::string TEST_EXPORT fileContents(const std::string &fileName, bool absoulte=false); void TEST_EXPORT printIssues(const libcellml::LoggerPtr &l, bool headings = false, bool cellmlElementTypes = false, bool rule = false); void TEST_EXPORT printModel(const libcellml::ModelPtr &model, bool includeMaths = true); From ea06360c40b74074446c9436bf3220f9f25ea314 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 19 May 2026 16:53:54 +1200 Subject: [PATCH 02/36] Investigations into cahcing equivalent variables lookup. --- src/CMakeLists.txt | 1 + src/internaltypes.h | 26 +++- src/printer.cpp | 4 +- src/unionfind.h | 60 +++++++++ src/validator.cpp | 108 ++++++++++++++- tests/CMakeLists.txt | 1 + tests/investigations/investigations.cpp | 172 ++++++++++++++++++++++++ tests/investigations/tests.cmake | 18 +++ tests/test_utils.cpp | 10 +- tests/test_utils.h | 2 +- 10 files changed, 389 insertions(+), 13 deletions(-) create mode 100644 src/unionfind.h create mode 100644 tests/investigations/investigations.cpp create mode 100644 tests/investigations/tests.cmake diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 59caf0233b..bbc9cb281e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -149,6 +149,7 @@ set(GIT_HEADER_FILES ${CMAKE_CURRENT_SOURCE_DIR}/namespaces.h ${CMAKE_CURRENT_SOURCE_DIR}/parentedentity_p.h ${CMAKE_CURRENT_SOURCE_DIR}/reset_p.h + ${CMAKE_CURRENT_SOURCE_DIR}/unionfind.h ${CMAKE_CURRENT_SOURCE_DIR}/units_p.h ${CMAKE_CURRENT_SOURCE_DIR}/utilities.h ${CMAKE_CURRENT_SOURCE_DIR}/variable_p.h diff --git a/src/internaltypes.h b/src/internaltypes.h index e21e35ed67..dcb67919b9 100644 --- a/src/internaltypes.h +++ b/src/internaltypes.h @@ -42,13 +42,14 @@ using UniqueNames = std::set; /**< Type definition for a set of uni using NodeAttributeNamespaceInfo = std::vector>; /**< Type definition for attribute namespace information. */ // VariableMap +using VariableStdPair = std::pair; /**< Type definition for Variable pointer pair using standard libary. */ using VariableMap = std::vector; /**< Type definition for vector of VariablePair.*/ using VariableMapIterator = VariableMap::const_iterator; /**< Type definition of const iterator for vector of VariablePair.*/ // ComponentMap -using ComponentPair = std::pair; /**< Type definition for Component pointer pair.*/ -using ComponentMap = std::vector; /**< Type definition for vector of ComponentPair.*/ -using ComponentMapIterator = ComponentMap::const_iterator; /**< Type definition of const iterator for vector of ComponentPair.*/ +using ComponentStdPair = std::pair; /**< Type definition for Component pointer pair using standard library.*/ +using ComponentMap = std::vector; /**< Type definition for vector of ComponentStdPair.*/ +using ComponentMapIterator = ComponentMap::const_iterator; /**< Type definition of const iterator for vector of ComponentStdPair.*/ using VariablePtrs = std::vector; /**< Type definition for list of variables. */ @@ -79,6 +80,25 @@ using UnitsConstPtr = std::shared_ptr; /**< Type definition for sha using ConnectionMap = std::map; /**< Type definition for a connection map.*/ using NamePairList = std::vector; /**< Type definition for a list of a pair of names. */ +using ComponentRawPtrPair = std::pair; +using ConnectionIdMap = std::map; + +struct ComponentPair { + + bool operator==(const ComponentPair& other) const { + return c1 == other.c1 && c2 == other.c2; + } + + ComponentPtr c1; + ComponentPtr c2; +}; + +struct ComponentPairHash { + size_t operator()(const ComponentPair& p) const { + return std::hash()(p.c1) ^ std::hash()(p.c2); + } +}; + /** * @brief Class for defining an epoch in the history of a @ref Component or @ref Units. * diff --git a/src/printer.cpp b/src/printer.cpp index 6a39d3a5f5..d88bed4e1d 100644 --- a/src/printer.cpp +++ b/src/printer.cpp @@ -77,7 +77,7 @@ std::string printConnections(const ComponentMap &componentMap, const VariableMap for (auto iterPair = componentMap.begin(); iterPair < componentMap.end(); ++iterPair) { ComponentPtr currentComponent1 = iterPair->first; ComponentPtr currentComponent2 = iterPair->second; - ComponentPair currentComponentPair = std::make_pair(currentComponent1, currentComponent2); + ComponentStdPair currentComponentPair = std::make_pair(currentComponent1, currentComponent2); // Check whether this set of connections has already been serialised. bool pairFound = false; for (const auto &serialisedIterPair : serialisedComponentMap) { @@ -178,7 +178,7 @@ void buildMapsForComponentsVariables(const ComponentPtr &component, ComponentMap ComponentPtr component1 = owningComponent(variable); ComponentPtr component2 = owningComponent(equivalentVariable); // Also create a component map pair corresponding with the variable map pair. - ComponentPair componentPair = std::make_pair(component1, component2); + ComponentStdPair componentPair = std::make_pair(component1, component2); componentMap.push_back(componentPair); } } diff --git a/src/unionfind.h b/src/unionfind.h new file mode 100644 index 0000000000..f421fb3504 --- /dev/null +++ b/src/unionfind.h @@ -0,0 +1,60 @@ +/* +Copyright libCellML Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include + +template +class UnionFind { +public: + T find(const T& x) { + auto it = parent.find(x); + if (it == parent.end()) { + parent[x] = x; + rank[x] = 0; + return x; + } + + if (it->second != x) { + it->second = find(it->second); // Path compression + } + return it->second; + } + + void unite(const T& a, const T& b) { + T rootA = find(a); + T rootB = find(b); + + if (rootA == rootB) return; + + // Union by rank + if (rank[rootA] < rank[rootB]) { + parent[rootA] = rootB; + } else if (rank[rootA] > rank[rootB]) { + parent[rootB] = rootA; + } else { + parent[rootB] = rootA; + rank[rootA]++; + } + } + + bool connected(const T& a, const T& b) { + return find(a) == find(b); + } + +private: + std::unordered_map parent; + std::unordered_map rank; +}; diff --git a/src/validator.cpp b/src/validator.cpp index 55a1db1186..976740f515 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -35,6 +35,7 @@ limitations under the License. #include "issue_p.h" #include "logger_p.h" #include "namespaces.h" +#include "unionfind.h" #include "utilities.h" #include "xmldoc.h" #include "xmlutils.h" @@ -639,8 +640,9 @@ class Validator::ValidatorImpl: public LoggerImpl * @param component The component to check. * @param idMap The IdMap object to construct. * @param reportedConnections A set of connection identifiers to prevent duplicate reporting. + * @param connectionIds A map of connection identifiers to prevent duplicate reporting of connections. */ - void buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set &reportedConnections); + void buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set &reportedConnections, const ConnectionIdMap &connectionIds); /** @brief Utility function to add an item to the idMap. * @@ -2692,11 +2694,102 @@ void Validator::ValidatorImpl::addIdMapItem(const std::string &id, const std::st } } +void gatherComponents(const ComponentPtr &component, std::vector &allComponents) +{ + allComponents.push_back(component); + for (size_t c = 0; c < component->componentCount(); ++c) { + gatherComponents(component->component(c), allComponents); + } +} + IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) { + UnionFind uf; + + // Traverse all components and variables + for (size_t c = 0; c < model->componentCount(); ++c) { + auto component = model->component(c); + + for (size_t i = 0; i < component->variableCount(); ++i) { + auto v = component->variable(i); + + for (size_t e = 0; e < v->equivalentVariableCount(); ++e) { + auto equiv = v->equivalentVariable(e); + + if (equiv != nullptr) { + uf.unite(v, equiv); + } + } + } + } + + std::unordered_map> groups; + for (size_t c = 0; c < model->componentCount(); ++c) { + auto component = model->component(c); + + for (size_t i = 0; i < component->variableCount(); ++i) { + auto v = component->variable(i); + auto root = uf.find(v); + + groups[root].push_back(v); + } + } + + std::unordered_map, ComponentPairHash> connectionMap; + // using VarPair>, ComponentPairHash> connectionMap; + + // for (auto& [root, vars] : groups) { + // for (size_t i = 0; i < vars.size(); ++i) { + // for (size_t j = i + 1; j < vars.size(); ++j) { + // auto v1 = vars[i]; + // auto v2 = vars[j]; + + // auto c1 = owningComponent(v1); + // auto c2 = owningComponent(v2); + + // if (!c1 || !c2 || c1 == c2) continue; + + // // Normalize ordering + // ComponentPair key = (c1 < c2) + // ? ComponentPair{c1, c2} + // : ComponentPair{c2, c1}; + + // connectionMap[key].emplace_back(v1, v2); + // } + // } + // } + IdMap idMap; std::string info; std::set reportedConnections; + + std::vector allComponents; + for (size_t c = 0; c < model->componentCount(); ++c) { + gatherComponents(model->component(c), allComponents); + } + + ConnectionIdMap connectionIds; + for (const auto &comp : allComponents) { + for (size_t i = 0; i < comp->variableCount(); ++i) { + auto item = comp->variable(i); + for (size_t e = 0; e < item->equivalentVariableCount(); ++e) { + auto equiv = item->equivalentVariable(e); + auto equivParent = owningComponent(equiv); + if (equivParent != nullptr) { + // Normalize the key order (min pointer first, max pointer second) + auto key = comp.get() < equivParent.get() + ? std::make_pair(comp.get(), equivParent.get()) + : std::make_pair(equivParent.get(), comp.get()); + + // If we haven't processed this component connection yet, do it once + if (connectionIds.find(key) == connectionIds.end()) { + connectionIds[key] = ""; //Variable::equivalenceConnectionId(item, equiv); + } + } + } + } + } + // Model. if (!model->id().empty()) { info = " - model '" + model->name() + "'"; @@ -2748,12 +2841,12 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) // Start recursion through encapsulation hierarchy. for (size_t c = 0; c < model->componentCount(); ++c) { - buildComponentIdMap(model->component(c), idMap, reportedConnections); + buildComponentIdMap(model->component(c), idMap, reportedConnections, connectionIds); } return idMap; } -void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set &reportedConnections) +void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set &reportedConnections, const ConnectionIdMap &connectionIds) { std::string info; @@ -2807,7 +2900,12 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component addIdMapItem(mappingId, info, idMap); } // Connections. - auto connectionId = Variable::equivalenceConnectionId(item, equiv); + auto key = component.get() < equivParent.get() + ? std::make_pair(component.get(), equivParent.get()) + : std::make_pair(equivParent.get(), component.get()); + + auto connectionId = connectionIds.at(key); + // auto connectionId = Variable::equivalenceConnectionId(item, equiv); std::string connection = component->name() < equivParent->name() ? component->name() + equivParent->name() : equivParent->name() + component->name(); if ((s1 < s2) && !connectionId.empty() && (reportedConnections.count(connection) == 0)) { std::string connectionDescription = @@ -2879,7 +2977,7 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component // Child components. for (size_t c = 0; c < component->componentCount(); ++c) { - buildComponentIdMap(component->component(c), idMap, reportedConnections); + buildComponentIdMap(component->component(c), idMap, reportedConnections, connectionIds); } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1dbf26da87..b66e31cdc5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -47,6 +47,7 @@ include(units/tests.cmake) include(validator/tests.cmake) include(variable/tests.cmake) include(version/tests.cmake) +include(investigations/tests.cmake) set(TEST_EXPORTDEFINITIONS_H "${CMAKE_CURRENT_BINARY_DIR}/test_exportdefinitions.h") diff --git a/tests/investigations/investigations.cpp b/tests/investigations/investigations.cpp new file mode 100644 index 0000000000..d5490872b8 --- /dev/null +++ b/tests/investigations/investigations.cpp @@ -0,0 +1,172 @@ +/* +Copyright libCellML Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include "gtest/gtest.h" +#include + +#include + +#include "test_utils.h" + +const char* BENCHMARKING_MODEL_ROOT = std::getenv("BENCHMARKING_MODEL_ROOT"); + +TEST(Investigations, exponentialTimeConsumption11) +{ + const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/image_to_model.cellml"; + const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/"; + auto importer = libcellml::Importer::create(false); + + auto parser = libcellml::Parser::create(false); + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + Debug() << modelPath; + Debug() << parser->issue(0)->description(); + Debug() << originalModel->name(); + + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(3), importer->issueCount()); + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + auto analyser = libcellml::Analyser::create(); + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + printIssues(analyser); +} + +TEST(Investigations, DISABLED_exponentialTimeConsumption246) +{ + const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_246_vessels/image_to_model.cellml"; + const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_246_vessels/"; + auto importer = libcellml::Importer::create(false); + + auto parser = libcellml::Parser::create(false); + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + Debug() << parser->issue(0)->description(); + Debug() << originalModel->name(); + + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(3), importer->issueCount()); + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + auto analyser = libcellml::Analyser::create(); + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + printIssues(analyser); +} + +TEST(Investigations, DISABLED_exponentialTimeConsumption380) +{ + const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_380_vessels/image_to_model.cellml"; + const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_380_vessels/"; + auto importer = libcellml::Importer::create(false); + + auto parser = libcellml::Parser::create(false); + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + Debug() << modelPath; + Debug() << parser->issue(0)->description(); + Debug() << originalModel->name(); + + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(3), importer->issueCount()); + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + auto analyser = libcellml::Analyser::create(); + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + printIssues(analyser); +} + +TEST(Investigations, DISABLED_exponentialTimeConsumption524) +{ + const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/image_to_model.cellml"; + const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/"; + auto importer = libcellml::Importer::create(false); + + auto parser = libcellml::Parser::create(false); + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + Debug() << modelPath; + Debug() << parser->issue(0)->description(); + Debug() << originalModel->name(); + + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(3), importer->issueCount()); + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + auto analyser = libcellml::Analyser::create(); + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + printIssues(analyser); +} + + +TEST(Investigations, DISABLED_exponentialTimeConsumptionOthers) +{ + const std::vector vesselCounts = { 246, 380, 524 }; + + auto importer = libcellml::Importer::create(false); + auto parser = libcellml::Parser::create(false); + auto printer = libcellml::Printer::create(); + auto analyser = libcellml::Analyser::create(); + auto generator = libcellml::Generator::create(); + + for (int vesselCount : vesselCounts) { + SCOPED_TRACE("Vessel count: " + std::to_string(vesselCount)); + std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_" + std::to_string(vesselCount) + "_vessels/image_to_model.cellml"; + std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_" + std::to_string(vesselCount) + "_vessels/"; + auto originalModel = parser->parseModel(fileContents(modelPath, true)); + EXPECT_EQ(size_t(1), parser->issueCount()); + + importer->resolveImports(originalModel, modelImportPath); + + EXPECT_EQ(size_t(0), importer->issueCount()); + for (size_t i = 0; i < importer->issueCount(); ++i) { + Debug() << "[" << i << "]: " << importer->issue(i)->description(); + } + + auto flatModel = importer->flattenModel(originalModel); + EXPECT_EQ(size_t(0), importer->issueCount()); + EXPECT_NE(nullptr, flatModel); + + analyser->analyseModel(flatModel); + EXPECT_EQ(size_t(0), analyser->issueCount()); + + const auto implementationCode = generator->implementationCode(analyser->analyserModel()); + EXPECT_NE(size_t(0), implementationCode.length()); + } +} diff --git a/tests/investigations/tests.cmake b/tests/investigations/tests.cmake new file mode 100644 index 0000000000..1b01f81fa1 --- /dev/null +++ b/tests/investigations/tests.cmake @@ -0,0 +1,18 @@ + +# Set the test name, 'test_' will be prepended to the +# name set here +set(CURRENT_TEST investigations) +# Set a category name to enable running commands like: +# ctest -R +# which will run the tests matching this category-label. +# Can be left empty (or just not set) +set(${CURRENT_TEST}_CATEGORY misc) +list(APPEND LIBCELLML_TESTS ${CURRENT_TEST}) +# Using absolute path relative to this file +set(${CURRENT_TEST}_SRCS + ${CMAKE_CURRENT_LIST_DIR}/investigations.cpp +) +set(${CURRENT_TEST}_HDRS +) + + diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 21f57ed540..27fa619c3e 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -35,12 +35,18 @@ std::string resourcePath(const std::string &resourceRelativePath) return TESTS_RESOURCE_LOCATION + "/" + resourceRelativePath; } -std::string fileContents(const std::string &fileName) +std::string fileContents(const std::string &fileName, bool absoulte) { - std::ifstream file(resourcePath(fileName)); + std::ifstream file; + if (absoulte) { + file.open(fileName); + } else { + file.open(resourcePath(fileName)); + } std::stringstream buffer; buffer << file.rdbuf(); + file.close(); return buffer.str(); } diff --git a/tests/test_utils.h b/tests/test_utils.h index 1199138682..c9c6347e92 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -82,7 +82,7 @@ std::chrono::steady_clock::time_point TEST_EXPORT timeNow(); int TEST_EXPORT elapsedTime(const std::chrono::steady_clock::time_point &startTime); std::string TEST_EXPORT resourcePath(const std::string &resourceRelativePath = ""); -std::string TEST_EXPORT fileContents(const std::string &fileName); +std::string TEST_EXPORT fileContents(const std::string &fileName, bool absoulte=false); void TEST_EXPORT printIssues(const libcellml::LoggerPtr &l, bool headings = false, bool cellmlElementTypes = false, bool rule = false); void TEST_EXPORT printModel(const libcellml::ModelPtr &model, bool includeMaths = true); From db5ea7019eb262ce35eced4f5301ba1773185d38 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Thu, 21 May 2026 09:44:11 +1200 Subject: [PATCH 03/36] Progress in improving caching for model analysis. --- src/analyser.cpp | 10 +++- src/analyser_p.h | 1 + src/analysermodel.cpp | 69 ++++++++++++++++++++++++- src/analysermodel_p.h | 5 ++ src/validator.cpp | 2 +- src/variable.cpp | 3 +- tests/investigations/investigations.cpp | 4 +- 7 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/analyser.cpp b/src/analyser.cpp index f2d52abea4..c753496518 100644 --- a/src/analyser.cpp +++ b/src/analyser.cpp @@ -401,9 +401,14 @@ AnalyserInternalVariablePtr Analyser::AnalyserImpl::internalVariable(const Varia { // Find and return, if there is one, the internal variable associated with // the given variable. + auto rawPtr = reinterpret_cast(variable.get()); + if (mInternalVariableMap.count(rawPtr) > 0) { + return mInternalVariableMap[rawPtr]; + } for (const auto &internalVariable : mInternalVariables) { if (mAnalyserModel->areEquivalentVariables(variable, internalVariable->mVariable)) { + mInternalVariableMap[rawPtr] = internalVariable; return internalVariable; } } @@ -414,6 +419,7 @@ AnalyserInternalVariablePtr Analyser::AnalyserImpl::internalVariable(const Varia auto res = AnalyserInternalVariable::create(variable); mInternalVariables.push_back(res); + mInternalVariableMap[rawPtr] = res; return res; } @@ -2320,11 +2326,13 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model) mAnalyserModel = AnalyserModel::AnalyserModelImpl::create(model); mInternalVariables.clear(); + mInternalVariableMap.clear(); mInternalEquations.clear(); mCiCnUnits.clear(); - mAnalyserModel->mPimpl->buildEquivalentVariablesCache(); + // mAnalyserModel->mPimpl->buildEquivalentVariablesCache(); + mAnalyserModel->mPimpl->buildEquivalentVariablesCache2(); // Recursively analyse the model's components, so that we end up with an AST // for each of the model's equations. diff --git a/src/analyser_p.h b/src/analyser_p.h index f888861de4..5d1fec3288 100644 --- a/src/analyser_p.h +++ b/src/analyser_p.h @@ -160,6 +160,7 @@ class Analyser::AnalyserImpl: public Logger::LoggerImpl AnalyserExternalVariablePtrs mExternalVariables; AnalyserInternalVariablePtrs mInternalVariables; + std::unordered_map mInternalVariableMap; AnalyserInternalEquationPtrs mInternalEquations; GeneratorProfilePtr mGeneratorProfile = GeneratorProfile::create(); diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 05703ff303..9fb3221c73 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -21,6 +21,8 @@ limitations under the License. #include "analysermodel_p.h" #include "utilities.h" +#include "debug.h" + namespace libcellml { AnalyserModelPtr AnalyserModel::AnalyserModelImpl::create(const ModelPtr &model) @@ -43,6 +45,54 @@ AnalyserModel::~AnalyserModel() delete mPimpl; } +void exploreEquivalentVariables(const VariablePtr &variable, std::set &equivalentGroup, std::set &visited) +{ + auto rawPtr = reinterpret_cast(variable.get()); + + if (visited.count(rawPtr) == 0) { + visited.insert(rawPtr); + equivalentGroup.insert(rawPtr); + + for (size_t i = 0; i < variable->equivalentVariableCount(); ++i) { + exploreEquivalentVariables(variable->equivalentVariable(i), equivalentGroup, visited); + } + } +} + +void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache2() +{ + std::set visited; + std::vector< std::set > equivalentVariableGroups; + mEquivalentVariableCache2.clear(); + + for (size_t i = 0; i < mModel->componentCount(); ++i) { + buildEquivalentVariablesCache2(mModel->component(i), visited, equivalentVariableGroups); + } +} + +void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache2(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups) +{ + for (size_t i = 0; i < component->variableCount(); ++i) { + auto variable = component->variable(i); + auto rawPtr = reinterpret_cast(variable.get()); + + if (visited.count(rawPtr) == 0) { + std::set equivalentGroup; + exploreEquivalentVariables(variable, equivalentGroup, visited); + size_t groupIndex = equivalentVariableGroups.size(); + + for (uintptr_t v : equivalentGroup) { + mEquivalentVariableCache2[v] = groupIndex; + } + equivalentVariableGroups.push_back(equivalentGroup); + } + } + + for (size_t i = 0; i < component->componentCount(); ++i) { + buildEquivalentVariablesCache2(component->component(i), visited, equivalentVariableGroups); + } +} + void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache() { mEquivalentVariableCache.clear(); @@ -527,7 +577,24 @@ bool AnalyserModel::areEquivalentVariables(const VariablePtr &variable1, auto v1 = reinterpret_cast(variable1.get()); auto v2 = reinterpret_cast(variable2.get()); - return mPimpl->find(v1) == mPimpl->find(v2); + // Debug() << "0000000"; + // auto a = mPimpl->mEquivalentVariableCache2.count(v1); + // Debug() << a; + // auto b = mPimpl->mEquivalentVariableCache2.count(v2); + // Debug() << b; + // if (a > 0) { + // Debug() << "v1: " << mPimpl->mEquivalentVariableCache2[v1]; + // } + // if (b > 0) { + // Debug() << "v2: " << mPimpl->mEquivalentVariableCache2[v2]; + + // } + + return (mPimpl->mEquivalentVariableCache2.count(v1) > 0) + && (mPimpl->mEquivalentVariableCache2.count(v2) > 0) + && (mPimpl->mEquivalentVariableCache2[v1] == mPimpl->mEquivalentVariableCache2[v2]); + + //return mPimpl->find(v1) == mPimpl->find(v2); } } // namespace libcellml diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index c7f3114af1..a495494bc0 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -18,6 +18,7 @@ limitations under the License. #include #include +#include #include "libcellml/analysermodel.h" @@ -46,6 +47,7 @@ struct AnalyserModel::AnalyserModelImpl std::vector mAnalyserEquations; std::unordered_map mEquivalentVariableCache; + std::unordered_map mEquivalentVariableCache2; uintptr_t find(uintptr_t x) { @@ -101,6 +103,9 @@ struct AnalyserModel::AnalyserModelImpl void buildEquivalentVariablesCache(); void buildEquivalentVariablesCache(const ComponentPtr &component); + void buildEquivalentVariablesCache2(); + void buildEquivalentVariablesCache2(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups); + AnalyserModelImpl(const ModelPtr &model); }; diff --git a/src/validator.cpp b/src/validator.cpp index 976740f515..0b22ac7bcb 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2781,7 +2781,7 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) ? std::make_pair(comp.get(), equivParent.get()) : std::make_pair(equivParent.get(), comp.get()); - // If we haven't processed this component connection yet, do it once + // If we haven't processed this component connection yet, do it once if (connectionIds.find(key) == connectionIds.end()) { connectionIds[key] = ""; //Variable::equivalenceConnectionId(item, equiv); } diff --git a/src/variable.cpp b/src/variable.cpp index 5644b63219..f6a11a945a 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -223,7 +223,8 @@ bool haveEquivalentVariables(const Variable *variable1, testedVariables.push_back(variable2); - for (size_t i = 0; i < variable2->equivalentVariableCount(); ++i) { + const size_t variable2EquivalentVariableCount = variable2->equivalentVariableCount(); + for (size_t i = 0; i < variable2EquivalentVariableCount; ++i) { Variable *equivalentVariable2 = variable2->equivalentVariable(i).get(); if ((std::find(testedVariables.begin(), testedVariables.end(), equivalentVariable2) == testedVariables.end()) diff --git a/tests/investigations/investigations.cpp b/tests/investigations/investigations.cpp index d5490872b8..208526ab59 100644 --- a/tests/investigations/investigations.cpp +++ b/tests/investigations/investigations.cpp @@ -23,7 +23,7 @@ limitations under the License. const char* BENCHMARKING_MODEL_ROOT = std::getenv("BENCHMARKING_MODEL_ROOT"); -TEST(Investigations, exponentialTimeConsumption11) +TEST(Investigations, DISABLED_exponentialTimeConsumption11) { const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/image_to_model.cellml"; const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/"; @@ -106,7 +106,7 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption380) printIssues(analyser); } -TEST(Investigations, DISABLED_exponentialTimeConsumption524) +TEST(Investigations, exponentialTimeConsumption524) { const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/image_to_model.cellml"; const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/"; From 89e46dfae5eec3fff8e8f8289d82c8568b532a48 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Mon, 15 Jun 2026 17:14:32 +1200 Subject: [PATCH 04/36] Fix code formatting issues. --- src/analysermodel.cpp | 8 ++++---- src/analysermodel_p.h | 2 +- src/internaltypes.h | 15 +++++++++------ src/unionfind.h | 15 ++++++++++----- src/validator.cpp | 10 +++------- tests/investigations/investigations.cpp | 9 ++------- tests/test_utils.h | 2 +- 7 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 9fb3221c73..82b2262163 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -62,7 +62,7 @@ void exploreEquivalentVariables(const VariablePtr &variable, std::set void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache2() { std::set visited; - std::vector< std::set > equivalentVariableGroups; + std::vector> equivalentVariableGroups; mEquivalentVariableCache2.clear(); for (size_t i = 0; i < mModel->componentCount(); ++i) { @@ -591,10 +591,10 @@ bool AnalyserModel::areEquivalentVariables(const VariablePtr &variable1, // } return (mPimpl->mEquivalentVariableCache2.count(v1) > 0) - && (mPimpl->mEquivalentVariableCache2.count(v2) > 0) - && (mPimpl->mEquivalentVariableCache2[v1] == mPimpl->mEquivalentVariableCache2[v2]); + && (mPimpl->mEquivalentVariableCache2.count(v2) > 0) + && (mPimpl->mEquivalentVariableCache2[v1] == mPimpl->mEquivalentVariableCache2[v2]); - //return mPimpl->find(v1) == mPimpl->find(v2); + // return mPimpl->find(v1) == mPimpl->find(v2); } } // namespace libcellml diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index a495494bc0..a0ab6d8845 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -17,8 +17,8 @@ limitations under the License. #pragma once #include -#include #include +#include #include "libcellml/analysermodel.h" diff --git a/src/internaltypes.h b/src/internaltypes.h index dcb67919b9..d17a20a078 100644 --- a/src/internaltypes.h +++ b/src/internaltypes.h @@ -80,12 +80,13 @@ using UnitsConstPtr = std::shared_ptr; /**< Type definition for sha using ConnectionMap = std::map; /**< Type definition for a connection map.*/ using NamePairList = std::vector; /**< Type definition for a list of a pair of names. */ -using ComponentRawPtrPair = std::pair; +using ComponentRawPtrPair = std::pair; using ConnectionIdMap = std::map; -struct ComponentPair { - - bool operator==(const ComponentPair& other) const { +struct ComponentPair +{ + bool operator==(const ComponentPair &other) const + { return c1 == other.c1 && c2 == other.c2; } @@ -93,8 +94,10 @@ struct ComponentPair { ComponentPtr c2; }; -struct ComponentPairHash { - size_t operator()(const ComponentPair& p) const { +struct ComponentPairHash +{ + size_t operator()(const ComponentPair &p) const + { return std::hash()(p.c1) ^ std::hash()(p.c2); } }; diff --git a/src/unionfind.h b/src/unionfind.h index f421fb3504..371dceded2 100644 --- a/src/unionfind.h +++ b/src/unionfind.h @@ -17,9 +17,11 @@ limitations under the License. #include template -class UnionFind { +class UnionFind +{ public: - T find(const T& x) { + T find(const T &x) + { auto it = parent.find(x); if (it == parent.end()) { parent[x] = x; @@ -33,11 +35,13 @@ class UnionFind { return it->second; } - void unite(const T& a, const T& b) { + void unite(const T &a, const T &b) + { T rootA = find(a); T rootB = find(b); - if (rootA == rootB) return; + if (rootA == rootB) + return; // Union by rank if (rank[rootA] < rank[rootB]) { @@ -50,7 +54,8 @@ class UnionFind { } } - bool connected(const T& a, const T& b) { + bool connected(const T &a, const T &b) + { return find(a) == find(b); } diff --git a/src/validator.cpp b/src/validator.cpp index 0b22ac7bcb..0824472c4e 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2777,13 +2777,11 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) auto equivParent = owningComponent(equiv); if (equivParent != nullptr) { // Normalize the key order (min pointer first, max pointer second) - auto key = comp.get() < equivParent.get() - ? std::make_pair(comp.get(), equivParent.get()) - : std::make_pair(equivParent.get(), comp.get()); + auto key = comp.get() < equivParent.get() ? std::make_pair(comp.get(), equivParent.get()) : std::make_pair(equivParent.get(), comp.get()); // If we haven't processed this component connection yet, do it once if (connectionIds.find(key) == connectionIds.end()) { - connectionIds[key] = ""; //Variable::equivalenceConnectionId(item, equiv); + connectionIds[key] = ""; // Variable::equivalenceConnectionId(item, equiv); } } } @@ -2900,9 +2898,7 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component addIdMapItem(mappingId, info, idMap); } // Connections. - auto key = component.get() < equivParent.get() - ? std::make_pair(component.get(), equivParent.get()) - : std::make_pair(equivParent.get(), component.get()); + auto key = component.get() < equivParent.get() ? std::make_pair(component.get(), equivParent.get()) : std::make_pair(equivParent.get(), component.get()); auto connectionId = connectionIds.at(key); // auto connectionId = Variable::equivalenceConnectionId(item, equiv); diff --git a/tests/investigations/investigations.cpp b/tests/investigations/investigations.cpp index 208526ab59..634ce09ac2 100644 --- a/tests/investigations/investigations.cpp +++ b/tests/investigations/investigations.cpp @@ -21,7 +21,7 @@ limitations under the License. #include "test_utils.h" -const char* BENCHMARKING_MODEL_ROOT = std::getenv("BENCHMARKING_MODEL_ROOT"); +const char *BENCHMARKING_MODEL_ROOT = std::getenv("BENCHMARKING_MODEL_ROOT"); TEST(Investigations, DISABLED_exponentialTimeConsumption11) { @@ -36,7 +36,6 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption11) Debug() << parser->issue(0)->description(); Debug() << originalModel->name(); - importer->resolveImports(originalModel, modelImportPath); EXPECT_EQ(size_t(3), importer->issueCount()); @@ -63,7 +62,6 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption246) Debug() << parser->issue(0)->description(); Debug() << originalModel->name(); - importer->resolveImports(originalModel, modelImportPath); EXPECT_EQ(size_t(3), importer->issueCount()); @@ -91,7 +89,6 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption380) Debug() << parser->issue(0)->description(); Debug() << originalModel->name(); - importer->resolveImports(originalModel, modelImportPath); EXPECT_EQ(size_t(3), importer->issueCount()); @@ -119,7 +116,6 @@ TEST(Investigations, exponentialTimeConsumption524) Debug() << parser->issue(0)->description(); Debug() << originalModel->name(); - importer->resolveImports(originalModel, modelImportPath); EXPECT_EQ(size_t(3), importer->issueCount()); @@ -134,10 +130,9 @@ TEST(Investigations, exponentialTimeConsumption524) printIssues(analyser); } - TEST(Investigations, DISABLED_exponentialTimeConsumptionOthers) { - const std::vector vesselCounts = { 246, 380, 524 }; + const std::vector vesselCounts = {246, 380, 524}; auto importer = libcellml::Importer::create(false); auto parser = libcellml::Parser::create(false); diff --git a/tests/test_utils.h b/tests/test_utils.h index c9c6347e92..6c5f081a89 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -82,7 +82,7 @@ std::chrono::steady_clock::time_point TEST_EXPORT timeNow(); int TEST_EXPORT elapsedTime(const std::chrono::steady_clock::time_point &startTime); std::string TEST_EXPORT resourcePath(const std::string &resourceRelativePath = ""); -std::string TEST_EXPORT fileContents(const std::string &fileName, bool absoulte=false); +std::string TEST_EXPORT fileContents(const std::string &fileName, bool absoulte = false); void TEST_EXPORT printIssues(const libcellml::LoggerPtr &l, bool headings = false, bool cellmlElementTypes = false, bool rule = false); void TEST_EXPORT printModel(const libcellml::ModelPtr &model, bool includeMaths = true); From fa252bbe1f1a1c784540b9b42130769c25841aba Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 19 Jun 2026 00:29:00 +1200 Subject: [PATCH 05/36] Add search optional parameter to equivalenceVariableId variable API function. --- src/api/libcellml/variable.h | 7 ++++++- src/variable.cpp | 20 ++++++++++++-------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/api/libcellml/variable.h b/src/api/libcellml/variable.h index 2d9e6292dc..d76f82559c 100644 --- a/src/api/libcellml/variable.h +++ b/src/api/libcellml/variable.h @@ -174,14 +174,17 @@ class LIBCELLML_EXPORT Variable: public NamedEntity * * Get the connection identifier set for the equivalence defined with the given variables. * The variables are commutative. If no connection identifier is set the empty string is returned. + * The optional parameter @p search will traverse the equivalence network to find the connection identifier for the + * equivalence defined by the two variables. By default this is true. * * If the two variables are not equivalent the empty string is returned. * * @param variable1 Variable one of the equivalence. * @param variable2 Variable two of the equivalence. + * @param search Optional parameter to search the equivalence network for the connection identifier, true by default. * @return the @c std::string connection identifier. */ - static std::string equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2); + static std::string equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool search=true); /** * @brief Clear equivalent connection identifier for this equivalence. @@ -465,6 +468,8 @@ class LIBCELLML_EXPORT Variable: public NamedEntity */ VariablePtr clone() const; + std::string connectionIdMap() const; /**< Get the connection identifier map for this variable, @private. */ + private: bool doEquals(const EntityPtr &other) const override; /**< Virtual implementation method for equals, @private. */ diff --git a/src/variable.cpp b/src/variable.cpp index f6a11a945a..35d06f47fa 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -432,18 +432,22 @@ std::string Variable::equivalenceMappingId(const VariablePtr &variable1, const V return id; } -std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2) +std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool search) { std::string id; if ((variable1 != nullptr) && (variable2 != nullptr)) { - if (variable1->hasEquivalentVariable(variable2, true)) { - auto map = createConnectionMap(variable1, variable2); - for (auto &it : map) { - id = it.first->pFunc()->equivalentConnectionId(it.second); - } - if (id.empty()) { - id = variable1->pFunc()->equivalentConnectionId(variable2); + if (search) { + if (variable1->hasEquivalentVariable(variable2, true)) { + auto map = createConnectionMap(variable1, variable2); + for (auto &it : map) { + id = it.first->pFunc()->equivalentConnectionId(it.second); + } + if (id.empty()) { + id = variable1->pFunc()->equivalentConnectionId(variable2); + } } + } else { + id = variable1->pFunc()->equivalentConnectionId(variable2); } } return id; From fdc2e3267fed90a9a5f5c8bb16bbe8d5a074ef5f Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 19 Jun 2026 00:29:38 +1200 Subject: [PATCH 06/36] Make use of search parameter in equivalenceVariableId to reduce time taken to collect equivalence ids. --- src/validator.cpp | 89 ++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 66 deletions(-) diff --git a/src/validator.cpp b/src/validator.cpp index 0824472c4e..32c8213538 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2704,61 +2704,6 @@ void gatherComponents(const ComponentPtr &component, std::vector & IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) { - UnionFind uf; - - // Traverse all components and variables - for (size_t c = 0; c < model->componentCount(); ++c) { - auto component = model->component(c); - - for (size_t i = 0; i < component->variableCount(); ++i) { - auto v = component->variable(i); - - for (size_t e = 0; e < v->equivalentVariableCount(); ++e) { - auto equiv = v->equivalentVariable(e); - - if (equiv != nullptr) { - uf.unite(v, equiv); - } - } - } - } - - std::unordered_map> groups; - for (size_t c = 0; c < model->componentCount(); ++c) { - auto component = model->component(c); - - for (size_t i = 0; i < component->variableCount(); ++i) { - auto v = component->variable(i); - auto root = uf.find(v); - - groups[root].push_back(v); - } - } - - std::unordered_map, ComponentPairHash> connectionMap; - // using VarPair>, ComponentPairHash> connectionMap; - - // for (auto& [root, vars] : groups) { - // for (size_t i = 0; i < vars.size(); ++i) { - // for (size_t j = i + 1; j < vars.size(); ++j) { - // auto v1 = vars[i]; - // auto v2 = vars[j]; - - // auto c1 = owningComponent(v1); - // auto c2 = owningComponent(v2); - - // if (!c1 || !c2 || c1 == c2) continue; - - // // Normalize ordering - // ComponentPair key = (c1 < c2) - // ? ComponentPair{c1, c2} - // : ComponentPair{c2, c1}; - - // connectionMap[key].emplace_back(v1, v2); - // } - // } - // } - IdMap idMap; std::string info; std::set reportedConnections; @@ -2768,21 +2713,33 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) gatherComponents(model->component(c), allComponents); } + using Key = std::pair; + + struct PairHash { + size_t operator()(const Key& p) const { + return std::hash()(p.first) ^ + (std::hash()(p.second) << 1); + } + }; + ConnectionIdMap connectionIds; - for (const auto &comp : allComponents) { - for (size_t i = 0; i < comp->variableCount(); ++i) { - auto item = comp->variable(i); - for (size_t e = 0; e < item->equivalentVariableCount(); ++e) { - auto equiv = item->equivalentVariable(e); + std::unordered_set visitedPairs; + + for (const auto& comp : allComponents) { + auto rawPtr = comp.get(); + const size_t varCount = comp->variableCount(); + for (size_t i = 0; i < varCount; ++i) { + auto currentVariable = comp->variable(i); + for (size_t e = 0; e < currentVariable->equivalentVariableCount(); ++e) { + auto equiv = currentVariable->equivalentVariable(e); auto equivParent = owningComponent(equiv); if (equivParent != nullptr) { - // Normalize the key order (min pointer first, max pointer second) - auto key = comp.get() < equivParent.get() ? std::make_pair(comp.get(), equivParent.get()) : std::make_pair(equivParent.get(), comp.get()); - - // If we haven't processed this component connection yet, do it once - if (connectionIds.find(key) == connectionIds.end()) { - connectionIds[key] = ""; // Variable::equivalenceConnectionId(item, equiv); + auto equivRawPtr = equivParent.get(); + Key key = (rawPtr < equivRawPtr) ? Key{rawPtr, equivRawPtr} : Key{equivRawPtr, rawPtr}; + if (!visitedPairs.insert(key).second) { + continue; // Skip if we've already processed this pair } + connectionIds[key] = Variable::equivalenceConnectionId(currentVariable, equiv, false); } } } From f16c9ab21c2a2dd256d6ee9d382b25ea27da50f7 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 19 Jun 2026 01:05:46 +1200 Subject: [PATCH 07/36] Implement a group staged cache for the analyzer equivalent variables. --- src/CMakeLists.txt | 1 - src/analyser.cpp | 3 -- src/analysermodel.cpp | 69 ++++++------------------- src/analysermodel_p.h | 30 +---------- src/api/libcellml/analysermodel.h | 13 ++--- src/internaltypes.h | 19 ------- src/unionfind.h | 65 ----------------------- src/validator.cpp | 17 +++--- tests/investigations/investigations.cpp | 17 ++---- 9 files changed, 35 insertions(+), 199 deletions(-) delete mode 100644 src/unionfind.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8ca855dec9..488d2a773a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -149,7 +149,6 @@ set(GIT_HEADER_FILES ${CMAKE_CURRENT_SOURCE_DIR}/namespaces.h ${CMAKE_CURRENT_SOURCE_DIR}/parentedentity_p.h ${CMAKE_CURRENT_SOURCE_DIR}/reset_p.h - ${CMAKE_CURRENT_SOURCE_DIR}/unionfind.h ${CMAKE_CURRENT_SOURCE_DIR}/units_p.h ${CMAKE_CURRENT_SOURCE_DIR}/utilities.h ${CMAKE_CURRENT_SOURCE_DIR}/variable_p.h diff --git a/src/analyser.cpp b/src/analyser.cpp index c753496518..5162150822 100644 --- a/src/analyser.cpp +++ b/src/analyser.cpp @@ -2331,9 +2331,6 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model) mCiCnUnits.clear(); - // mAnalyserModel->mPimpl->buildEquivalentVariablesCache(); - mAnalyserModel->mPimpl->buildEquivalentVariablesCache2(); - // Recursively analyse the model's components, so that we end up with an AST // for each of the model's equations. diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 82b2262163..7415b80300 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -21,13 +21,17 @@ limitations under the License. #include "analysermodel_p.h" #include "utilities.h" -#include "debug.h" - namespace libcellml { AnalyserModelPtr AnalyserModel::AnalyserModelImpl::create(const ModelPtr &model) { - return std::shared_ptr {new AnalyserModel(model)}; + auto res = std::shared_ptr {new AnalyserModel(model)}; + + if (model) { + res->mPimpl->buildEquivalentVariablesCache(); + } + + return res; } AnalyserModel::AnalyserModelImpl::AnalyserModelImpl(const ModelPtr &model) @@ -59,18 +63,18 @@ void exploreEquivalentVariables(const VariablePtr &variable, std::set } } -void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache2() +void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache() { std::set visited; std::vector> equivalentVariableGroups; - mEquivalentVariableCache2.clear(); + mEquivalentVariableCache.clear(); for (size_t i = 0; i < mModel->componentCount(); ++i) { - buildEquivalentVariablesCache2(mModel->component(i), visited, equivalentVariableGroups); + buildEquivalentVariablesCache(mModel->component(i), visited, equivalentVariableGroups); } } -void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache2(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups) +void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups) { for (size_t i = 0; i < component->variableCount(); ++i) { auto variable = component->variable(i); @@ -82,41 +86,14 @@ void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache2(const Comp size_t groupIndex = equivalentVariableGroups.size(); for (uintptr_t v : equivalentGroup) { - mEquivalentVariableCache2[v] = groupIndex; + mEquivalentVariableCache[v] = groupIndex; } equivalentVariableGroups.push_back(equivalentGroup); } } for (size_t i = 0; i < component->componentCount(); ++i) { - buildEquivalentVariablesCache2(component->component(i), visited, equivalentVariableGroups); - } -} - -void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache() -{ - mEquivalentVariableCache.clear(); - for (size_t i = 0; i < mModel->componentCount(); ++i) { - buildEquivalentVariablesCache(mModel->component(i)); - } -} - -void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const ComponentPtr &component) -{ - for (size_t i = 0; i < component->variableCount(); ++i) { - auto variable = component->variable(i); - for (size_t j = 0; j < variable->equivalentVariableCount(); ++j) { - auto equivalentVariable = variable->equivalentVariable(j); - auto v1 = reinterpret_cast(variable.get()); - auto v2 = reinterpret_cast(equivalentVariable.get()); - if (v2 < v1) { - std::swap(v1, v2); - } - unite(v1, v2); - } - } - for (size_t i = 0; i < component->componentCount(); ++i) { - buildEquivalentVariablesCache(component->component(i)); + buildEquivalentVariablesCache(component->component(i), visited, equivalentVariableGroups); } } @@ -577,24 +554,8 @@ bool AnalyserModel::areEquivalentVariables(const VariablePtr &variable1, auto v1 = reinterpret_cast(variable1.get()); auto v2 = reinterpret_cast(variable2.get()); - // Debug() << "0000000"; - // auto a = mPimpl->mEquivalentVariableCache2.count(v1); - // Debug() << a; - // auto b = mPimpl->mEquivalentVariableCache2.count(v2); - // Debug() << b; - // if (a > 0) { - // Debug() << "v1: " << mPimpl->mEquivalentVariableCache2[v1]; - // } - // if (b > 0) { - // Debug() << "v2: " << mPimpl->mEquivalentVariableCache2[v2]; - - // } - - return (mPimpl->mEquivalentVariableCache2.count(v1) > 0) - && (mPimpl->mEquivalentVariableCache2.count(v2) > 0) - && (mPimpl->mEquivalentVariableCache2[v1] == mPimpl->mEquivalentVariableCache2[v2]); - - // return mPimpl->find(v1) == mPimpl->find(v2); + return (mPimpl->mEquivalentVariableCache.count(v1) > 0) + && (mPimpl->mEquivalentVariableCache[v1] == mPimpl->mEquivalentVariableCache[v2]); } } // namespace libcellml diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index a0ab6d8845..ce7cd5d441 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -46,30 +46,7 @@ struct AnalyserModel::AnalyserModelImpl std::vector mExternalVariables; std::vector mAnalyserEquations; - std::unordered_map mEquivalentVariableCache; - std::unordered_map mEquivalentVariableCache2; - - uintptr_t find(uintptr_t x) - { - auto it = mEquivalentVariableCache.find(x); - if (it == mEquivalentVariableCache.end()) { - mEquivalentVariableCache[x] = x; - return x; - } - if (it->second != x) { - it->second = find(it->second); - } - return it->second; - } - - void unite(uintptr_t x, uintptr_t y) - { - const uintptr_t &rootX = find(x); - const uintptr_t &rootY = find(y); - if (rootX != rootY) { - mEquivalentVariableCache[rootY] = rootX; - } - } + std::unordered_map mEquivalentVariableCache; bool mNeedEqFunction = false; bool mNeedNeqFunction = false; @@ -101,10 +78,7 @@ struct AnalyserModel::AnalyserModelImpl static AnalyserModelPtr create(const ModelPtr &model = nullptr); void buildEquivalentVariablesCache(); - void buildEquivalentVariablesCache(const ComponentPtr &component); - - void buildEquivalentVariablesCache2(); - void buildEquivalentVariablesCache2(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups); + void buildEquivalentVariablesCache(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups); AnalyserModelImpl(const ModelPtr &model); }; diff --git a/src/api/libcellml/analysermodel.h b/src/api/libcellml/analysermodel.h index fdb6752e17..b465c1d9e0 100644 --- a/src/api/libcellml/analysermodel.h +++ b/src/api/libcellml/analysermodel.h @@ -600,14 +600,9 @@ class LIBCELLML_EXPORT AnalyserModel * Returns @c true if @p variable1 is equivalent to @p variable2 and * @c false otherwise. * - * This test is a cached version of the internal areEquivalentVariables() utility. - * The cache is built when the model is analysed. - * If the model has changed since it was analysed, then the cache might not be valid - * and the result of this test may be incorrect. - * - * This function is intended to be used by the @ref Analyser whilst it is analysing the model. - * It is not intended to be used by external code. - * Although, we do not restrict the use of this function in case it does have other applications. + * The function utilises caching which is constructed during the model + * analysis phase (@ref Analyser::analyseModel). The cache may become + * out of date if the model is changed after the model has been analysed. * * @param variable1 The @ref Variable to test if it is equivalent to * @p variable2. @@ -617,7 +612,7 @@ class LIBCELLML_EXPORT AnalyserModel * @return @c true if @p variable1 is equivalent to @p variable2 and * @c false otherwise. */ - bool areEquivalentVariables(const VariablePtr &variable1, + bool areEquivalentVariables(const VariablePtr &variable1, const VariablePtr &variable2); private: diff --git a/src/internaltypes.h b/src/internaltypes.h index d17a20a078..b04c47c42e 100644 --- a/src/internaltypes.h +++ b/src/internaltypes.h @@ -83,25 +83,6 @@ using NamePairList = std::vector; /**< Type definition for a list of a using ComponentRawPtrPair = std::pair; using ConnectionIdMap = std::map; -struct ComponentPair -{ - bool operator==(const ComponentPair &other) const - { - return c1 == other.c1 && c2 == other.c2; - } - - ComponentPtr c1; - ComponentPtr c2; -}; - -struct ComponentPairHash -{ - size_t operator()(const ComponentPair &p) const - { - return std::hash()(p.c1) ^ std::hash()(p.c2); - } -}; - /** * @brief Class for defining an epoch in the history of a @ref Component or @ref Units. * diff --git a/src/unionfind.h b/src/unionfind.h deleted file mode 100644 index 371dceded2..0000000000 --- a/src/unionfind.h +++ /dev/null @@ -1,65 +0,0 @@ -/* -Copyright libCellML Contributors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -#include - -template -class UnionFind -{ -public: - T find(const T &x) - { - auto it = parent.find(x); - if (it == parent.end()) { - parent[x] = x; - rank[x] = 0; - return x; - } - - if (it->second != x) { - it->second = find(it->second); // Path compression - } - return it->second; - } - - void unite(const T &a, const T &b) - { - T rootA = find(a); - T rootB = find(b); - - if (rootA == rootB) - return; - - // Union by rank - if (rank[rootA] < rank[rootB]) { - parent[rootA] = rootB; - } else if (rank[rootA] > rank[rootB]) { - parent[rootB] = rootA; - } else { - parent[rootB] = rootA; - rank[rootA]++; - } - } - - bool connected(const T &a, const T &b) - { - return find(a) == find(b); - } - -private: - std::unordered_map parent; - std::unordered_map rank; -}; diff --git a/src/validator.cpp b/src/validator.cpp index 32c8213538..d3cc4822cd 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -35,7 +35,6 @@ limitations under the License. #include "issue_p.h" #include "logger_p.h" #include "namespaces.h" -#include "unionfind.h" #include "utilities.h" #include "xmldoc.h" #include "xmlutils.h" @@ -47,6 +46,12 @@ namespace libcellml { */ using IssuesList = std::vector; +/** + * Type definition for a pair of raw variable pointers using standard library. + */ +using RawComponentPtrPair = std::pair; + + /** * @brief Validate that equivalent variable pairs in the @p model * have equivalent units. @@ -2713,17 +2718,15 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) gatherComponents(model->component(c), allComponents); } - using Key = std::pair; - struct PairHash { - size_t operator()(const Key& p) const { + size_t operator()(const RawComponentPtrPair& p) const { return std::hash()(p.first) ^ (std::hash()(p.second) << 1); } }; ConnectionIdMap connectionIds; - std::unordered_set visitedPairs; + std::unordered_set visitedPairs; for (const auto& comp : allComponents) { auto rawPtr = comp.get(); @@ -2735,7 +2738,7 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) auto equivParent = owningComponent(equiv); if (equivParent != nullptr) { auto equivRawPtr = equivParent.get(); - Key key = (rawPtr < equivRawPtr) ? Key{rawPtr, equivRawPtr} : Key{equivRawPtr, rawPtr}; + auto key = (rawPtr < equivRawPtr) ? RawComponentPtrPair{rawPtr, equivRawPtr} : RawComponentPtrPair{equivRawPtr, rawPtr}; if (!visitedPairs.insert(key).second) { continue; // Skip if we've already processed this pair } @@ -2855,7 +2858,7 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component addIdMapItem(mappingId, info, idMap); } // Connections. - auto key = component.get() < equivParent.get() ? std::make_pair(component.get(), equivParent.get()) : std::make_pair(equivParent.get(), component.get()); + auto key = component.get() < equivParent.get() ? RawComponentPtrPair{component.get(), equivParent.get()} : RawComponentPtrPair{equivParent.get(), component.get()}; auto connectionId = connectionIds.at(key); // auto connectionId = Variable::equivalenceConnectionId(item, equiv); diff --git a/tests/investigations/investigations.cpp b/tests/investigations/investigations.cpp index 634ce09ac2..8620137fac 100644 --- a/tests/investigations/investigations.cpp +++ b/tests/investigations/investigations.cpp @@ -27,14 +27,13 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption11) { const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/image_to_model.cellml"; const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/"; + + Debug() << "modelPath: " << modelPath; auto importer = libcellml::Importer::create(false); auto parser = libcellml::Parser::create(false); auto originalModel = parser->parseModel(fileContents(modelPath, true)); EXPECT_EQ(size_t(1), parser->issueCount()); - Debug() << modelPath; - Debug() << parser->issue(0)->description(); - Debug() << originalModel->name(); importer->resolveImports(originalModel, modelImportPath); @@ -59,8 +58,6 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption246) auto parser = libcellml::Parser::create(false); auto originalModel = parser->parseModel(fileContents(modelPath, true)); EXPECT_EQ(size_t(1), parser->issueCount()); - Debug() << parser->issue(0)->description(); - Debug() << originalModel->name(); importer->resolveImports(originalModel, modelImportPath); @@ -85,9 +82,6 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption380) auto parser = libcellml::Parser::create(false); auto originalModel = parser->parseModel(fileContents(modelPath, true)); EXPECT_EQ(size_t(1), parser->issueCount()); - Debug() << modelPath; - Debug() << parser->issue(0)->description(); - Debug() << originalModel->name(); importer->resolveImports(originalModel, modelImportPath); @@ -103,7 +97,7 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption380) printIssues(analyser); } -TEST(Investigations, exponentialTimeConsumption524) +TEST(Investigations, DISABLED_exponentialTimeConsumption524) { const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/image_to_model.cellml"; const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/"; @@ -112,9 +106,6 @@ TEST(Investigations, exponentialTimeConsumption524) auto parser = libcellml::Parser::create(false); auto originalModel = parser->parseModel(fileContents(modelPath, true)); EXPECT_EQ(size_t(1), parser->issueCount()); - Debug() << modelPath; - Debug() << parser->issue(0)->description(); - Debug() << originalModel->name(); importer->resolveImports(originalModel, modelImportPath); @@ -126,7 +117,7 @@ TEST(Investigations, exponentialTimeConsumption524) auto analyser = libcellml::Analyser::create(); analyser->analyseModel(flatModel); - EXPECT_EQ(size_t(0), analyser->issueCount()); + EXPECT_EQ(size_t(4), analyser->issueCount()); printIssues(analyser); } From f5e6df8723c6885653c2d19b1f873d6f97cd8c2d Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 19 Jun 2026 01:17:54 +1200 Subject: [PATCH 08/36] Remove deleted unionfind.h file from list of headers. --- src/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8ca855dec9..488d2a773a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -149,7 +149,6 @@ set(GIT_HEADER_FILES ${CMAKE_CURRENT_SOURCE_DIR}/namespaces.h ${CMAKE_CURRENT_SOURCE_DIR}/parentedentity_p.h ${CMAKE_CURRENT_SOURCE_DIR}/reset_p.h - ${CMAKE_CURRENT_SOURCE_DIR}/unionfind.h ${CMAKE_CURRENT_SOURCE_DIR}/units_p.h ${CMAKE_CURRENT_SOURCE_DIR}/utilities.h ${CMAKE_CURRENT_SOURCE_DIR}/variable_p.h From 92a18de82fa4c6c07647f28f32b547764cc88b12 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 19 Jun 2026 01:24:57 +1200 Subject: [PATCH 09/36] Tidy up use of raw component ptr pair type. --- src/internaltypes.h | 4 ++-- src/validator.cpp | 17 ++++++----------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/internaltypes.h b/src/internaltypes.h index b04c47c42e..7998d622e6 100644 --- a/src/internaltypes.h +++ b/src/internaltypes.h @@ -80,8 +80,8 @@ using UnitsConstPtr = std::shared_ptr; /**< Type definition for sha using ConnectionMap = std::map; /**< Type definition for a connection map.*/ using NamePairList = std::vector; /**< Type definition for a list of a pair of names. */ -using ComponentRawPtrPair = std::pair; -using ConnectionIdMap = std::map; +using ComponentRawPtrPair = std::pair; /**< Type definition for pair of raw component pointers. */ +using ConnectionIdMap = std::map; /**< Type definition for map of pair of raw component pointers to connection ID. */ /** * @brief Class for defining an epoch in the history of a @ref Component or @ref Units. diff --git a/src/validator.cpp b/src/validator.cpp index d3cc4822cd..a24db9f6cf 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -46,11 +46,6 @@ namespace libcellml { */ using IssuesList = std::vector; -/** - * Type definition for a pair of raw variable pointers using standard library. - */ -using RawComponentPtrPair = std::pair; - /** * @brief Validate that equivalent variable pairs in the @p model @@ -2719,14 +2714,14 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) } struct PairHash { - size_t operator()(const RawComponentPtrPair& p) const { - return std::hash()(p.first) ^ - (std::hash()(p.second) << 1); + size_t operator()(const ComponentRawPtrPair& p) const { + return std::hash()(p.first) ^ + (std::hash()(p.second) << 1); } }; ConnectionIdMap connectionIds; - std::unordered_set visitedPairs; + std::unordered_set visitedPairs; for (const auto& comp : allComponents) { auto rawPtr = comp.get(); @@ -2738,7 +2733,7 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) auto equivParent = owningComponent(equiv); if (equivParent != nullptr) { auto equivRawPtr = equivParent.get(); - auto key = (rawPtr < equivRawPtr) ? RawComponentPtrPair{rawPtr, equivRawPtr} : RawComponentPtrPair{equivRawPtr, rawPtr}; + auto key = (rawPtr < equivRawPtr) ? ComponentRawPtrPair{rawPtr, equivRawPtr} : ComponentRawPtrPair{equivRawPtr, rawPtr}; if (!visitedPairs.insert(key).second) { continue; // Skip if we've already processed this pair } @@ -2858,7 +2853,7 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component addIdMapItem(mappingId, info, idMap); } // Connections. - auto key = component.get() < equivParent.get() ? RawComponentPtrPair{component.get(), equivParent.get()} : RawComponentPtrPair{equivParent.get(), component.get()}; + auto key = component.get() < equivParent.get() ? ComponentRawPtrPair{component.get(), equivParent.get()} : ComponentRawPtrPair{equivParent.get(), component.get()}; auto connectionId = connectionIds.at(key); // auto connectionId = Variable::equivalenceConnectionId(item, equiv); From 7d16ceb66d873cfa75ed867e75cb3705283da1b2 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 19 Jun 2026 01:33:55 +1200 Subject: [PATCH 10/36] Fix code formatting. --- src/api/libcellml/analysermodel.h | 2 +- src/api/libcellml/variable.h | 2 +- src/validator.cpp | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/api/libcellml/analysermodel.h b/src/api/libcellml/analysermodel.h index 31b7b67d7e..b264be7729 100644 --- a/src/api/libcellml/analysermodel.h +++ b/src/api/libcellml/analysermodel.h @@ -612,7 +612,7 @@ class LIBCELLML_EXPORT AnalyserModel * @return @c true if @p variable1 is equivalent to @p variable2 and * @c false otherwise. */ - bool areEquivalentVariables(const VariablePtr &variable1, const VariablePtr &variable2); + bool areEquivalentVariables(const VariablePtr &variable1, const VariablePtr &variable2); private: AnalyserModel(const ModelPtr &model); /**< Constructor, @private. */ diff --git a/src/api/libcellml/variable.h b/src/api/libcellml/variable.h index d76f82559c..65f042f801 100644 --- a/src/api/libcellml/variable.h +++ b/src/api/libcellml/variable.h @@ -184,7 +184,7 @@ class LIBCELLML_EXPORT Variable: public NamedEntity * @param search Optional parameter to search the equivalence network for the connection identifier, true by default. * @return the @c std::string connection identifier. */ - static std::string equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool search=true); + static std::string equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool search = true); /** * @brief Clear equivalent connection identifier for this equivalence. diff --git a/src/validator.cpp b/src/validator.cpp index a24db9f6cf..7e5720bc0c 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -46,7 +46,6 @@ namespace libcellml { */ using IssuesList = std::vector; - /** * @brief Validate that equivalent variable pairs in the @p model * have equivalent units. @@ -2713,17 +2712,18 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) gatherComponents(model->component(c), allComponents); } - struct PairHash { - size_t operator()(const ComponentRawPtrPair& p) const { - return std::hash()(p.first) ^ - (std::hash()(p.second) << 1); + struct PairHash + { + size_t operator()(const ComponentRawPtrPair &p) const + { + return std::hash()(p.first) ^ (std::hash()(p.second) << 1); } }; ConnectionIdMap connectionIds; std::unordered_set visitedPairs; - for (const auto& comp : allComponents) { + for (const auto &comp : allComponents) { auto rawPtr = comp.get(); const size_t varCount = comp->variableCount(); for (size_t i = 0; i < varCount; ++i) { @@ -2733,7 +2733,7 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) auto equivParent = owningComponent(equiv); if (equivParent != nullptr) { auto equivRawPtr = equivParent.get(); - auto key = (rawPtr < equivRawPtr) ? ComponentRawPtrPair{rawPtr, equivRawPtr} : ComponentRawPtrPair{equivRawPtr, rawPtr}; + auto key = (rawPtr < equivRawPtr) ? ComponentRawPtrPair {rawPtr, equivRawPtr} : ComponentRawPtrPair {equivRawPtr, rawPtr}; if (!visitedPairs.insert(key).second) { continue; // Skip if we've already processed this pair } @@ -2853,7 +2853,7 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component addIdMapItem(mappingId, info, idMap); } // Connections. - auto key = component.get() < equivParent.get() ? ComponentRawPtrPair{component.get(), equivParent.get()} : ComponentRawPtrPair{equivParent.get(), component.get()}; + auto key = component.get() < equivParent.get() ? ComponentRawPtrPair {component.get(), equivParent.get()} : ComponentRawPtrPair {equivParent.get(), component.get()}; auto connectionId = connectionIds.at(key); // auto connectionId = Variable::equivalenceConnectionId(item, equiv); From bd3f06f3170e9fad52b9b63eaef309f61dbcdc2d Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 19 Jun 2026 01:35:27 +1200 Subject: [PATCH 11/36] Fix spelling errors. --- src/internaltypes.h | 2 +- tests/test_utils.cpp | 4 ++-- tests/test_utils.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/internaltypes.h b/src/internaltypes.h index 7998d622e6..29673e55c0 100644 --- a/src/internaltypes.h +++ b/src/internaltypes.h @@ -42,7 +42,7 @@ using UniqueNames = std::set; /**< Type definition for a set of uni using NodeAttributeNamespaceInfo = std::vector>; /**< Type definition for attribute namespace information. */ // VariableMap -using VariableStdPair = std::pair; /**< Type definition for Variable pointer pair using standard libary. */ +using VariableStdPair = std::pair; /**< Type definition for Variable pointer pair using standard library. */ using VariableMap = std::vector; /**< Type definition for vector of VariablePair.*/ using VariableMapIterator = VariableMap::const_iterator; /**< Type definition of const iterator for vector of VariablePair.*/ diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 27fa619c3e..483fe307ad 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -35,10 +35,10 @@ std::string resourcePath(const std::string &resourceRelativePath) return TESTS_RESOURCE_LOCATION + "/" + resourceRelativePath; } -std::string fileContents(const std::string &fileName, bool absoulte) +std::string fileContents(const std::string &fileName, bool absolute) { std::ifstream file; - if (absoulte) { + if (absolute) { file.open(fileName); } else { file.open(resourcePath(fileName)); diff --git a/tests/test_utils.h b/tests/test_utils.h index 6c5f081a89..199a9e5e38 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -82,7 +82,7 @@ std::chrono::steady_clock::time_point TEST_EXPORT timeNow(); int TEST_EXPORT elapsedTime(const std::chrono::steady_clock::time_point &startTime); std::string TEST_EXPORT resourcePath(const std::string &resourceRelativePath = ""); -std::string TEST_EXPORT fileContents(const std::string &fileName, bool absoulte = false); +std::string TEST_EXPORT fileContents(const std::string &fileName, bool absolute = false); void TEST_EXPORT printIssues(const libcellml::LoggerPtr &l, bool headings = false, bool cellmlElementTypes = false, bool rule = false); void TEST_EXPORT printModel(const libcellml::ModelPtr &model, bool includeMaths = true); From 640379355ac8498a24cc7cf0d37d75523e346912 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Fri, 19 Jun 2026 01:37:46 +1200 Subject: [PATCH 12/36] Remove function connectionIdMap that is no longer required. --- src/api/libcellml/variable.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/api/libcellml/variable.h b/src/api/libcellml/variable.h index 65f042f801..1542aed286 100644 --- a/src/api/libcellml/variable.h +++ b/src/api/libcellml/variable.h @@ -468,8 +468,6 @@ class LIBCELLML_EXPORT Variable: public NamedEntity */ VariablePtr clone() const; - std::string connectionIdMap() const; /**< Get the connection identifier map for this variable, @private. */ - private: bool doEquals(const EntityPtr &other) const override; /**< Virtual implementation method for equals, @private. */ From cb2ddb9d3aa2c0c22db73cc21395d1f1eb1e2fbd Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Sat, 20 Jun 2026 00:03:55 +1200 Subject: [PATCH 13/36] Rework detection of equivalence connection id. --- src/api/libcellml/variable.h | 6 +- src/variable.cpp | 29 ++++++-- src/variable_p.h | 9 +++ tests/connection/connection.cpp | 91 ++++++++++++++++++++++--- tests/investigations/investigations.cpp | 2 +- tests/variable/variable.cpp | 22 ------ 6 files changed, 117 insertions(+), 42 deletions(-) diff --git a/src/api/libcellml/variable.h b/src/api/libcellml/variable.h index 1542aed286..3e71d0441b 100644 --- a/src/api/libcellml/variable.h +++ b/src/api/libcellml/variable.h @@ -174,17 +174,17 @@ class LIBCELLML_EXPORT Variable: public NamedEntity * * Get the connection identifier set for the equivalence defined with the given variables. * The variables are commutative. If no connection identifier is set the empty string is returned. - * The optional parameter @p search will traverse the equivalence network to find the connection identifier for the + * The optional parameter @p deepSearch will traverse the equivalence network to find the connection identifier for the * equivalence defined by the two variables. By default this is true. * * If the two variables are not equivalent the empty string is returned. * * @param variable1 Variable one of the equivalence. * @param variable2 Variable two of the equivalence. - * @param search Optional parameter to search the equivalence network for the connection identifier, true by default. + * @param deepSearch Optional parameter to deepSearch the equivalence network for the connection identifier, true by default. * @return the @c std::string connection identifier. */ - static std::string equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool search = true); + static std::string equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool deepSearch = true); /** * @brief Clear equivalent connection identifier for this equivalence. diff --git a/src/variable.cpp b/src/variable.cpp index 35d06f47fa..a51614df7c 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -124,7 +124,10 @@ bool Variable::removeEquivalence(const VariablePtr &variable1, const VariablePtr { if ((variable1 != nullptr) && (variable2 != nullptr)) { if (variable1->pFunc()->unsetEquivalentTo(variable2)) { - return variable2->pFunc()->unsetEquivalentTo(variable1); + variable2->pFunc()->unsetEquivalentTo(variable1); + variable1->pFunc()->unsafeResetEquivalenceIds(variable2); + variable2->pFunc()->unsafeResetEquivalenceIds(variable1); + return true; } } @@ -140,7 +143,10 @@ void Variable::removeAllEquivalences() equivalentVariable->pFunc()->unsetEquivalentTo(thisVariable); } } + pFunc()->mEquivalentVariables.clear(); + pFunc()->mConnectionIdMap.clear(); + pFunc()->mMappingIdMap.clear(); } VariablePtr Variable::equivalentVariable(size_t index) const @@ -181,6 +187,12 @@ void Variable::VariableImpl::cleanExpiredVariables() mEquivalentVariables.erase(std::remove_if(mEquivalentVariables.begin(), mEquivalentVariables.end(), [=](const VariableWeakPtr &variableWeak) -> bool { return variableWeak.expired(); }), mEquivalentVariables.end()); } +void Variable::VariableImpl::unsafeResetEquivalenceIds(const VariablePtr &equivalentVariable) +{ + setEquivalentMappingId(equivalentVariable, ""); + setEquivalentConnectionId(equivalentVariable, ""); +} + bool Variable::VariableImpl::hasEquivalentVariable(const VariablePtr &equivalentVariable, bool considerIndirectEquivalences) const { bool equivalent = false; @@ -432,19 +444,20 @@ std::string Variable::equivalenceMappingId(const VariablePtr &variable1, const V return id; } -std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool search) +std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool deepSearch) { std::string id; if ((variable1 != nullptr) && (variable2 != nullptr)) { - if (search) { + if (deepSearch) { if (variable1->hasEquivalentVariable(variable2, true)) { auto map = createConnectionMap(variable1, variable2); for (auto &it : map) { id = it.first->pFunc()->equivalentConnectionId(it.second); + if (!id.empty()) { + return id; + } } - if (id.empty()) { - id = variable1->pFunc()->equivalentConnectionId(variable2); - } + id = variable1->pFunc()->equivalentConnectionId(variable2); } } else { id = variable1->pFunc()->equivalentConnectionId(variable2); @@ -457,6 +470,10 @@ void Variable::removeEquivalenceConnectionId(const VariablePtr &variable1, const { if ((variable1 != nullptr) && (variable2 != nullptr)) { if (variable1->hasEquivalentVariable(variable2, true)) { + for (auto &it : createConnectionMap(variable1, variable2)) { + it.first->pFunc()->setEquivalentConnectionId(it.second, ""); + it.second->pFunc()->setEquivalentConnectionId(it.first, ""); + } variable1->pFunc()->setEquivalentConnectionId(variable2, ""); variable2->pFunc()->setEquivalentConnectionId(variable1, ""); } diff --git a/src/variable_p.h b/src/variable_p.h index 2b1e6fc56b..23507aa4e0 100644 --- a/src/variable_p.h +++ b/src/variable_p.h @@ -173,6 +173,15 @@ class Variable::VariableImpl: public NamedEntityImpl */ std::string equivalentConnectionId(const VariablePtr &equivalentVariable) const; + /** + * @brief Reset the connection and mapping ids associated with this variable and equivalent variable. + * + * This method will reset the connection id and the mapping id to empty. + * It will not check that the @p equivalentVariable is valid, this method expects the equivalent variable + * to be safe before using. + */ + void unsafeResetEquivalenceIds(const VariablePtr &equivalentVariable); + std::vector::iterator findEquivalentVariable(const VariablePtr &equivalentVariable); std::vector::const_iterator findEquivalentVariable(const VariablePtr &equivalentVariable) const; }; diff --git a/tests/connection/connection.cpp b/tests/connection/connection.cpp index 64ce2ea81a..023973bce8 100644 --- a/tests/connection/connection.cpp +++ b/tests/connection/connection.cpp @@ -19,7 +19,7 @@ limitations under the License. #include "test_utils.h" #include -TEST(Variable, addEquivalenceNullptrFirstParameter) +TEST(Connection, addEquivalenceNullptrFirstParameter) { libcellml::VariablePtr v1 = nullptr; libcellml::VariablePtr v2 = libcellml::Variable::create(); @@ -28,7 +28,7 @@ TEST(Variable, addEquivalenceNullptrFirstParameter) EXPECT_FALSE(v2->hasEquivalentVariable(v1)); } -TEST(Variable, addEquivalenceNullptrSecondParameter) +TEST(Connection, addEquivalenceNullptrSecondParameter) { libcellml::VariablePtr v1 = libcellml::Variable::create(); libcellml::VariablePtr v2 = nullptr; @@ -37,14 +37,14 @@ TEST(Variable, addEquivalenceNullptrSecondParameter) EXPECT_FALSE(v1->hasEquivalentVariable(v2)); } -TEST(Variable, addEquivalenceNullptrBothParameters) +TEST(Connection, addEquivalenceNullptrBothParameters) { libcellml::VariablePtr v1 = nullptr; libcellml::VariablePtr v2 = nullptr; libcellml::Variable::addEquivalence(v1, v2); } -TEST(Variable, addAndGetEquivalentVariable) +TEST(Connection, addAndGetEquivalentVariable) { libcellml::VariablePtr v1 = libcellml::Variable::create(); libcellml::VariablePtr v2 = libcellml::Variable::create(); @@ -52,7 +52,7 @@ TEST(Variable, addAndGetEquivalentVariable) EXPECT_EQ(v2, v1->equivalentVariable(0)); } -TEST(Variable, addAndGetEquivalentVariableReciprocal) +TEST(Connection, addAndGetEquivalentVariableReciprocal) { libcellml::VariablePtr v1 = libcellml::Variable::create(); libcellml::VariablePtr v2 = libcellml::Variable::create(); @@ -60,7 +60,7 @@ TEST(Variable, addAndGetEquivalentVariableReciprocal) EXPECT_EQ(v1, v2->equivalentVariable(0)); } -TEST(Variable, addTwoEquivalentVariablesAndCount) +TEST(Connection, addTwoEquivalentVariablesAndCount) { libcellml::VariablePtr v1 = libcellml::Variable::create(); libcellml::VariablePtr v2 = libcellml::Variable::create(); @@ -72,7 +72,7 @@ TEST(Variable, addTwoEquivalentVariablesAndCount) EXPECT_EQ(e, a); } -TEST(Variable, addDuplicateEquivalentVariablesAndCount) +TEST(Connection, addDuplicateEquivalentVariablesAndCount) { libcellml::VariablePtr v1 = libcellml::Variable::create(); libcellml::VariablePtr v2 = libcellml::Variable::create(); @@ -85,7 +85,7 @@ TEST(Variable, addDuplicateEquivalentVariablesAndCount) EXPECT_EQ(e, a); } -TEST(Variable, hasNoEquivalentVariable) +TEST(Connection, hasNoEquivalentVariable) { libcellml::VariablePtr v1 = libcellml::Variable::create(); libcellml::VariablePtr v2 = libcellml::Variable::create(); @@ -108,7 +108,7 @@ TEST(Variable, hasNoEquivalentVariable) EXPECT_FALSE(v1->hasEquivalentVariable(v2, true)); } -TEST(Variable, hasIndirectEquivalentVariable) +TEST(Connection, hasIndirectEquivalentVariable) { libcellml::VariablePtr v1 = libcellml::Variable::create(); libcellml::VariablePtr v2 = libcellml::Variable::create(); @@ -118,7 +118,7 @@ TEST(Variable, hasIndirectEquivalentVariable) EXPECT_TRUE(v1->hasEquivalentVariable(v3, true)); } -TEST(Variable, connectionId) +TEST(Connection, connectionId) { libcellml::VariablePtr v1 = libcellml::Variable::create(); libcellml::VariablePtr v2 = libcellml::Variable::create(); @@ -1440,3 +1440,74 @@ TEST(Connection, repeatedMapVariables) EXPECT_EQ_ISSUES(expectedIssues, p); } + +TEST(Connection, addEquivalenceReturnsFalseProperly) +{ + auto m = libcellml::Model::create("m"); + auto c1 = libcellml::Component::create("c1"); + auto c2 = libcellml::Component::create("c2"); + auto v1 = libcellml::Variable::create("v1"); + auto v2 = libcellml::Variable::create("v2"); + + EXPECT_TRUE(m->addComponent(c1)); + EXPECT_TRUE(m->addComponent(c2)); + EXPECT_TRUE(c1->addVariable(v1)); + EXPECT_TRUE(c2->addVariable(v2)); + + // Create a connection with self variable, expect no connections have been created. + EXPECT_FALSE(libcellml::Variable::addEquivalence(v1, v1)); + EXPECT_EQ(size_t(0), v1->equivalentVariableCount()); + + // Create a connection with one nullptr, expect no connections have been created. + EXPECT_FALSE(libcellml::Variable::addEquivalence(v2, nullptr)); + EXPECT_EQ(size_t(0), v2->equivalentVariableCount()); +} + +TEST(Connection, addEquivalenceConnectionIdPropagation) +{ + auto m = libcellml::Model::create("m"); + auto c1 = libcellml::Component::create("c1"); + auto c2 = libcellml::Component::create("c2"); + auto v1 = libcellml::Variable::create("v1"); + auto v2 = libcellml::Variable::create("v2"); + auto v3 = libcellml::Variable::create("v3"); + auto v4 = libcellml::Variable::create("v4"); + auto v5 = libcellml::Variable::create("v5"); + auto v6 = libcellml::Variable::create("v6"); + + m->addComponent(c1); + m->addComponent(c2); + c1->addVariable(v1); + c2->addVariable(v2); + c1->addVariable(v3); + c2->addVariable(v4); + c1->addVariable(v5); + c2->addVariable(v6); + + libcellml::Variable::addEquivalence(v1, v2); + libcellml::Variable::addEquivalence(v3, v4); + libcellml::Variable::setEquivalenceConnectionId(v1, v2, "connection_01"); + EXPECT_EQ("connection_01", libcellml::Variable::equivalenceConnectionId(v3, v4)); + + libcellml::Variable::addEquivalence(v5, v6); + EXPECT_EQ("connection_01", libcellml::Variable::equivalenceConnectionId(v5, v6)); + EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v5, v6, false)); + + libcellml::Variable::removeEquivalenceConnectionId(v1, v2); + EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); + EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v3, v4)); + EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v5, v6)); +} + +TEST(Connection, removeEquivalenceConnectionIdFromVariablesThatAreNotInComponents) +{ + auto v1 = libcellml::Variable::create("v1"); + auto v2 = libcellml::Variable::create("v2"); + + libcellml::Variable::addEquivalence(v1, v2); + libcellml::Variable::setEquivalenceConnectionId(v1, v2, "connection_01"); + EXPECT_EQ("connection_01", libcellml::Variable::equivalenceConnectionId(v1, v2)); + + libcellml::Variable::removeEquivalenceConnectionId(v1, v2); + EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); +} \ No newline at end of file diff --git a/tests/investigations/investigations.cpp b/tests/investigations/investigations.cpp index 0dddba1172..7906aef2d9 100644 --- a/tests/investigations/investigations.cpp +++ b/tests/investigations/investigations.cpp @@ -96,7 +96,7 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption380) printIssues(analyser); } -TEST(Investigations, DISABLED_exponentialTimeConsumption524) +TEST(Investigations, exponentialTimeConsumption524) { const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/image_to_model.cellml"; const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/"; diff --git a/tests/variable/variable.cpp b/tests/variable/variable.cpp index 9da5e20701..3380024f91 100644 --- a/tests/variable/variable.cpp +++ b/tests/variable/variable.cpp @@ -1890,25 +1890,3 @@ TEST(Variable, addVariableDuplicates) EXPECT_EQ(size_t(1), apple->variableCount()); EXPECT_EQ(size_t(1), tomato->variableCount()); } - -TEST(Variable, addEquivalenceReturnsFalseProperly) -{ - auto m = libcellml::Model::create("m"); - auto c1 = libcellml::Component::create("c1"); - auto c2 = libcellml::Component::create("c2"); - auto v1 = libcellml::Variable::create("v1"); - auto v2 = libcellml::Variable::create("v2"); - - EXPECT_TRUE(m->addComponent(c1)); - EXPECT_TRUE(m->addComponent(c2)); - EXPECT_TRUE(c1->addVariable(v1)); - EXPECT_TRUE(c2->addVariable(v2)); - - // Create a connection with self variable, expect no connections have been created. - EXPECT_FALSE(libcellml::Variable::addEquivalence(v1, v1)); - EXPECT_EQ(size_t(0), v1->equivalentVariableCount()); - - // Create a connection with one nullptr, expect no connections have been created. - EXPECT_FALSE(libcellml::Variable::addEquivalence(v2, nullptr)); - EXPECT_EQ(size_t(0), v2->equivalentVariableCount()); -} From 557ce7445d2662b5dafe4520be9a386489ce0ff8 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Sat, 20 Jun 2026 22:29:59 +1200 Subject: [PATCH 14/36] Add test to check that connection id is fully cleared when equivalence is removed. --- tests/connection/connection.cpp | 26 ++++++++++++++++++++++++- tests/investigations/investigations.cpp | 2 +- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/connection/connection.cpp b/tests/connection/connection.cpp index 023973bce8..696ad0accc 100644 --- a/tests/connection/connection.cpp +++ b/tests/connection/connection.cpp @@ -1510,4 +1510,28 @@ TEST(Connection, removeEquivalenceConnectionIdFromVariablesThatAreNotInComponent libcellml::Variable::removeEquivalenceConnectionId(v1, v2); EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); -} \ No newline at end of file +} + +TEST(Connection, addEquivalenceConnectionIdClearedAfterDisconnect) +{ + auto m = libcellml::Model::create("m"); + auto c1 = libcellml::Component::create("c1"); + auto c2 = libcellml::Component::create("c2"); + auto v1 = libcellml::Variable::create("v1"); + auto v2 = libcellml::Variable::create("v2"); + + m->addComponent(c1); + m->addComponent(c2); + c1->addVariable(v1); + c2->addVariable(v2); + + libcellml::Variable::addEquivalence(v1, v2); + libcellml::Variable::setEquivalenceConnectionId(v1, v2, "connection_01"); + EXPECT_EQ("connection_01", libcellml::Variable::equivalenceConnectionId(v1, v2)); + + libcellml::Variable::removeEquivalenceConnectionId(v1, v2); + EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); + + libcellml::Variable::addEquivalence(v1, v2); + EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); +} diff --git a/tests/investigations/investigations.cpp b/tests/investigations/investigations.cpp index 7906aef2d9..0dddba1172 100644 --- a/tests/investigations/investigations.cpp +++ b/tests/investigations/investigations.cpp @@ -96,7 +96,7 @@ TEST(Investigations, DISABLED_exponentialTimeConsumption380) printIssues(analyser); } -TEST(Investigations, exponentialTimeConsumption524) +TEST(Investigations, DISABLED_exponentialTimeConsumption524) { const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/image_to_model.cellml"; const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/"; From 55be8690fe1b3e6f1c598c1a51a632cf0b9ed899 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Sat, 20 Jun 2026 22:31:20 +1200 Subject: [PATCH 15/36] Remove investigations tests. --- tests/CMakeLists.txt | 1 - tests/investigations/investigations.cpp | 157 ------------------------ tests/investigations/tests.cmake | 18 --- 3 files changed, 176 deletions(-) delete mode 100644 tests/investigations/investigations.cpp delete mode 100644 tests/investigations/tests.cmake diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cffc108e58..b2b96add12 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -47,7 +47,6 @@ include(units/tests.cmake) include(validator/tests.cmake) include(variable/tests.cmake) include(version/tests.cmake) -include(investigations/tests.cmake) set(TEST_EXPORTDEFINITIONS_H "${CMAKE_CURRENT_BINARY_DIR}/test_exportdefinitions.h") diff --git a/tests/investigations/investigations.cpp b/tests/investigations/investigations.cpp deleted file mode 100644 index 0dddba1172..0000000000 --- a/tests/investigations/investigations.cpp +++ /dev/null @@ -1,157 +0,0 @@ -/* -Copyright libCellML Contributors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -#include "gtest/gtest.h" -#include - -#include - -#include "test_utils.h" - -const char *BENCHMARKING_MODEL_ROOT = std::getenv("BENCHMARKING_MODEL_ROOT"); - -TEST(Investigations, DISABLED_exponentialTimeConsumption11) -{ - const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/image_to_model.cellml"; - const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_11_vessels/"; - - auto importer = libcellml::Importer::create(false); - - auto parser = libcellml::Parser::create(false); - auto originalModel = parser->parseModel(fileContents(modelPath, true)); - EXPECT_EQ(size_t(1), parser->issueCount()); - - importer->resolveImports(originalModel, modelImportPath); - - EXPECT_EQ(size_t(3), importer->issueCount()); - - auto flatModel = importer->flattenModel(originalModel); - EXPECT_EQ(size_t(0), importer->issueCount()); - EXPECT_NE(nullptr, flatModel); - - auto analyser = libcellml::Analyser::create(); - analyser->analyseModel(flatModel); - EXPECT_EQ(size_t(0), analyser->issueCount()); - printIssues(analyser); -} - -TEST(Investigations, DISABLED_exponentialTimeConsumption246) -{ - const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_246_vessels/image_to_model.cellml"; - const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_246_vessels/"; - auto importer = libcellml::Importer::create(false); - - auto parser = libcellml::Parser::create(false); - auto originalModel = parser->parseModel(fileContents(modelPath, true)); - EXPECT_EQ(size_t(1), parser->issueCount()); - - importer->resolveImports(originalModel, modelImportPath); - - EXPECT_EQ(size_t(3), importer->issueCount()); - - auto flatModel = importer->flattenModel(originalModel); - EXPECT_EQ(size_t(0), importer->issueCount()); - EXPECT_NE(nullptr, flatModel); - - auto analyser = libcellml::Analyser::create(); - analyser->analyseModel(flatModel); - EXPECT_EQ(size_t(0), analyser->issueCount()); - printIssues(analyser); -} - -TEST(Investigations, DISABLED_exponentialTimeConsumption380) -{ - const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_380_vessels/image_to_model.cellml"; - const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_380_vessels/"; - auto importer = libcellml::Importer::create(false); - - auto parser = libcellml::Parser::create(false); - auto originalModel = parser->parseModel(fileContents(modelPath, true)); - EXPECT_EQ(size_t(1), parser->issueCount()); - - importer->resolveImports(originalModel, modelImportPath); - - EXPECT_EQ(size_t(3), importer->issueCount()); - - auto flatModel = importer->flattenModel(originalModel); - EXPECT_EQ(size_t(0), importer->issueCount()); - EXPECT_NE(nullptr, flatModel); - - auto analyser = libcellml::Analyser::create(); - analyser->analyseModel(flatModel); - EXPECT_EQ(size_t(0), analyser->issueCount()); - printIssues(analyser); -} - -TEST(Investigations, DISABLED_exponentialTimeConsumption524) -{ - const std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/image_to_model.cellml"; - const std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_524_vessels/"; - auto importer = libcellml::Importer::create(false); - - auto parser = libcellml::Parser::create(false); - auto originalModel = parser->parseModel(fileContents(modelPath, true)); - EXPECT_EQ(size_t(1), parser->issueCount()); - - importer->resolveImports(originalModel, modelImportPath); - - EXPECT_EQ(size_t(3), importer->issueCount()); - - auto flatModel = importer->flattenModel(originalModel); - EXPECT_EQ(size_t(0), importer->issueCount()); - EXPECT_NE(nullptr, flatModel); - - auto analyser = libcellml::Analyser::create(); - analyser->analyseModel(flatModel); - EXPECT_EQ(size_t(4), analyser->issueCount()); - printIssues(analyser); -} - -TEST(Investigations, DISABLED_exponentialTimeConsumptionOthers) -{ - const std::vector vesselCounts = {246, 380, 524}; - - auto importer = libcellml::Importer::create(false); - auto parser = libcellml::Parser::create(false); - auto printer = libcellml::Printer::create(); - auto analyser = libcellml::Analyser::create(); - auto generator = libcellml::Generator::create(); - - for (int vesselCount : vesselCounts) { - SCOPED_TRACE("Vessel count: " + std::to_string(vesselCount)); - std::string modelPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_" + std::to_string(vesselCount) + "_vessels/image_to_model.cellml"; - std::string modelImportPath = std::string(BENCHMARKING_MODEL_ROOT) + "image_to_model_" + std::to_string(vesselCount) + "_vessels/"; - auto originalModel = parser->parseModel(fileContents(modelPath, true)); - EXPECT_EQ(size_t(1), parser->issueCount()); - - importer->resolveImports(originalModel, modelImportPath); - - EXPECT_EQ(size_t(0), importer->issueCount()); - for (size_t i = 0; i < importer->issueCount(); ++i) { - Debug() << "[" << i << "]: " << importer->issue(i)->description(); - } - - auto flatModel = importer->flattenModel(originalModel); - EXPECT_EQ(size_t(0), importer->issueCount()); - EXPECT_NE(nullptr, flatModel); - - analyser->analyseModel(flatModel); - EXPECT_EQ(size_t(0), analyser->issueCount()); - - const auto implementationCode = generator->implementationCode(analyser->analyserModel()); - EXPECT_NE(size_t(0), implementationCode.length()); - } -} diff --git a/tests/investigations/tests.cmake b/tests/investigations/tests.cmake deleted file mode 100644 index 1b01f81fa1..0000000000 --- a/tests/investigations/tests.cmake +++ /dev/null @@ -1,18 +0,0 @@ - -# Set the test name, 'test_' will be prepended to the -# name set here -set(CURRENT_TEST investigations) -# Set a category name to enable running commands like: -# ctest -R -# which will run the tests matching this category-label. -# Can be left empty (or just not set) -set(${CURRENT_TEST}_CATEGORY misc) -list(APPEND LIBCELLML_TESTS ${CURRENT_TEST}) -# Using absolute path relative to this file -set(${CURRENT_TEST}_SRCS - ${CMAKE_CURRENT_LIST_DIR}/investigations.cpp -) -set(${CURRENT_TEST}_HDRS -) - - From 654cfba6e3b3d96417447d4bfdd0c2aae2af1dec Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Sat, 20 Jun 2026 22:41:27 +1200 Subject: [PATCH 16/36] Fix code formatting. --- tests/connection/connection.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/connection/connection.cpp b/tests/connection/connection.cpp index 696ad0accc..4789fe4b7f 100644 --- a/tests/connection/connection.cpp +++ b/tests/connection/connection.cpp @@ -1494,9 +1494,9 @@ TEST(Connection, addEquivalenceConnectionIdPropagation) EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v5, v6, false)); libcellml::Variable::removeEquivalenceConnectionId(v1, v2); - EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); - EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v3, v4)); - EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v5, v6)); + EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v1, v2)); + EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v3, v4)); + EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v5, v6)); } TEST(Connection, removeEquivalenceConnectionIdFromVariablesThatAreNotInComponents) @@ -1509,7 +1509,7 @@ TEST(Connection, removeEquivalenceConnectionIdFromVariablesThatAreNotInComponent EXPECT_EQ("connection_01", libcellml::Variable::equivalenceConnectionId(v1, v2)); libcellml::Variable::removeEquivalenceConnectionId(v1, v2); - EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); + EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v1, v2)); } TEST(Connection, addEquivalenceConnectionIdClearedAfterDisconnect) @@ -1530,8 +1530,8 @@ TEST(Connection, addEquivalenceConnectionIdClearedAfterDisconnect) EXPECT_EQ("connection_01", libcellml::Variable::equivalenceConnectionId(v1, v2)); libcellml::Variable::removeEquivalenceConnectionId(v1, v2); - EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); + EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v1, v2)); libcellml::Variable::addEquivalence(v1, v2); - EXPECT_EQ("" , libcellml::Variable::equivalenceConnectionId(v1, v2)); + EXPECT_EQ("", libcellml::Variable::equivalenceConnectionId(v1, v2)); } From 4758562043cea398a72df367d464da54c29b88cc Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Sat, 20 Jun 2026 23:23:18 +1200 Subject: [PATCH 17/36] Tidy the code changes. --- src/analysermodel_p.h | 2 +- src/internaltypes.h | 6 +++--- src/printer.cpp | 4 ++-- src/validator.cpp | 28 ++++++++++++++++------------ src/variable.cpp | 10 ++++++++-- 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index 61e4d05e70..937e689d98 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -78,8 +78,8 @@ struct AnalyserModel::AnalyserModelImpl static AnalyserModelPtr create(const ModelPtr &model = nullptr); - void buildEquivalentVariablesCache(); void buildEquivalentVariablesCache(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups); + void buildEquivalentVariablesCache(); AnalyserModelImpl(const ModelPtr &model); }; diff --git a/src/internaltypes.h b/src/internaltypes.h index 29673e55c0..777b846b82 100644 --- a/src/internaltypes.h +++ b/src/internaltypes.h @@ -47,9 +47,9 @@ using VariableMap = std::vector; /**< Type definition for vecto using VariableMapIterator = VariableMap::const_iterator; /**< Type definition of const iterator for vector of VariablePair.*/ // ComponentMap -using ComponentStdPair = std::pair; /**< Type definition for Component pointer pair using standard library.*/ -using ComponentMap = std::vector; /**< Type definition for vector of ComponentStdPair.*/ -using ComponentMapIterator = ComponentMap::const_iterator; /**< Type definition of const iterator for vector of ComponentStdPair.*/ +using ComponentPair = std::pair; /**< Type definition for Component pointer pair using standard library.*/ +using ComponentMap = std::vector; /**< Type definition for vector of ComponentPair.*/ +using ComponentMapIterator = ComponentMap::const_iterator; /**< Type definition of const iterator for vector of ComponentPair.*/ using VariablePtrs = std::vector; /**< Type definition for list of variables. */ diff --git a/src/printer.cpp b/src/printer.cpp index d88bed4e1d..6a39d3a5f5 100644 --- a/src/printer.cpp +++ b/src/printer.cpp @@ -77,7 +77,7 @@ std::string printConnections(const ComponentMap &componentMap, const VariableMap for (auto iterPair = componentMap.begin(); iterPair < componentMap.end(); ++iterPair) { ComponentPtr currentComponent1 = iterPair->first; ComponentPtr currentComponent2 = iterPair->second; - ComponentStdPair currentComponentPair = std::make_pair(currentComponent1, currentComponent2); + ComponentPair currentComponentPair = std::make_pair(currentComponent1, currentComponent2); // Check whether this set of connections has already been serialised. bool pairFound = false; for (const auto &serialisedIterPair : serialisedComponentMap) { @@ -178,7 +178,7 @@ void buildMapsForComponentsVariables(const ComponentPtr &component, ComponentMap ComponentPtr component1 = owningComponent(variable); ComponentPtr component2 = owningComponent(equivalentVariable); // Also create a component map pair corresponding with the variable map pair. - ComponentStdPair componentPair = std::make_pair(component1, component2); + ComponentPair componentPair = std::make_pair(component1, component2); componentMap.push_back(componentPair); } } diff --git a/src/validator.cpp b/src/validator.cpp index 7e5720bc0c..bbbcda9b3f 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2696,22 +2696,14 @@ void Validator::ValidatorImpl::addIdMapItem(const std::string &id, const std::st void gatherComponents(const ComponentPtr &component, std::vector &allComponents) { allComponents.push_back(component); + for (size_t c = 0; c < component->componentCount(); ++c) { gatherComponents(component->component(c), allComponents); } } IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) -{ - IdMap idMap; - std::string info; - std::set reportedConnections; - - std::vector allComponents; - for (size_t c = 0; c < model->componentCount(); ++c) { - gatherComponents(model->component(c), allComponents); - } - +{ struct PairHash { size_t operator()(const ComponentRawPtrPair &p) const @@ -2720,20 +2712,33 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) } }; + IdMap idMap; + std::string info; + std::set reportedConnections; + std::vector allComponents; + + for (size_t c = 0; c < model->componentCount(); ++c) { + gatherComponents(model->component(c), allComponents); + } + ConnectionIdMap connectionIds; std::unordered_set visitedPairs; for (const auto &comp : allComponents) { auto rawPtr = comp.get(); const size_t varCount = comp->variableCount(); + for (size_t i = 0; i < varCount; ++i) { auto currentVariable = comp->variable(i); + for (size_t e = 0; e < currentVariable->equivalentVariableCount(); ++e) { auto equiv = currentVariable->equivalentVariable(e); auto equivParent = owningComponent(equiv); + if (equivParent != nullptr) { auto equivRawPtr = equivParent.get(); auto key = (rawPtr < equivRawPtr) ? ComponentRawPtrPair {rawPtr, equivRawPtr} : ComponentRawPtrPair {equivRawPtr, rawPtr}; + if (!visitedPairs.insert(key).second) { continue; // Skip if we've already processed this pair } @@ -2854,10 +2859,9 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component } // Connections. auto key = component.get() < equivParent.get() ? ComponentRawPtrPair {component.get(), equivParent.get()} : ComponentRawPtrPair {equivParent.get(), component.get()}; - auto connectionId = connectionIds.at(key); - // auto connectionId = Variable::equivalenceConnectionId(item, equiv); std::string connection = component->name() < equivParent->name() ? component->name() + equivParent->name() : equivParent->name() + component->name(); + if ((s1 < s2) && !connectionId.empty() && (reportedConnections.count(connection) == 0)) { std::string connectionDescription = "between components '" + component->name() + "' and '" + equivParent->name() diff --git a/src/variable.cpp b/src/variable.cpp index a51614df7c..086bb82f13 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -235,8 +235,7 @@ bool haveEquivalentVariables(const Variable *variable1, testedVariables.push_back(variable2); - const size_t variable2EquivalentVariableCount = variable2->equivalentVariableCount(); - for (size_t i = 0; i < variable2EquivalentVariableCount; ++i) { + for (size_t i = 0; i < variable2->equivalentVariableCount(); ++i) { Variable *equivalentVariable2 = variable2->equivalentVariable(i).get(); if ((std::find(testedVariables.begin(), testedVariables.end(), equivalentVariable2) == testedVariables.end()) @@ -448,15 +447,19 @@ std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, cons { std::string id; if ((variable1 != nullptr) && (variable2 != nullptr)) { + if (deepSearch) { + if (variable1->hasEquivalentVariable(variable2, true)) { auto map = createConnectionMap(variable1, variable2); + for (auto &it : map) { id = it.first->pFunc()->equivalentConnectionId(it.second); if (!id.empty()) { return id; } } + id = variable1->pFunc()->equivalentConnectionId(variable2); } } else { @@ -469,11 +472,14 @@ std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, cons void Variable::removeEquivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2) { if ((variable1 != nullptr) && (variable2 != nullptr)) { + if (variable1->hasEquivalentVariable(variable2, true)) { + for (auto &it : createConnectionMap(variable1, variable2)) { it.first->pFunc()->setEquivalentConnectionId(it.second, ""); it.second->pFunc()->setEquivalentConnectionId(it.first, ""); } + variable1->pFunc()->setEquivalentConnectionId(variable2, ""); variable2->pFunc()->setEquivalentConnectionId(variable1, ""); } From 1b27b2d9bdf7338f0f4a03165c8344f7479d1a96 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Sat, 20 Jun 2026 23:25:55 +1200 Subject: [PATCH 18/36] Remove unused type definition. --- src/internaltypes.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/internaltypes.h b/src/internaltypes.h index 777b846b82..77d278877b 100644 --- a/src/internaltypes.h +++ b/src/internaltypes.h @@ -42,7 +42,6 @@ using UniqueNames = std::set; /**< Type definition for a set of uni using NodeAttributeNamespaceInfo = std::vector>; /**< Type definition for attribute namespace information. */ // VariableMap -using VariableStdPair = std::pair; /**< Type definition for Variable pointer pair using standard library. */ using VariableMap = std::vector; /**< Type definition for vector of VariablePair.*/ using VariableMapIterator = VariableMap::const_iterator; /**< Type definition of const iterator for vector of VariablePair.*/ From 745af5488b8476edbbacbf8b5264f7dcce7c3b64 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Sat, 20 Jun 2026 23:36:39 +1200 Subject: [PATCH 19/36] Fix code formatting errors. --- src/validator.cpp | 2 +- src/variable.cpp | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/validator.cpp b/src/validator.cpp index bbbcda9b3f..f8eeebb1c3 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2703,7 +2703,7 @@ void gatherComponents(const ComponentPtr &component, std::vector & } IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) -{ +{ struct PairHash { size_t operator()(const ComponentRawPtrPair &p) const diff --git a/src/variable.cpp b/src/variable.cpp index 086bb82f13..12e7f4f221 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -447,9 +447,7 @@ std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, cons { std::string id; if ((variable1 != nullptr) && (variable2 != nullptr)) { - if (deepSearch) { - if (variable1->hasEquivalentVariable(variable2, true)) { auto map = createConnectionMap(variable1, variable2); @@ -472,9 +470,7 @@ std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, cons void Variable::removeEquivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2) { if ((variable1 != nullptr) && (variable2 != nullptr)) { - if (variable1->hasEquivalentVariable(variable2, true)) { - for (auto &it : createConnectionMap(variable1, variable2)) { it.first->pFunc()->setEquivalentConnectionId(it.second, ""); it.second->pFunc()->setEquivalentConnectionId(it.first, ""); From 298fb51386d76ac2006179f33781f93ca00a4681 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 10:54:55 +1200 Subject: [PATCH 20/36] Variable: slight update to an API description. --- src/api/libcellml/variable.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/libcellml/variable.h b/src/api/libcellml/variable.h index 3e71d0441b..5bfa4630c9 100644 --- a/src/api/libcellml/variable.h +++ b/src/api/libcellml/variable.h @@ -181,7 +181,7 @@ class LIBCELLML_EXPORT Variable: public NamedEntity * * @param variable1 Variable one of the equivalence. * @param variable2 Variable two of the equivalence. - * @param deepSearch Optional parameter to deepSearch the equivalence network for the connection identifier, true by default. + * @param deepSearch Optional parameter to deep search the equivalence network for the connection identifier, true by default. * @return the @c std::string connection identifier. */ static std::string equivalenceConnectionId(const VariablePtr &variable1, const VariablePtr &variable2, bool deepSearch = true); From 797d696b1d753ce9ea9fce2ce6f9dcbc7f1b8746 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 10:55:22 +1200 Subject: [PATCH 21/36] Analyser: some minor cleaning up. --- src/analyser.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/analyser.cpp b/src/analyser.cpp index 16691bf10a..e536af80b6 100644 --- a/src/analyser.cpp +++ b/src/analyser.cpp @@ -402,6 +402,7 @@ AnalyserInternalVariablePtr Analyser::AnalyserImpl::internalVariable(const Varia { // Find and return, if there is one, the internal variable associated with // the given variable. + auto rawPtr = reinterpret_cast(variable.get()); if (mInternalVariableMap.count(rawPtr) > 0) { return mInternalVariableMap[rawPtr]; @@ -410,6 +411,7 @@ AnalyserInternalVariablePtr Analyser::AnalyserImpl::internalVariable(const Varia for (const auto &internalVariable : mInternalVariables) { if (mAnalyserModel->areEquivalentVariables(variable, internalVariable->mVariable)) { mInternalVariableMap[rawPtr] = internalVariable; + return internalVariable; } } @@ -420,6 +422,7 @@ AnalyserInternalVariablePtr Analyser::AnalyserImpl::internalVariable(const Varia auto res = AnalyserInternalVariable::create(variable); mInternalVariables.push_back(res); + mInternalVariableMap[rawPtr] = res; return res; From ed5200865f8ecfb2ed6db4add5bdeddffe52a6f3 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 10:55:59 +1200 Subject: [PATCH 22/36] Analyser: slight improvement (one lookup instead of two). --- src/analyser.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/analyser.cpp b/src/analyser.cpp index e536af80b6..401936d6f0 100644 --- a/src/analyser.cpp +++ b/src/analyser.cpp @@ -404,8 +404,10 @@ AnalyserInternalVariablePtr Analyser::AnalyserImpl::internalVariable(const Varia // the given variable. auto rawPtr = reinterpret_cast(variable.get()); - if (mInternalVariableMap.count(rawPtr) > 0) { - return mInternalVariableMap[rawPtr]; + auto rawPtrIt = mInternalVariableMap.find(rawPtr); + + if (rawPtrIt != mInternalVariableMap.end()) { + return rawPtrIt->second; } for (const auto &internalVariable : mInternalVariables) { From cd80609ac8bb3835a4d94b71be54637db360fa87 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 11:05:07 +1200 Subject: [PATCH 23/36] Test utils: reverted changes made to fileContents(). We don't take advantage of the optional `absolute` parameter. (Regarding `file.close()`, it's done automatically, so no actual need for it.) --- tests/test_utils.cpp | 10 ++-------- tests/test_utils.h | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 483fe307ad..21f57ed540 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -35,18 +35,12 @@ std::string resourcePath(const std::string &resourceRelativePath) return TESTS_RESOURCE_LOCATION + "/" + resourceRelativePath; } -std::string fileContents(const std::string &fileName, bool absolute) +std::string fileContents(const std::string &fileName) { - std::ifstream file; - if (absolute) { - file.open(fileName); - } else { - file.open(resourcePath(fileName)); - } + std::ifstream file(resourcePath(fileName)); std::stringstream buffer; buffer << file.rdbuf(); - file.close(); return buffer.str(); } diff --git a/tests/test_utils.h b/tests/test_utils.h index 199a9e5e38..1199138682 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -82,7 +82,7 @@ std::chrono::steady_clock::time_point TEST_EXPORT timeNow(); int TEST_EXPORT elapsedTime(const std::chrono::steady_clock::time_point &startTime); std::string TEST_EXPORT resourcePath(const std::string &resourceRelativePath = ""); -std::string TEST_EXPORT fileContents(const std::string &fileName, bool absolute = false); +std::string TEST_EXPORT fileContents(const std::string &fileName); void TEST_EXPORT printIssues(const libcellml::LoggerPtr &l, bool headings = false, bool cellmlElementTypes = false, bool rule = false); void TEST_EXPORT printModel(const libcellml::ModelPtr &model, bool includeMaths = true); From 5713d32701161d62168fb5f7963ecb6be495ea42 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 11:33:33 +1200 Subject: [PATCH 24/36] AnalyserModel: various speed improvements. - Converted `std::set` to `std::unordered_set`, resulting in O(1) average per operation instead of O(log n) per operation. - Replaced double hash lookup with a single one. --- src/analysermodel.cpp | 13 ++++++------- src/analysermodel_p.h | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 958f5e668b..181b0de8db 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -48,12 +48,11 @@ AnalyserModel::~AnalyserModel() delete mPimpl; } -void exploreEquivalentVariables(const VariablePtr &variable, std::set &equivalentGroup, std::set &visited) +void exploreEquivalentVariables(const VariablePtr &variable, std::unordered_set &equivalentGroup, std::unordered_set &visited) { auto rawPtr = reinterpret_cast(variable.get()); - if (visited.count(rawPtr) == 0) { - visited.insert(rawPtr); + if (visited.insert(rawPtr).second) { equivalentGroup.insert(rawPtr); for (size_t i = 0; i < variable->equivalentVariableCount(); ++i) { @@ -64,8 +63,8 @@ void exploreEquivalentVariables(const VariablePtr &variable, std::set void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache() { - std::set visited; - std::vector> equivalentVariableGroups; + std::unordered_set visited; + std::vector> equivalentVariableGroups; mEquivalentVariableCache.clear(); for (size_t i = 0; i < mModel->componentCount(); ++i) { @@ -73,14 +72,14 @@ void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache() } } -void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups) +void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const ComponentPtr &component, std::unordered_set &visited, std::vector> &equivalentVariableGroups) { for (size_t i = 0; i < component->variableCount(); ++i) { auto variable = component->variable(i); auto rawPtr = reinterpret_cast(variable.get()); if (visited.count(rawPtr) == 0) { - std::set equivalentGroup; + std::unordered_set equivalentGroup; exploreEquivalentVariables(variable, equivalentGroup, visited); size_t groupIndex = equivalentVariableGroups.size(); diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index 937e689d98..138cf14629 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -17,8 +17,8 @@ limitations under the License. #pragma once #include -#include #include +#include #include "libcellml/analysermodel.h" @@ -78,7 +78,7 @@ struct AnalyserModel::AnalyserModelImpl static AnalyserModelPtr create(const ModelPtr &model = nullptr); - void buildEquivalentVariablesCache(const ComponentPtr &component, std::set &visited, std::vector> &equivalentVariableGroups); + void buildEquivalentVariablesCache(const ComponentPtr &component, std::unordered_set &visited, std::vector> &equivalentVariableGroups); void buildEquivalentVariablesCache(); AnalyserModelImpl(const ModelPtr &model); From 4906bb8079f009810c79fa2270321408f0c33e41 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 11:41:26 +1200 Subject: [PATCH 25/36] AnalyserModel: replaced `equivalentVariableGroups` with `groupCount`. `equivalentVariableGroups` wasn't being used as such. So, to replace it with `groupCount` means that we: - eliminate n copies of std::unordered_set; - remove all associated heap allocations from those sets and the vector itself; and - save memory by not storing data that's never queried. --- src/analysermodel.cpp | 14 +++++++------- src/analysermodel_p.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 181b0de8db..18dbf1096b 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -64,15 +64,15 @@ void exploreEquivalentVariables(const VariablePtr &variable, std::unordered_set< void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache() { std::unordered_set visited; - std::vector> equivalentVariableGroups; + size_t groupCount = 0; mEquivalentVariableCache.clear(); for (size_t i = 0; i < mModel->componentCount(); ++i) { - buildEquivalentVariablesCache(mModel->component(i), visited, equivalentVariableGroups); + buildEquivalentVariablesCache(mModel->component(i), visited, groupCount); } } -void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const ComponentPtr &component, std::unordered_set &visited, std::vector> &equivalentVariableGroups) +void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const ComponentPtr &component, std::unordered_set &visited, size_t &groupCount) { for (size_t i = 0; i < component->variableCount(); ++i) { auto variable = component->variable(i); @@ -81,17 +81,17 @@ void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const Compo if (visited.count(rawPtr) == 0) { std::unordered_set equivalentGroup; exploreEquivalentVariables(variable, equivalentGroup, visited); - size_t groupIndex = equivalentVariableGroups.size(); for (uintptr_t v : equivalentGroup) { - mEquivalentVariableCache[v] = groupIndex; + mEquivalentVariableCache[v] = groupCount; } - equivalentVariableGroups.push_back(equivalentGroup); + + ++groupCount; } } for (size_t i = 0; i < component->componentCount(); ++i) { - buildEquivalentVariablesCache(component->component(i), visited, equivalentVariableGroups); + buildEquivalentVariablesCache(component->component(i), visited, groupCount); } } diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index 138cf14629..89f9dd4319 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -78,7 +78,7 @@ struct AnalyserModel::AnalyserModelImpl static AnalyserModelPtr create(const ModelPtr &model = nullptr); - void buildEquivalentVariablesCache(const ComponentPtr &component, std::unordered_set &visited, std::vector> &equivalentVariableGroups); + void buildEquivalentVariablesCache(const ComponentPtr &component, std::unordered_set &visited, size_t &groupCount); void buildEquivalentVariablesCache(); AnalyserModelImpl(const ModelPtr &model); From 2ee4d2486edc89b96dead438c5253bae25244df8 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 11:46:16 +1200 Subject: [PATCH 26/36] AnalyserModel: replaced some double hash lookups with one. --- src/analysermodel.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 18dbf1096b..219a4f97bd 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -561,8 +561,19 @@ bool AnalyserModel::areEquivalentVariables(const VariablePtr &variable1, const auto v1 = reinterpret_cast(variable1.get()); const auto v2 = reinterpret_cast(variable2.get()); - return (mPimpl->mEquivalentVariableCache.count(v1) > 0) - && (mPimpl->mEquivalentVariableCache[v1] == mPimpl->mEquivalentVariableCache[v2]); + const auto it1 = mPimpl->mEquivalentVariableCache.find(v1); + + if (it1 == mPimpl->mEquivalentVariableCache.end()) { + return false; + } + + const auto it2 = mPimpl->mEquivalentVariableCache.find(v2); + + if (it2 == mPimpl->mEquivalentVariableCache.end()) { + return false; + } + + return it1->second == it2->second; } } // namespace libcellml From 65b890269bf4588904911d345eef1130ecd90ffa Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 11:56:51 +1200 Subject: [PATCH 27/36] AnalyserModel: slight improvements to `typeAsString()` and `hasExternalVariables()`. --- src/analysermodel.cpp | 59 ++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 219a4f97bd..bad72ea7bb 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -113,20 +113,21 @@ AnalyserModel::Type AnalyserModel::type() const return mPimpl->mType; } -static const std::map typeToString = { - {AnalyserModel::Type::UNKNOWN, "unknown"}, - {AnalyserModel::Type::ODE, "ode"}, - {AnalyserModel::Type::DAE, "dae"}, - {AnalyserModel::Type::NLA, "nla"}, - {AnalyserModel::Type::ALGEBRAIC, "algebraic"}, - {AnalyserModel::Type::INVALID, "invalid"}, - {AnalyserModel::Type::UNDERCONSTRAINED, "underconstrained"}, - {AnalyserModel::Type::OVERCONSTRAINED, "overconstrained"}, - {AnalyserModel::Type::UNSUITABLY_CONSTRAINED, "unsuitably_constrained"}}; - std::string AnalyserModel::typeAsString(Type type) { - return typeToString.at(type); + static constexpr const char *names[] = { + "unknown", + "algebraic", + "dae", + "invalid", + "nla", + "ode", + "overconstrained", + "underconstrained", + "unsuitably_constrained" + }; + + return names[static_cast(type)]; } bool AnalyserModel::hasExternalVariables() const @@ -289,9 +290,37 @@ AnalyserVariablePtr AnalyserModel::analyserVariable(const VariablePtr &variable) return {}; } - for (const auto &analyserVariable : analyserVariables(shared_from_this())) { - if (areEquivalentVariables(variable, analyserVariable->variable())) { - return analyserVariable; + if (mPimpl->mVoi && areEquivalentVariables(variable, mPimpl->mVoi->variable())) { + return mPimpl->mVoi; + } + + for (const auto &state : mPimpl->mStates) { + if (areEquivalentVariables(variable, state->variable())) { + return state; + } + } + + for (const auto &constant : mPimpl->mConstants) { + if (areEquivalentVariables(variable, constant->variable())) { + return constant; + } + } + + for (const auto &computedConstant : mPimpl->mComputedConstants) { + if (areEquivalentVariables(variable, computedConstant->variable())) { + return computedConstant; + } + } + + for (const auto &algebraicVariable : mPimpl->mAlgebraicVariables) { + if (areEquivalentVariables(variable, algebraicVariable->variable())) { + return algebraicVariable; + } + } + + for (const auto &externalVariable : mPimpl->mExternalVariables) { + if (areEquivalentVariables(variable, externalVariable->variable())) { + return externalVariable; } } From 7fdb63adacebb1afb76f1be88fb90f1c7e12f642 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 12:48:37 +1200 Subject: [PATCH 28/36] Validator: make gatherComponents() iterative rather than recursive. --- src/validator.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/validator.cpp b/src/validator.cpp index f8eeebb1c3..115f66c500 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2695,10 +2695,20 @@ void Validator::ValidatorImpl::addIdMapItem(const std::string &id, const std::st void gatherComponents(const ComponentPtr &component, std::vector &allComponents) { - allComponents.push_back(component); + std::vector stack; - for (size_t c = 0; c < component->componentCount(); ++c) { - gatherComponents(component->component(c), allComponents); + stack.push_back(component); + + while (!stack.empty()) { + allComponents.emplace_back(std::move(stack.back())); + stack.pop_back(); + + auto *current = allComponents.back().get(); + const auto childCount = current->componentCount(); + + for (size_t i = 0; i < childCount; ++i) { + stack.emplace_back(current->component(i)); + } } } From 5e34db3871f8675eefc8d3c9c189602a8b806b1b Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 12:56:33 +1200 Subject: [PATCH 29/36] Validator: improved the populating of connectionIds. --- src/validator.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/validator.cpp b/src/validator.cpp index 115f66c500..61f7eb6d73 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2714,14 +2714,6 @@ void gatherComponents(const ComponentPtr &component, std::vector & IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) { - struct PairHash - { - size_t operator()(const ComponentRawPtrPair &p) const - { - return std::hash()(p.first) ^ (std::hash()(p.second) << 1); - } - }; - IdMap idMap; std::string info; std::set reportedConnections; @@ -2732,7 +2724,6 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) } ConnectionIdMap connectionIds; - std::unordered_set visitedPairs; for (const auto &comp : allComponents) { auto rawPtr = comp.get(); @@ -2748,11 +2739,11 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) if (equivParent != nullptr) { auto equivRawPtr = equivParent.get(); auto key = (rawPtr < equivRawPtr) ? ComponentRawPtrPair {rawPtr, equivRawPtr} : ComponentRawPtrPair {equivRawPtr, rawPtr}; + auto [iter, inserted] = connectionIds.try_emplace(key, Variable::equivalenceConnectionId(currentVariable, equiv, false)); - if (!visitedPairs.insert(key).second) { + if (!inserted) { continue; // Skip if we've already processed this pair } - connectionIds[key] = Variable::equivalenceConnectionId(currentVariable, equiv, false); } } } From 9abadbc3e5fbc1eb909a80cfff82c90bd327b2de Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 13:39:58 +1200 Subject: [PATCH 30/36] Variable: some minor cleaning up. --- src/variable.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/variable.cpp b/src/variable.cpp index 12e7f4f221..06983ee978 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -127,6 +127,7 @@ bool Variable::removeEquivalence(const VariablePtr &variable1, const VariablePtr variable2->pFunc()->unsetEquivalentTo(variable1); variable1->pFunc()->unsafeResetEquivalenceIds(variable2); variable2->pFunc()->unsafeResetEquivalenceIds(variable1); + return true; } } @@ -453,6 +454,7 @@ std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, cons for (auto &it : map) { id = it.first->pFunc()->equivalentConnectionId(it.second); + if (!id.empty()) { return id; } From 1cbd582926b72cb8df1ebb26aef6a30962dc22c2 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 11:56:51 +1200 Subject: [PATCH 31/36] AnalyserModel: slight improvements to `typeAsString()` and `hasExternalVariables()`. --- src/analysermodel.cpp | 59 ++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 219a4f97bd..9e86c66c4f 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -113,20 +113,21 @@ AnalyserModel::Type AnalyserModel::type() const return mPimpl->mType; } -static const std::map typeToString = { - {AnalyserModel::Type::UNKNOWN, "unknown"}, - {AnalyserModel::Type::ODE, "ode"}, - {AnalyserModel::Type::DAE, "dae"}, - {AnalyserModel::Type::NLA, "nla"}, - {AnalyserModel::Type::ALGEBRAIC, "algebraic"}, - {AnalyserModel::Type::INVALID, "invalid"}, - {AnalyserModel::Type::UNDERCONSTRAINED, "underconstrained"}, - {AnalyserModel::Type::OVERCONSTRAINED, "overconstrained"}, - {AnalyserModel::Type::UNSUITABLY_CONSTRAINED, "unsuitably_constrained"}}; - std::string AnalyserModel::typeAsString(Type type) { - return typeToString.at(type); + static constexpr const char *names[] = { + "unknown", + "algebraic", + "dae", + "invalid", + "nla", + "ode", + "overconstrained", + "underconstrained", + "unsuitably_constrained", + }; + + return names[static_cast(type)]; } bool AnalyserModel::hasExternalVariables() const @@ -289,9 +290,37 @@ AnalyserVariablePtr AnalyserModel::analyserVariable(const VariablePtr &variable) return {}; } - for (const auto &analyserVariable : analyserVariables(shared_from_this())) { - if (areEquivalentVariables(variable, analyserVariable->variable())) { - return analyserVariable; + if (mPimpl->mVoi && areEquivalentVariables(variable, mPimpl->mVoi->variable())) { + return mPimpl->mVoi; + } + + for (const auto &state : mPimpl->mStates) { + if (areEquivalentVariables(variable, state->variable())) { + return state; + } + } + + for (const auto &constant : mPimpl->mConstants) { + if (areEquivalentVariables(variable, constant->variable())) { + return constant; + } + } + + for (const auto &computedConstant : mPimpl->mComputedConstants) { + if (areEquivalentVariables(variable, computedConstant->variable())) { + return computedConstant; + } + } + + for (const auto &algebraicVariable : mPimpl->mAlgebraicVariables) { + if (areEquivalentVariables(variable, algebraicVariable->variable())) { + return algebraicVariable; + } + } + + for (const auto &externalVariable : mPimpl->mExternalVariables) { + if (areEquivalentVariables(variable, externalVariable->variable())) { + return externalVariable; } } From 5bf9973460d6aa78959a6e2103ff3b0f99d5ae4a Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 12:48:37 +1200 Subject: [PATCH 32/36] Validator: make gatherComponents() iterative rather than recursive. --- src/validator.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/validator.cpp b/src/validator.cpp index f8eeebb1c3..115f66c500 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2695,10 +2695,20 @@ void Validator::ValidatorImpl::addIdMapItem(const std::string &id, const std::st void gatherComponents(const ComponentPtr &component, std::vector &allComponents) { - allComponents.push_back(component); + std::vector stack; - for (size_t c = 0; c < component->componentCount(); ++c) { - gatherComponents(component->component(c), allComponents); + stack.push_back(component); + + while (!stack.empty()) { + allComponents.emplace_back(std::move(stack.back())); + stack.pop_back(); + + auto *current = allComponents.back().get(); + const auto childCount = current->componentCount(); + + for (size_t i = 0; i < childCount; ++i) { + stack.emplace_back(current->component(i)); + } } } From bebf51daf7cf2c4a80996a3d265b2dce11ad1ad9 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 12:56:33 +1200 Subject: [PATCH 33/36] Validator: improved the populating of connectionIds. --- src/validator.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/validator.cpp b/src/validator.cpp index 115f66c500..61f7eb6d73 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2714,14 +2714,6 @@ void gatherComponents(const ComponentPtr &component, std::vector & IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) { - struct PairHash - { - size_t operator()(const ComponentRawPtrPair &p) const - { - return std::hash()(p.first) ^ (std::hash()(p.second) << 1); - } - }; - IdMap idMap; std::string info; std::set reportedConnections; @@ -2732,7 +2724,6 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) } ConnectionIdMap connectionIds; - std::unordered_set visitedPairs; for (const auto &comp : allComponents) { auto rawPtr = comp.get(); @@ -2748,11 +2739,11 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) if (equivParent != nullptr) { auto equivRawPtr = equivParent.get(); auto key = (rawPtr < equivRawPtr) ? ComponentRawPtrPair {rawPtr, equivRawPtr} : ComponentRawPtrPair {equivRawPtr, rawPtr}; + auto [iter, inserted] = connectionIds.try_emplace(key, Variable::equivalenceConnectionId(currentVariable, equiv, false)); - if (!visitedPairs.insert(key).second) { + if (!inserted) { continue; // Skip if we've already processed this pair } - connectionIds[key] = Variable::equivalenceConnectionId(currentVariable, equiv, false); } } } From 60a44e44d5b70586183a99184fc688cac85ec5f2 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 13:39:58 +1200 Subject: [PATCH 34/36] Variable: some minor cleaning up. --- src/variable.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/variable.cpp b/src/variable.cpp index 12e7f4f221..06983ee978 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -127,6 +127,7 @@ bool Variable::removeEquivalence(const VariablePtr &variable1, const VariablePtr variable2->pFunc()->unsetEquivalentTo(variable1); variable1->pFunc()->unsafeResetEquivalenceIds(variable2); variable2->pFunc()->unsafeResetEquivalenceIds(variable1); + return true; } } @@ -453,6 +454,7 @@ std::string Variable::equivalenceConnectionId(const VariablePtr &variable1, cons for (auto &it : map) { id = it.first->pFunc()->equivalentConnectionId(it.second); + if (!id.empty()) { return id; } From 59a5a0bbf396cb8a3640e9b8f5df7b12b1731589 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Mon, 22 Jun 2026 14:07:20 +1200 Subject: [PATCH 35/36] Addressed a coverage issue. It was in `AnalyserModel::areEquivalentVariables()`. `it2 == mPimpl->mEquivalentVariableCache.end()` never evaluated to `true`. --- tests/coverage/coverage.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/coverage/coverage.cpp b/tests/coverage/coverage.cpp index 18f567abe8..3a8aa192e5 100644 --- a/tests/coverage/coverage.cpp +++ b/tests/coverage/coverage.cpp @@ -602,6 +602,8 @@ TEST(Coverage, analyserAreEquivalentVariables) auto variable = model->component("membrane")->variable("V"); EXPECT_FALSE(analyserModel->areEquivalentVariables(nullptr, variable)); EXPECT_FALSE(analyserModel->areEquivalentVariables(variable, nullptr)); + auto otherVariable = libcellml::Variable::create("other"); + EXPECT_FALSE(analyserModel->areEquivalentVariables(variable, otherVariable)); } void checkAstTypeAsString(const libcellml::AnalyserEquationAstPtr &ast) From d3e759d563c006a6213253bfbb2d962861f8633c Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Mon, 22 Jun 2026 15:38:54 +1200 Subject: [PATCH 36/36] Remove some redundant code. --- src/validator.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/validator.cpp b/src/validator.cpp index 61f7eb6d73..3a581c76e4 100644 --- a/src/validator.cpp +++ b/src/validator.cpp @@ -2739,11 +2739,7 @@ IdMap Validator::ValidatorImpl::buildModelIdMap(const ModelPtr &model) if (equivParent != nullptr) { auto equivRawPtr = equivParent.get(); auto key = (rawPtr < equivRawPtr) ? ComponentRawPtrPair {rawPtr, equivRawPtr} : ComponentRawPtrPair {equivRawPtr, rawPtr}; - auto [iter, inserted] = connectionIds.try_emplace(key, Variable::equivalenceConnectionId(currentVariable, equiv, false)); - - if (!inserted) { - continue; // Skip if we've already processed this pair - } + connectionIds.try_emplace(key, Variable::equivalenceConnectionId(currentVariable, equiv, false)); } } }