Merge pull request #763 from agarny/cache-isSameOrEquivalentVariable

Analyser/Generator: cache variable equivalences. Closes #762.

src/analyser.cpp

@@ -161,7 +161,8 @@ struct AnalyserInternalEquation
     static bool hasKnownVariables(const std::vector<AnalyserInternalVariablePtr> &variables);
     static bool hasNonConstantVariables(const std::vector<AnalyserInternalVariablePtr> &variables);
 
-    bool check(size_t &equationOrder, size_t &stateIndex, size_t &variableIndex);
+    bool check(size_t &equationOrder, size_t &stateIndex, size_t &variableIndex,
+               const AnalyserModelPtr &model);
 };
 
 AnalyserInternalEquation::AnalyserInternalEquation(const ComponentPtr &component)
@@ -222,7 +223,8 @@ bool AnalyserInternalEquation::hasNonConstantVariables(const std::vector<Analyse
 }
 
 bool AnalyserInternalEquation::check(size_t &equationOrder, size_t &stateIndex,
-                                     size_t &variableIndex)
+                                     size_t &variableIndex,
+                                     const AnalyserModelPtr &model)
 {
     // Nothing to check if the equation has already been given an order (i.e.
     // everything is fine).
@@ -280,7 +282,7 @@ bool AnalyserInternalEquation::check(size_t &equationOrder, size_t &stateIndex,
         for (size_t i = 0; i < mComponent->variableCount(); ++i) {
             auto localVariable = mComponent->variable(i);
 
-            if (isSameOrEquivalentVariable(variable->mVariable, localVariable)) {
+            if (model->areEquivalentVariables(variable->mVariable, localVariable)) {
                 variable->setVariable(localVariable, false);
 
                 break;
@@ -487,7 +489,7 @@ AnalyserInternalVariablePtr Analyser::AnalyserImpl::internalVariable(const Varia
     AnalyserInternalVariablePtr res = nullptr;
 
     for (const auto &internalVariable : mInternalVariables) {
-        if (isSameOrEquivalentVariable(variable, internalVariable->mVariable)) {
+        if (mModel->areEquivalentVariables(variable, internalVariable->mVariable)) {
             res = internalVariable;
 
             break;
@@ -517,7 +519,7 @@ VariablePtr Analyser::AnalyserImpl::voiFirstOccurrence(const VariablePtr &variab
     for (size_t i = 0; i < component->variableCount(); ++i) {
         auto componentVariable = component->variable(i);
 
-        if (isSameOrEquivalentVariable(variable, componentVariable)) {
+        if (mModel->areEquivalentVariables(variable, componentVariable)) {
             return componentVariable;
         }
     }
@@ -1069,7 +1071,7 @@ void Analyser::AnalyserImpl::analyseEquationAst(const AnalyserEquationAstPtr &as
                     break;
                 }
             }
-        } else if (!isSameOrEquivalentVariable(variable, mModel->mPimpl->mVoi->variable())) {
+        } else if (!mModel->areEquivalentVariables(variable, mModel->mPimpl->mVoi->variable())) {
             auto issue = Issue::create();
 
             issue->setDescription("Variable '" + mModel->mPimpl->mVoi->variable()->name()
@@ -1126,8 +1128,6 @@ void Analyser::AnalyserImpl::analyseEquationAst(const AnalyserEquationAstPtr &as
 
 double Analyser::AnalyserImpl::scalingFactor(const VariablePtr &variable)
 {
-    // Return the scaling factor for the given variable.
-
     return Units::scalingFactor(variable->units(), internalVariable(variable)->mVariable->units());
 }
 
@@ -1304,7 +1304,7 @@ void Analyser::AnalyserImpl::analyseModel(const ModelPtr &model)
             relevantCheck = false;
 
             for (const auto &internalEquation : mInternalEquations) {
-                relevantCheck = internalEquation->check(equationOrder, stateIndex, variableIndex)
+                relevantCheck = internalEquation->check(equationOrder, stateIndex, variableIndex, mModel)
                                 || relevantCheck;
             }
         } while (relevantCheck);

src/analyserexternalvariable.cpp

@@ -76,7 +76,7 @@ bool AnalyserExternalVariable::addDependency(const VariablePtr &variable)
     if ((pimplVariable != nullptr)
         && (owningModel(variable) == owningModel(pimplVariable))
         && (mPimpl->findDependency(variable) == mPimpl->mDependencies.end())
-        && !isSameOrEquivalentVariable(variable, pimplVariable)) {
+        && !areEquivalentVariables(variable, pimplVariable)) {
         mPimpl->mDependencies.push_back(variable);
 
         return true;

src/analysermodel.cpp

@@ -17,6 +17,7 @@ limitations under the License.
 #include "libcellml/analysermodel.h"
 
 #include "analysermodel_p.h"
+#include "utilities.h"
 
 namespace libcellml {
 
@@ -357,4 +358,39 @@ bool AnalyserModel::needAcothFunction() const
     return mPimpl->mNeedAcothFunction;
 }
 
+bool AnalyserModel::areEquivalentVariables(const VariablePtr &variable1,
+                                           const VariablePtr &variable2)
+{
+    // This is a cached version of the areEquivalentVariables() utility. Indeed,
+    // an AnalyserModel object refers to a static version of a model, which
+    // means that we can safely cache the result of a call to that utility. In
+    // turn, this means that we can speed up any feature (e.g., code generation)
+    // that also relies on that utility. When it comes to the key for the cache,
+    // we use the Cantor pairing function with the address of the two variables
+    // as parameters, thus ensuring the uniqueness of the key (see
+    // https://en.wikipedia.org/wiki/Pairing_function#Cantor_pairing_function).
+
+    auto v1 = reinterpret_cast<uintptr_t>(variable1.get());
+    auto v2 = reinterpret_cast<uintptr_t>(variable2.get());
+
+    if (v2 < v1) {
+        v1 += v2;
+        v2 = v1 - v2;
+        v1 = v1 - v2;
+    }
+
+    auto key = ((v1 + v2) * (v1 + v2 + 1) >> 1U) + v2;
+    auto cacheKey = mPimpl->mCachedEquivalentVariables.find(key);
+
+    if (cacheKey != mPimpl->mCachedEquivalentVariables.end()) {
+        return cacheKey->second;
+    }
+
+    bool res = libcellml::areEquivalentVariables(variable1, variable2);
+
+    mPimpl->mCachedEquivalentVariables[key] = res;
+
+    return res;
+}
+
 } // namespace libcellml

src/analysermodel_p.h

@@ -60,6 +60,8 @@ struct AnalyserModel::AnalyserModelImpl
     bool mNeedAsechFunction = false;
     bool mNeedAcschFunction = false;
     bool mNeedAcothFunction = false;
+
+    std::map<uintptr_t, bool> mCachedEquivalentVariables;
 };
 
 } // namespace libcellml

src/api/libcellml/analysermodel.h

@@ -472,6 +472,33 @@ class LIBCELLML_EXPORT AnalyserModel
      */
     bool needAcothFunction() const;
 
+    /**
+     * @brief Test to determine if @p variable1 and @p variable2 are equivalent.
+     *
+     * Test to see if @p variable1 is the same as or equivalent to @p variable2.
+     * Returns @c true if @p variable1 is equivalent to @p variable2 and
+     * @c false otherwise.
+     *
+     * To test for equivalence is time consuming, so caching is used to speed
+     * things up. During the analysis of a model, various tests are performed
+     * and their result cached. So, if you test two variables that were tested
+     * during the analysis then the cached result will be returned otherwise the
+     * two variables will be properly tested and their result cached. This works
+     * because an @c AnalyserModel always refers to a static version of a
+     * @c Model. However, this might break if a @c Model is modified after it
+     * has been analysed.
+     *
+     * @param variable1 The @c Variable to test if it is equivalent to
+     * @p variable2.
+     * @param variable2 The @c Variable that is potentially equivalent to
+     * @p variable1.
+     *
+     * @return @c true if @p variable1 is equivalent to @p variable2 and
+     * @c false otherwise.
+     */
+    bool areEquivalentVariables(const VariablePtr &variable1,
+                                const VariablePtr &variable2);
+
 private:
     AnalyserModel(); /**< Constructor. */
 

src/bindings/interface/analysermodel.i

@@ -121,6 +121,9 @@
 %feature("docstring") libcellml::AnalyserModel::needAcothFunction
 "Tests if this :class:`AnalyserModel` object needs an \"arc hyperbolic cotangent\" function.";
 
+%feature("docstring") libcellml::AnalyserModel::areEquivalentVariables
+"Tests if the two variables are equivalents.";
+
 %{
 #include "libcellml/analysermodel.h"
 %}

src/generator.cpp

@@ -171,11 +171,11 @@ AnalyserVariablePtr Generator::GeneratorImpl::analyserVariable(const VariablePtr
     auto modelVoi = mLockedModel->voi();
 
     if ((modelVoi != nullptr)
-        && isSameOrEquivalentVariable(variable, modelVoi->variable())) {
+        && mLockedModel->areEquivalentVariables(variable, modelVoi->variable())) {
         res = modelVoi;
     } else {
         for (const auto &modelState : mLockedModel->states()) {
-            if (isSameOrEquivalentVariable(variable, modelState->variable())) {
+            if (mLockedModel->areEquivalentVariables(variable, modelState->variable())) {
                 res = modelState;
 
                 break;
@@ -184,7 +184,7 @@ AnalyserVariablePtr Generator::GeneratorImpl::analyserVariable(const VariablePtr
 
         if (res == nullptr) {
             for (const auto &modelVariable : mLockedModel->variables()) {
-                if (isSameOrEquivalentVariable(variable, modelVariable->variable())) {
+                if (mLockedModel->areEquivalentVariables(variable, modelVariable->variable())) {
                     res = modelVariable;
 
                     break;

src/utilities.cpp

@@ -453,8 +453,8 @@ size_t getVariableIndexInComponent(const ComponentPtr &component, const Variable
     return index;
 }
 
-bool isSameOrEquivalentVariable(const VariablePtr &variable1,
-                                const VariablePtr &variable2)
+bool areEquivalentVariables(const VariablePtr &variable1,
+                            const VariablePtr &variable2)
 {
     return (variable1 == variable2) || variable1->hasEquivalentVariable(variable2, true);
 }

src/utilities.h

@@ -431,23 +431,21 @@ bool isStandardPrefixName(const std::string &name);
 size_t getVariableIndexInComponent(const ComponentPtr &component, const VariablePtr &variable);
 
 /**
- * @brief Test to determine if @p variable1 and @p variable2 are the same or
- * (directly or indirectly) equivalent.
+ * @brief Test to determine if @p variable1 and @p variable2 are equivalent.
  *
- * Test to see if @p variable1 is the same or (directly or indirectly)
- * equivalent to @p variable2. Returns @c true if @p variable1 is the same or
- * (directly or indirectly) equivalent to @p variable2 and @c false otherwise.
+ * Test to see if @p variable1 is the same as or equivalent to @p variable2.
+ * Returns @c true if @p variable1 is the same as or equivalent to @p variable2
+ * and @c false otherwise.
  *
- * @param variable1 The @c Variable to test if it is the same or (directly or
- * indirectly) equivalent to @p variable2.
- * @param variable2 The @c Variable that is potentially the same or (directly or
- * indirectly) equivalent to @p variable1.
+ * @param variable1 The @c Variable to test if it is equivalent to @p variable2.
+ * @param variable2 The @c Variable that is potentially equivalent to
+ * @p variable1.
  *
- * @return @c true if @p variable1 is the same or (directly or indirectly)
- * equivalent to @p variable2 and @c false otherwise.
+ * @return @c true if @p variable1 is equivalent to @p variable2 and @c false
+ * otherwise.
  */
-bool isSameOrEquivalentVariable(const VariablePtr &variable1,
-                                const VariablePtr &variable2);
+bool areEquivalentVariables(const VariablePtr &variable1,
+                            const VariablePtr &variable2);
 
 /**
  * @brief Test to determine if @p entity1 is a child of @p entity2.

tests/bindings/python/test_analyser.py

@@ -144,6 +144,8 @@ def test_coverage(self):
         self.assertFalse(am.needAcschFunction())
         self.assertFalse(am.needAcothFunction())
 
+        self.assertTrue(am.areEquivalentVariables(am.voi().variable(), am.voi().variable()))
+
         # Ensure coverage for AnalyserVariable.
 
         av = am.variable(3)