diff --git a/src/analyser.cpp b/src/analyser.cpp index e79550d274..00c89e83a8 100644 --- a/src/analyser.cpp +++ b/src/analyser.cpp @@ -25,12 +25,13 @@ limitations under the License. #include "libcellml/analyserequation.h" #include "libcellml/analyserexternalvariable.h" -#include "libcellml/generatorprofile.h" +#include "libcellml/analysermodel.h" #include "libcellml/validator.h" #include "analyser_p.h" #include "analyserequation_p.h" #include "analyserequationast_p.h" +#include "analysermodel_p.h" #include "analyservariable_p.h" #include "commonutils.h" #include "generator_p.h" diff --git a/src/analyser_p.h b/src/analyser_p.h index f888861de4..43ea5d9a1d 100644 --- a/src/analyser_p.h +++ b/src/analyser_p.h @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +#include "libcellml/analyser.h" #include "libcellml/generatorprofile.h" #include "libcellml/issue.h" diff --git a/src/analysermodel.cpp b/src/analysermodel.cpp index fe6c16ca7e..3cddd3ea39 100644 --- a/src/analysermodel.cpp +++ b/src/analysermodel.cpp @@ -16,16 +16,21 @@ limitations under the License. #include "libcellml/analysermodel.h" -#include "libcellml/analyservariable.h" - #include "analysermodel_p.h" +#include "analyservariable_p.h" #include "utilities.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) @@ -43,6 +48,38 @@ AnalyserModel::~AnalyserModel() delete mPimpl; } +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); + } + + uniteEquivalentVariableAddresses(v1, v2); + } + } + + for (size_t i = 0; i < component->componentCount(); ++i) { + buildEquivalentVariablesCache(component->component(i)); + } +} + +void AnalyserModel::AnalyserModelImpl::buildEquivalentVariablesCache() +{ + mEquivalentVariableCache.clear(); + + for (size_t i = 0; i < mModel->componentCount(); ++i) { + buildEquivalentVariablesCache(mModel->component(i)); + } +} + bool AnalyserModel::isValid() const { switch (mPimpl->mType) { @@ -498,25 +535,18 @@ bool AnalyserModel::areEquivalentVariables(const VariablePtr &variable1, // turn, this means that we can speed up any feature (e.g., code generation) // that also relies on that utility. - auto v1 = reinterpret_cast(variable1.get()); - auto v2 = reinterpret_cast(variable2.get()); - - if (v1 > v2) { - std::swap(v1, v2); + if ((variable1 == nullptr) || (variable2 == nullptr)) { + return false; } - auto key = AnalyserModel::AnalyserModelImpl::VariableKeyPair {v1, v2}; - auto it = mPimpl->mCachedEquivalentVariables.find(key); - - if (it != mPimpl->mCachedEquivalentVariables.end()) { - return it->second; + if (variable1 == variable2) { + return true; } - auto res = libcellml::areEquivalentVariables(variable1, variable2); - - mPimpl->mCachedEquivalentVariables.emplace(key, res); + const auto v1 = reinterpret_cast(variable1.get()); + const auto v2 = reinterpret_cast(variable2.get()); - return res; + return mPimpl->findVariableAddress(v1) == mPimpl->findVariableAddress(v2); } } // namespace libcellml diff --git a/src/analysermodel_p.h b/src/analysermodel_p.h index d473ffab28..adee6853e2 100644 --- a/src/analysermodel_p.h +++ b/src/analysermodel_p.h @@ -46,32 +46,34 @@ struct AnalyserModel::AnalyserModelImpl std::vector mAnalyserEquations; - struct VariableKeyPair + std::unordered_map mEquivalentVariableCache; + + uintptr_t findVariableAddress(uintptr_t x) { - uintptr_t first; - uintptr_t second; + auto it = mEquivalentVariableCache.find(x); + + if (it == mEquivalentVariableCache.end()) { + mEquivalentVariableCache[x] = x; - bool operator==(const VariableKeyPair &other) const - { - return (first == other.first) & (second == other.second); + return x; } - }; - struct VariableKeyPairHash - { - size_t operator()(const VariableKeyPair &pair) const - { - // A simple and portable hash function for a pair of pointers. + if (it->second != x) { + it->second = findVariableAddress(it->second); + } - size_t hash = pair.first; + return it->second; + } - hash ^= pair.second + 0x9e3779b9 + (hash << 6) + (hash >> 2); + void uniteEquivalentVariableAddresses(uintptr_t x, uintptr_t y) + { + const uintptr_t &rootX = findVariableAddress(x); + const uintptr_t &rootY = findVariableAddress(y); - return hash; + if (rootX != rootY) { + mEquivalentVariableCache[rootY] = rootX; } - }; - - std::unordered_map mCachedEquivalentVariables; + } bool mNeedEqFunction = false; bool mNeedNeqFunction = false; @@ -102,6 +104,9 @@ struct AnalyserModel::AnalyserModelImpl static AnalyserModelPtr create(const ModelPtr &model = nullptr); + void buildEquivalentVariablesCache(const ComponentPtr &component); + void buildEquivalentVariablesCache(); + AnalyserModelImpl(const ModelPtr &model); }; diff --git a/src/api/libcellml/analysermodel.h b/src/api/libcellml/analysermodel.h index 12806e6764..688a143e03 100644 --- a/src/api/libcellml/analysermodel.h +++ b/src/api/libcellml/analysermodel.h @@ -600,14 +600,13 @@ class LIBCELLML_EXPORT AnalyserModel * 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 @ref AnalyserModel always refers to a static version of a - * @ref Model. However, this might break if a @ref Model is modified after it - * has been analysed. + * 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. + * + * @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. diff --git a/tests/coverage/coverage.cpp b/tests/coverage/coverage.cpp index 15b8699f01..18f567abe8 100644 --- a/tests/coverage/coverage.cpp +++ b/tests/coverage/coverage.cpp @@ -589,6 +589,21 @@ TEST(Coverage, analyserTypes) EXPECT_EQ("algebraic_variable", libcellml::AnalyserVariable::typeAsString(analyserModel->algebraicVariable(0)->type())); } +TEST(Coverage, analyserAreEquivalentVariables) +{ + auto analyser = libcellml::Analyser::create(); + auto parser = libcellml::Parser::create(); + auto model = parser->parseModel(fileContents("generator/hodgkin_huxley_squid_axon_model_1952/model.cellml")); + + analyser->analyseModel(model); + + auto analyserModel = analyser->analyserModel(); + EXPECT_FALSE(analyserModel->areEquivalentVariables(nullptr, nullptr)); + auto variable = model->component("membrane")->variable("V"); + EXPECT_FALSE(analyserModel->areEquivalentVariables(nullptr, variable)); + EXPECT_FALSE(analyserModel->areEquivalentVariables(variable, nullptr)); +} + void checkAstTypeAsString(const libcellml::AnalyserEquationAstPtr &ast) { if (ast != nullptr) {