From c06e1b805a85e724e9568c691b5c5d335469a6c7 Mon Sep 17 00:00:00 2001 From: Finbar Argus Date: Sun, 11 Oct 2026 10:27:54 +1300 Subject: [PATCH 1/3] Generator, parser and analyser: lookups instead of whole-model scans - AnalyserModel::analyserVariable(): a map from equivalence group to analyser variable, built at the end of analysis. - Generator: each variable's index among the tracked variables of its list, computed once per list and generation. - Parser: connections find their components in a map of the model's components by name. - Analyser: the equations computing each variable, gathered in one pass over the equations. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ew9LjxY3znMKSNg8AK2kSn --- src/analyser.cpp | 18 ++++++++- src/analysermodel.cpp | 61 ++++++++++++++++--------------- src/analysermodel_p.h | 4 ++ src/generator.cpp | 20 ++++++---- src/generator_p.h | 4 ++ src/parser.cpp | 54 ++++++++++++++++++++++++--- tests/component/encapsulation.cpp | 3 ++ tests/coverage/coverage.cpp | 1 + 8 files changed, 119 insertions(+), 46 deletions(-) diff --git a/src/analyser.cpp b/src/analyser.cpp index 1c3594e124..b2f00377d8 100644 --- a/src/analyser.cpp +++ b/src/analyser.cpp @@ -2935,6 +2935,16 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model) auto algebraicVariableIndex = MAX_SIZE_T; auto externalVariableIndex = MAX_SIZE_T; + // Retrieve the equations used to compute each variable, in equation order. + + std::unordered_map> equationsOfVariable; + + for (const auto &internalEquation : mInternalEquations) { + for (const auto &unknownVariable : internalEquation->mUnknownVariables) { + equationsOfVariable[unknownVariable.get()].push_back(internalEquation); + } + } + for (const auto &internalVariable : mInternalVariables) { // Determine the type of the variable. @@ -2974,8 +2984,10 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model) AnalyserEquationPtrs equations; auto isNlaEquation = false; - for (const auto &internalEquation : mInternalEquations) { - if (std::find(internalEquation->mUnknownVariables.begin(), internalEquation->mUnknownVariables.end(), internalVariable) != internalEquation->mUnknownVariables.end()) { + auto computing = equationsOfVariable.find(internalVariable.get()); + + if (computing != equationsOfVariable.end()) { + for (const auto &internalEquation : computing->second) { equations.push_back(aie2aeMappings[internalEquation]); if ((aie2aetMappings.find(internalEquation) != aie2aetMappings.end()) @@ -3167,6 +3179,8 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model) analyserEquation->mPimpl->mIsStateRateBased = isStateRateBased(analyserEquation, checkedEquations); } + + mAnalyserModel->mPimpl->buildAnalyserVariableByGroup(); } AnalyserExternalVariablePtrs::const_iterator Analyser::AnalyserImpl::findExternalVariable(const VariablePtr &variable) const diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 9e86c66c4f..323dd42388 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -284,47 +284,48 @@ AnalyserVariablePtr AnalyserModel::externalVariable(size_t index) const return mPimpl->mExternalVariables[index]; } -AnalyserVariablePtr AnalyserModel::analyserVariable(const VariablePtr &variable) +void AnalyserModel::AnalyserModelImpl::buildAnalyserVariableByGroup() { - if (!isValid() || (variable == nullptr)) { - return {}; - } + // A group keeps its first analyser variable, in the order: variable of integration, states, constants, computed + // constants, algebraic variables, external variables. - if (mPimpl->mVoi && areEquivalentVariables(variable, mPimpl->mVoi->variable())) { - return mPimpl->mVoi; - } + auto add = [this](const AnalyserVariablePtr &analyserVariable) { + mAnalyserVariableByGroup.emplace(mEquivalentVariableCache.at(reinterpret_cast(analyserVariable->variable().get())), analyserVariable); + }; - for (const auto &state : mPimpl->mStates) { - if (areEquivalentVariables(variable, state->variable())) { - return state; - } + if (mVoi != nullptr) { + add(mVoi); } - - for (const auto &constant : mPimpl->mConstants) { - if (areEquivalentVariables(variable, constant->variable())) { - return constant; - } + for (const auto &state : mStates) { + add(state); } - - for (const auto &computedConstant : mPimpl->mComputedConstants) { - if (areEquivalentVariables(variable, computedConstant->variable())) { - return computedConstant; - } + for (const auto &constant : mConstants) { + add(constant); + } + for (const auto &computedConstant : mComputedConstants) { + add(computedConstant); } + for (const auto &algebraicVariable : mAlgebraicVariables) { + add(algebraicVariable); + } + for (const auto &externalVariable : mExternalVariables) { + add(externalVariable); + } +} - for (const auto &algebraicVariable : mPimpl->mAlgebraicVariables) { - if (areEquivalentVariables(variable, algebraicVariable->variable())) { - return algebraicVariable; - } +AnalyserVariablePtr AnalyserModel::analyserVariable(const VariablePtr &variable) +{ + if (!isValid() || (variable == nullptr)) { + return {}; } - for (const auto &externalVariable : mPimpl->mExternalVariables) { - if (areEquivalentVariables(variable, externalVariable->variable())) { - return externalVariable; - } + auto group = mPimpl->mEquivalentVariableCache.find(reinterpret_cast(variable.get())); + + if (group == mPimpl->mEquivalentVariableCache.end()) { + return {}; } - return {}; + return mPimpl->mAnalyserVariableByGroup.at(group->second); } size_t AnalyserModel::analyserEquationCount() const diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index 89f9dd4319..e77448b698 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -49,6 +49,10 @@ struct AnalyserModel::AnalyserModelImpl std::unordered_map mEquivalentVariableCache; + std::unordered_map mAnalyserVariableByGroup; /**< Analyser variable of each equivalence group of mEquivalentVariableCache. */ + + void buildAnalyserVariableByGroup(); + bool mNeedEqFunction = false; bool mNeedNeqFunction = false; bool mNeedLtFunction = false; diff --git a/src/generator.cpp b/src/generator.cpp index 4a4ecc2ec4..15275521c6 100644 --- a/src/generator.cpp +++ b/src/generator.cpp @@ -40,6 +40,7 @@ namespace libcellml { void Generator::GeneratorImpl::reset() { mCode = {}; + mTrackedIndexCache.clear(); } std::string Generator::GeneratorImpl::analyserVariableIndexString(const AnalyserVariablePtr &analyserVariable) @@ -47,28 +48,31 @@ std::string Generator::GeneratorImpl::analyserVariableIndexString(const Analyser // Determine the actual index of the analyser variable in the list of analyser variables by accounting for the fact // that some analyser variables may be untracked. + auto cached = mTrackedIndexCache.find(analyserVariable.get()); + + if (cached != mTrackedIndexCache.end()) { + return convertToString(cached->second); + } + auto analyserVariables = libcellml::analyserVariables(analyserVariable); if (analyserVariables.empty()) { return convertToString(analyserVariable->index()); } - size_t i = MAX_SIZE_T; - size_t res = MAX_SIZE_T; + // Index every variable of this list: a tracked variable's index is the number of tracked variables before it. - for (;;) { - auto analyserVar = analyserVariables[++i]; + size_t res = MAX_SIZE_T; + for (const auto &analyserVar : analyserVariables) { if (isTrackedVariable(analyserVar, true)) { ++res; } - if (analyserVariable == analyserVar) { - break; - } + mTrackedIndexCache[analyserVar.get()] = res; } - return convertToString(res); + return convertToString(mTrackedIndexCache[analyserVariable.get()]); } bool Generator::GeneratorImpl::isTrackedEquation(const AnalyserEquationPtr &analyserEquation, bool tracked) diff --git a/src/generator_p.h b/src/generator_p.h index 5c1a3f642a..e8f900f7f9 100644 --- a/src/generator_p.h +++ b/src/generator_p.h @@ -16,6 +16,8 @@ limitations under the License. #pragma once +#include + #include "libcellml/generator.h" #include "libcellml/generatorprofile.h" @@ -42,6 +44,8 @@ struct Generator::GeneratorImpl: public Logger::LoggerImpl GeneratorProfilePtr mProfile; GeneratorVariableTrackerPtr mVariableTracker; + std::unordered_map mTrackedIndexCache; /**< Index of a variable among the tracked variables of its list, for the code being generated. */ + void reset(); std::string analyserVariableIndexString(const AnalyserVariablePtr &analyserVariable); diff --git a/src/parser.cpp b/src/parser.cpp index 7728f4ccd6..7efb15d3b5 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -18,6 +18,7 @@ limitations under the License. #include #include +#include #include #include "libcellml/component.h" @@ -99,6 +100,27 @@ class Parser::ParserImpl: public Logger::LoggerImpl */ void loadConnection(const ModelPtr &model, const XmlNodePtr &node, NamePairList &usedConnections); + std::unordered_map mComponentsByName; /**< The model's components by name, for loading its connections. */ + + /** + * @brief Add the components of @p entity and its descendants to mComponentsByName. + * + * A name keeps the first component found in the order Model::component(name, true) searches: an entity's own + * components, then the descendants of each in turn. + * + * @param entity The @c ComponentEntity whose components to add. + */ + void indexComponents(const ComponentEntityPtr &entity); + + /** + * @brief The component named @p name in mComponentsByName, or @c nullptr. + * + * @param name The component's name. + * + * @return The component, or @c nullptr if there is none. + */ + ComponentPtr findComponentByName(const std::string &name) const; + /** * @brief Update the @p model with an encapsulation parsed from @p node. * @@ -602,9 +624,14 @@ void Parser::ParserImpl::loadModel(const ModelPtr &model, const std::string &inp } NamePairList usedConnections; + mComponentsByName.clear(); + if (!connectionNodes.empty()) { + indexComponents(model); + } for (const auto &connectionNode : connectionNodes) { loadConnection(model, connectionNode, usedConnections); } + mComponentsByName.clear(); // Link units to their names. DescriptionList issueList; @@ -997,6 +1024,23 @@ void Parser::ParserImpl::loadVariable(const VariablePtr &variable, const XmlNode } } +void Parser::ParserImpl::indexComponents(const ComponentEntityPtr &entity) +{ + for (size_t i = 0; i < entity->componentCount(); ++i) { + auto component = entity->component(i); + mComponentsByName.emplace(component->name(), component); + } + for (size_t i = 0; i < entity->componentCount(); ++i) { + indexComponents(entity->component(i)); + } +} + +ComponentPtr Parser::ParserImpl::findComponentByName(const std::string &name) const +{ + auto it = mComponentsByName.find(name); + return (it != mComponentsByName.end()) ? it->second : nullptr; +} + void Parser::ParserImpl::loadConnection(const ModelPtr &model, const XmlNodePtr &node, NamePairList &usedConnections) { // Define types for variable and component pairs, and their identifiers. @@ -1294,9 +1338,8 @@ void Parser::ParserImpl::loadConnection(const ModelPtr &model, const XmlNodePtr ComponentPtr component1 = nullptr; ComponentPtr component2 = nullptr; // Now check the objects exist in the model. TODO Remove as is validation? - if (model->containsComponent(componentNamePair.first)) { - component1 = model->component(componentNamePair.first); - } else { + component1 = findComponentByName(componentNamePair.first); + if (component1 == nullptr) { if (!component1Missing) { auto issue = Issue::IssueImpl::create(); issue->mPimpl->setDescription("Connection in model '" + model->name() + "' specifies '" + componentNamePair.first + "' as component_1 but it does not exist in the model."); @@ -1305,9 +1348,8 @@ void Parser::ParserImpl::loadConnection(const ModelPtr &model, const XmlNodePtr addIssue(issue); } } - if (model->containsComponent(componentNamePair.second)) { - component2 = model->component(componentNamePair.second); - } else { + component2 = findComponentByName(componentNamePair.second); + if (component2 == nullptr) { if (!component2Missing) { auto issue = Issue::IssueImpl::create(); issue->mPimpl->setDescription("Connection in model '" + model->name() + "' specifies '" + componentNamePair.second + "' as component_2 but it does not exist in the model."); diff --git a/tests/component/encapsulation.cpp b/tests/component/encapsulation.cpp index 25640cdce9..0e372a9d91 100644 --- a/tests/component/encapsulation.cpp +++ b/tests/component/encapsulation.cpp @@ -369,6 +369,9 @@ TEST(Encapsulation, encapsulatedComponentMethods) // Contains component EXPECT_TRUE(c->containsComponent("comp5")); + c->addComponent(c4n); + EXPECT_TRUE(c->containsComponent("comp3")); + c->removeComponent(c4n); // Get component EXPECT_EQ(c4, c->component("comp4")); const libcellml::ComponentPtr constC4 = c->component("comp4"); diff --git a/tests/coverage/coverage.cpp b/tests/coverage/coverage.cpp index 3a8aa192e5..dff6532aa7 100644 --- a/tests/coverage/coverage.cpp +++ b/tests/coverage/coverage.cpp @@ -604,6 +604,7 @@ TEST(Coverage, analyserAreEquivalentVariables) EXPECT_FALSE(analyserModel->areEquivalentVariables(variable, nullptr)); auto otherVariable = libcellml::Variable::create("other"); EXPECT_FALSE(analyserModel->areEquivalentVariables(variable, otherVariable)); + EXPECT_FALSE(analyserModel->areEquivalentVariables(otherVariable, variable)); } void checkAstTypeAsString(const libcellml::AnalyserEquationAstPtr &ast) From 8c974e8b374cf629b2d6c8a77bb97336abe79369 Mon Sep 17 00:00:00 2001 From: Finbar Argus Date: Mon, 12 Oct 2026 16:09:52 +1300 Subject: [PATCH 2/3] AnalyserModel: document buildAnalyserVariableByGroup() in analysermodel_p.h Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Ew9LjxY3znMKSNg8AK2kSn --- src/analysermodel.cpp | 3 --- src/analysermodel_p.h | 7 +++++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 323dd42388..22c21e7307 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -286,9 +286,6 @@ AnalyserVariablePtr AnalyserModel::externalVariable(size_t index) const void AnalyserModel::AnalyserModelImpl::buildAnalyserVariableByGroup() { - // A group keeps its first analyser variable, in the order: variable of integration, states, constants, computed - // constants, algebraic variables, external variables. - auto add = [this](const AnalyserVariablePtr &analyserVariable) { mAnalyserVariableByGroup.emplace(mEquivalentVariableCache.at(reinterpret_cast(analyserVariable->variable().get())), analyserVariable); }; diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index e77448b698..48358e03fa 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -51,6 +51,13 @@ struct AnalyserModel::AnalyserModelImpl std::unordered_map mAnalyserVariableByGroup; /**< Analyser variable of each equivalence group of mEquivalentVariableCache. */ + /** + * @brief Build the analyser variable of each equivalence group. + * + * Fill @c mAnalyserVariableByGroup from @c mEquivalentVariableCache. A group keeps its first analyser variable, in + * the order: variable of integration, states, constants, computed constants, algebraic variables, external + * variables. + */ void buildAnalyserVariableByGroup(); bool mNeedEqFunction = false; From efd460e033339951ea09f3c75cc6abc251e2e117 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Tue, 13 Oct 2026 13:13:56 +1300 Subject: [PATCH 3/3] Analyser/Generator: some minor cleaning up. --- src/analyser.cpp | 21 ++++++++++----------- src/analysermodel.cpp | 5 +++++ src/analysermodel_p.h | 14 +++----------- src/generator_p.h | 2 +- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/analyser.cpp b/src/analyser.cpp index b2f00377d8..bf3892819f 100644 --- a/src/analyser.cpp +++ b/src/analyser.cpp @@ -2925,16 +2925,6 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model) aie2aetMappings.emplace(internalEquation, type); } - // Make our internal variables available through our API. - - std::map aiv2avMappings; - std::map v2avMappings; - auto stateIndex = MAX_SIZE_T; - auto constantIndex = MAX_SIZE_T; - auto computedConstantIndex = MAX_SIZE_T; - auto algebraicVariableIndex = MAX_SIZE_T; - auto externalVariableIndex = MAX_SIZE_T; - // Retrieve the equations used to compute each variable, in equation order. std::unordered_map> equationsOfVariable; @@ -2945,6 +2935,16 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model) } } + // Make our internal variables available through our API. + + std::map aiv2avMappings; + std::map v2avMappings; + auto stateIndex = MAX_SIZE_T; + auto constantIndex = MAX_SIZE_T; + auto computedConstantIndex = MAX_SIZE_T; + auto algebraicVariableIndex = MAX_SIZE_T; + auto externalVariableIndex = MAX_SIZE_T; + for (const auto &internalVariable : mInternalVariables) { // Determine the type of the variable. @@ -2983,7 +2983,6 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model) AnalyserEquationPtrs equations; auto isNlaEquation = false; - auto computing = equationsOfVariable.find(internalVariable.get()); if (computing != equationsOfVariable.end()) { diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index 22c21e7307..a69055bb7f 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -293,18 +293,23 @@ void AnalyserModel::AnalyserModelImpl::buildAnalyserVariableByGroup() if (mVoi != nullptr) { add(mVoi); } + for (const auto &state : mStates) { add(state); } + for (const auto &constant : mConstants) { add(constant); } + for (const auto &computedConstant : mComputedConstants) { add(computedConstant); } + for (const auto &algebraicVariable : mAlgebraicVariables) { add(algebraicVariable); } + for (const auto &externalVariable : mExternalVariables) { add(externalVariable); } diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index 48358e03fa..b63b9558c5 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -48,17 +48,7 @@ struct AnalyserModel::AnalyserModelImpl std::vector mAnalyserEquations; std::unordered_map mEquivalentVariableCache; - - std::unordered_map mAnalyserVariableByGroup; /**< Analyser variable of each equivalence group of mEquivalentVariableCache. */ - - /** - * @brief Build the analyser variable of each equivalence group. - * - * Fill @c mAnalyserVariableByGroup from @c mEquivalentVariableCache. A group keeps its first analyser variable, in - * the order: variable of integration, states, constants, computed constants, algebraic variables, external - * variables. - */ - void buildAnalyserVariableByGroup(); + std::unordered_map mAnalyserVariableByGroup; bool mNeedEqFunction = false; bool mNeedNeqFunction = false; @@ -92,6 +82,8 @@ struct AnalyserModel::AnalyserModelImpl void buildEquivalentVariablesCache(const ComponentPtr &component, std::unordered_set &visited, size_t &groupCount); void buildEquivalentVariablesCache(); + void buildAnalyserVariableByGroup(); + AnalyserModelImpl(const ModelPtr &model); }; diff --git a/src/generator_p.h b/src/generator_p.h index e8f900f7f9..c1aee62aa7 100644 --- a/src/generator_p.h +++ b/src/generator_p.h @@ -44,7 +44,7 @@ struct Generator::GeneratorImpl: public Logger::LoggerImpl GeneratorProfilePtr mProfile; GeneratorVariableTrackerPtr mVariableTracker; - std::unordered_map mTrackedIndexCache; /**< Index of a variable among the tracked variables of its list, for the code being generated. */ + std::unordered_map mTrackedIndexCache; void reset();