Investigations into caching equivalent variables

src/analyser.cpp

@@ -402,9 +402,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<uintptr_t>(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;
         }
     }
@@ -415,6 +420,7 @@ AnalyserInternalVariablePtr Analyser::AnalyserImpl::internalVariable(const Varia
     auto res = AnalyserInternalVariable::create(variable);
 
     mInternalVariables.push_back(res);
+    mInternalVariableMap[rawPtr] = res;
 
     return res;
 }
@@ -2321,6 +2327,7 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model)
     mAnalyserModel = AnalyserModel::AnalyserModelImpl::create(model);
 
     mInternalVariables.clear();
+    mInternalVariableMap.clear();
     mInternalEquations.clear();
 
     mCiCnUnits.clear();

src/analyser_p.h

@@ -161,6 +161,7 @@ class Analyser::AnalyserImpl: public Logger::LoggerImpl
     AnalyserExternalVariablePtrs mExternalVariables;
 
     AnalyserInternalVariablePtrs mInternalVariables;
+    std::unordered_map<std::uintptr_t, AnalyserInternalVariablePtr> mInternalVariableMap;
     AnalyserInternalEquationPtrs mInternalEquations;
 
     GeneratorProfilePtr mGeneratorProfile = GeneratorProfile::create();

src/analysermodel.cpp

@@ -48,35 +48,51 @@ AnalyserModel::~AnalyserModel()
     delete mPimpl;
 }
 
-void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const ComponentPtr &component)
+void exploreEquivalentVariables(const VariablePtr &variable, std::set<uintptr_t> &equivalentGroup, std::set<uintptr_t> &visited)
 {
-    for (size_t i = 0; i < component->variableCount(); ++i) {
-        auto variable = component->variable(i);
+    auto rawPtr = reinterpret_cast<uintptr_t>(variable.get());
 
-        for (size_t j = 0; j < variable->equivalentVariableCount(); ++j) {
-            auto equivalentVariable = variable->equivalentVariable(j);
-            auto v1 = reinterpret_cast<uintptr_t>(variable.get());
-            auto v2 = reinterpret_cast<uintptr_t>(equivalentVariable.get());
+    if (visited.count(rawPtr) == 0) {
+        visited.insert(rawPtr);
+        equivalentGroup.insert(rawPtr);
 
-            if (v2 < v1) {
-                std::swap(v1, v2);
-            }
-
-            uniteEquivalentVariableAddresses(v1, v2);
+        for (size_t i = 0; i < variable->equivalentVariableCount(); ++i) {
+            exploreEquivalentVariables(variable->equivalentVariable(i), equivalentGroup, visited);
         }
     }
-
-    for (size_t i = 0; i < component->componentCount(); ++i) {
-        buildEquivalentVariablesCache(component->component(i));
-    }
 }
 
 void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache()
 {
+    std::set<uintptr_t> visited;
+    std::vector<std::set<uintptr_t>> equivalentVariableGroups;
     mEquivalentVariableCache.clear();
 
     for (size_t i = 0; i < mModel->componentCount(); ++i) {
-        buildEquivalentVariablesCache(mModel->component(i));
+        buildEquivalentVariablesCache(mModel->component(i), visited, equivalentVariableGroups);
+    }
+}
+
+void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache(const ComponentPtr &component, std::set<uintptr_t> &visited, std::vector<std::set<uintptr_t>> &equivalentVariableGroups)
+{
+    for (size_t i = 0; i < component->variableCount(); ++i) {
+        auto variable = component->variable(i);
+        auto rawPtr = reinterpret_cast<uintptr_t>(variable.get());
+
+        if (visited.count(rawPtr) == 0) {
+            std::set<uintptr_t> equivalentGroup;
+            exploreEquivalentVariables(variable, equivalentGroup, visited);
+            size_t groupIndex = equivalentVariableGroups.size();
+
+            for (uintptr_t v : equivalentGroup) {
+                mEquivalentVariableCache[v] = groupIndex;
+            }
+            equivalentVariableGroups.push_back(equivalentGroup);
+        }
+    }
+
+    for (size_t i = 0; i < component->componentCount(); ++i) {
+        buildEquivalentVariablesCache(component->component(i), visited, equivalentVariableGroups);
     }
 }
 
@@ -546,7 +562,8 @@ bool AnalyserModel::areEquivalentVariables(const VariablePtr &variable1,
     const auto v1 = reinterpret_cast<uintptr_t>(variable1.get());
     const auto v2 = reinterpret_cast<uintptr_t>(variable2.get());
 
-    return mPimpl->findVariableAddress(v1) == mPimpl->findVariableAddress(v2);
+    return (mPimpl->mEquivalentVariableCache.count(v1) > 0)
+           && (mPimpl->mEquivalentVariableCache[v1] == mPimpl->mEquivalentVariableCache[v2]);
 }
 
 } // namespace libcellml

src/analysermodel_p.h

@@ -17,6 +17,7 @@ limitations under the License.
 #pragma once
 
 #include <cstdint>
+#include <set>
 #include <unordered_map>
 
 #include "libcellml/analysermodel.h"
@@ -46,34 +47,7 @@ struct AnalyserModel::AnalyserModelImpl
 
     std::vector<AnalyserEquationPtr> mAnalyserEquations;
 
-    std::unordered_map<uintptr_t, uintptr_t> mEquivalentVariableCache;
-
-    uintptr_t findVariableAddress(uintptr_t x)
-    {
-        auto it = mEquivalentVariableCache.find(x);
-
-        if (it == mEquivalentVariableCache.end()) {
-            mEquivalentVariableCache[x] = x;
-
-            return x;
-        }
-
-        if (it->second != x) {
-            it->second = findVariableAddress(it->second);
-        }
-
-        return it->second;
-    }
-
-    void uniteEquivalentVariableAddresses(uintptr_t x, uintptr_t y)
-    {
-        const uintptr_t &rootX = findVariableAddress(x);
-        const uintptr_t &rootY = findVariableAddress(y);
-
-        if (rootX != rootY) {
-            mEquivalentVariableCache[rootY] = rootX;
-        }
-    }
+    std::unordered_map<uintptr_t, size_t> mEquivalentVariableCache;
 
     bool mNeedEqFunction = false;
     bool mNeedNeqFunction = false;
@@ -104,8 +78,8 @@ struct AnalyserModel::AnalyserModelImpl
 
     static AnalyserModelPtr create(const ModelPtr &model = nullptr);
 
-    void buildEquivalentVariablesCache(const ComponentPtr &component);
     void buildEquivalentVariablesCache();
+    void buildEquivalentVariablesCache(const ComponentPtr &component, std::set<uintptr_t> &visited, std::vector<std::set<uintptr_t>> &equivalentVariableGroups);
 
     AnalyserModelImpl(const ModelPtr &model);
 };

src/api/libcellml/analysermodel.h

@@ -604,10 +604,6 @@ class LIBCELLML_EXPORT AnalyserModel
      * analysis phase (@ref Analyser::analyseModel). The cache may become
      * out of date if the model is changed after the model has been analysed.
      *
-     * @note This function is primarily designed for use during model analysis
-     *       by the @ref Analyser. While external usage is not programmatically
-     *       restricted, it is not the primary intended use case.
-     *
      * @param variable1 The @ref Variable to test if it is equivalent to
      * @p variable2.
      * @param variable2 The @ref Variable that is potentially equivalent to
@@ -616,8 +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. */

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.

src/internaltypes.h

@@ -42,13 +42,14 @@ using UniqueNames = std::set<std::string>; /**< Type definition for a set of uni
 using NodeAttributeNamespaceInfo = std::vector<std::tuple<std::string, std::string, std::string, std::string, std::string>>; /**< Type definition for attribute namespace information. */
 
 // VariableMap
+using VariableStdPair = std::pair<VariablePtr, VariablePtr>; /**< Type definition for Variable pointer pair using standard library. */
 using VariableMap = std::vector<VariablePairPtr>; /**< 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<ComponentPtr, ComponentPtr>; /**< Type definition for Component pointer pair.*/
-using ComponentMap = std::vector<ComponentPair>; /**< Type definition for vector of ComponentPair.*/
-using ComponentMapIterator = ComponentMap::const_iterator; /**< Type definition of const iterator for vector of ComponentPair.*/
+using ComponentStdPair = std::pair<ComponentPtr, ComponentPtr>; /**< Type definition for Component pointer pair using standard library.*/
+using ComponentMap = std::vector<ComponentStdPair>; /**< Type definition for vector of ComponentStdPair.*/
+using ComponentMapIterator = ComponentMap::const_iterator; /**< Type definition of const iterator for vector of ComponentStdPair.*/
 
 using VariablePtrs = std::vector<VariablePtr>; /**< Type definition for list of variables. */
 
@@ -79,6 +80,9 @@ using UnitsConstPtr = std::shared_ptr<const Units>; /**< Type definition for sha
 using ConnectionMap = std::map<VariablePtr, VariablePtr>; /**< Type definition for a connection map.*/
 using NamePairList = std::vector<NamePair>; /**< Type definition for a list of a pair of names. */
 
+using ComponentRawPtrPair = std::pair<const Component *, const Component *>; /**< Type definition for pair of raw component pointers. */
+using ConnectionIdMap = std::map<ComponentRawPtrPair, std::string>; /**< 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.
  *

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);
             }
         }

src/validator.cpp

@@ -639,8 +639,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<std::string> &reportedConnections);
+    void buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set<std::string> &reportedConnections, const ConnectionIdMap &connectionIds);
 
     /** @brief Utility function to add an item to the idMap.
      *
@@ -2692,11 +2693,56 @@ void Validator::ValidatorImpl::addIdMapItem(const std::string &id, const std::st
     }
 }
 
+void gatherComponents(const ComponentPtr &component, std::vector<ComponentPtr> &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<std::string> reportedConnections;
+
+    std::vector<ComponentPtr> allComponents;
+    for (size_t c = 0; c < model->componentCount(); ++c) {
+        gatherComponents(model->component(c), allComponents);
+    }
+
+    struct PairHash
+    {
+        size_t operator()(const ComponentRawPtrPair &p) const
+        {
+            return std::hash<const Component *>()(p.first) ^ (std::hash<const Component *>()(p.second) << 1);
+        }
+    };
+
+    ConnectionIdMap connectionIds;
+    std::unordered_set<ComponentRawPtrPair, PairHash> 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
+                    }
+                    connectionIds[key] = Variable::equivalenceConnectionId(currentVariable, equiv, false);
+                }
+            }
+        }
+    }
+
     // Model.
     if (!model->id().empty()) {
         info = " - model '" + model->name() + "'";
@@ -2748,12 +2794,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<std::string> &reportedConnections)
+void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component, IdMap &idMap, std::set<std::string> &reportedConnections, const ConnectionIdMap &connectionIds)
 {
     std::string info;
 
@@ -2807,7 +2853,10 @@ void Validator::ValidatorImpl::buildComponentIdMap(const ComponentPtr &component
                     addIdMapItem(mappingId, info, idMap);
                 }
                 // Connections.
-                auto connectionId = Variable::equivalenceConnectionId(item, equiv);
+                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 =
@@ -2879,7 +2928,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);
     }
 }