Generator: don't untrack a variable that is needed to compute an exte…

…rnal variable.
cellml · Oct 31, 2024 · 33bab8f · 33bab8f
1 parent d4fdf65
commit 33bab8f
Show file tree

Hide file tree

Showing 29 changed files with 2,528 additions and 1,347 deletions.
diff --git a/src/api/libcellml/issue.h b/src/api/libcellml/issue.h
@@ -222,6 +222,8 @@ class LIBCELLML_EXPORT Issue
         GENERATOR_EXTERNAL_VARIABLE_NOT_UNTRACKABLE,
         GENERATOR_NLA_BASED_VARIABLE_ALWAYS_TRACKED,
         GENERATOR_NLA_BASED_VARIABLE_NOT_UNTRACKABLE,
+        GENERATOR_EXTERNALLY_NEEDED_VARIABLE_ALWAYS_TRACKED,
+        GENERATOR_EXTERNALLY_NEEDED_VARIABLE_NOT_UNTRACKABLE,
 
         // Placeholder for further references:
         UNSPECIFIED

diff --git a/src/generator.cpp b/src/generator.cpp
@@ -109,6 +109,27 @@ bool Generator::GeneratorImpl::isUntrackedVariable(const AnalyserVariablePtr &va
     return doIsTrackedVariable(variable, false);
 }
 
+void Generator::GeneratorImpl::addNeededToComputeExternalVariableIssue(const AnalyserVariablePtr &variable, bool tracked)
+{
+    auto issue = Issue::IssueImpl::create();
+
+    issue->mPimpl->setDescription("Variable '" + variable->variable()->name()
+                                  + "' in component '" + owningComponent(variable->variable())->name()
+                                  + "' is needed to compute an external variable and "
+                                  + (tracked ?
+                                         "is therefore always tracked." :
+                                         "cannot therefore be untracked."));
+    issue->mPimpl->setReferenceRule(tracked ?
+                                        Issue::ReferenceRule::GENERATOR_EXTERNALLY_NEEDED_VARIABLE_ALWAYS_TRACKED :
+                                        Issue::ReferenceRule::GENERATOR_EXTERNALLY_NEEDED_VARIABLE_NOT_UNTRACKABLE);
+
+    if (tracked) {
+        issue->mPimpl->setLevel(Issue::Level::MESSAGE);
+    }
+
+    addIssue(issue);
+}
+
 bool Generator::GeneratorImpl::trackableVariable(const AnalyserVariablePtr &variable, bool tracked, bool canAddIssue)
 {
     // A trackable variable is a variable that is not a variable of integration, a state, or an external variable, which
@@ -141,6 +162,50 @@ bool Generator::GeneratorImpl::trackableVariable(const AnalyserVariablePtr &vari
         }
     }
 
+    // A trackable variable is also not a variable that is needed to compute an external variable.
+
+    for (const auto &external : variable->model()->externals()) {
+        auto externalEquationPimpl = external->equations().front()->mPimpl;
+
+        // Check whether the variable is a constant dependency.
+
+        for (const auto &constantDependency : externalEquationPimpl->mConstantDependencies) {
+            if (variable == constantDependency) {
+                if (canAddIssue) {
+                    addNeededToComputeExternalVariableIssue(variable, tracked);
+                }
+
+                return false;
+            }
+        }
+
+        // Check whether the variable is a computed constant dependency or an algebraic dependency.
+
+        for (const auto &dependency : externalEquationPimpl->mDependencies) {
+            auto dependencyPimpl = dependency.lock()->mPimpl;
+
+            for (const auto &computedConstant : dependencyPimpl->mComputedConstants) {
+                if (variable == computedConstant) {
+                    if (canAddIssue) {
+                        addNeededToComputeExternalVariableIssue(variable, tracked);
+                    }
+
+                    return false;
+                }
+            }
+
+            for (const auto &algebraic : dependencyPimpl->mAlgebraic) {
+                if (variable == algebraic) {
+                    if (canAddIssue) {
+                        addNeededToComputeExternalVariableIssue(variable, tracked);
+                    }
+
+                    return false;
+                }
+            }
+        }
+    }
+
     return true;
 }
 

diff --git a/src/generator_p.h b/src/generator_p.h
@@ -54,6 +54,8 @@ struct Generator::GeneratorImpl: public Logger::LoggerImpl
     bool isTrackedVariable(const AnalyserVariablePtr &variable);
     bool isUntrackedVariable(const AnalyserVariablePtr &variable);
 
+    void addNeededToComputeExternalVariableIssue(const AnalyserVariablePtr &variable, bool tracked);
+
     bool trackableVariable(const AnalyserVariablePtr &variable, bool tracked, bool canAddIssue = true);
     bool specialVariable(const AnalyserVariablePtr &variable, const AnalyserVariablePtr &specialVariable, bool tracked,
                          Issue::ReferenceRule trackedReferenceRule, Issue::ReferenceRule untrackedReferenceRule);

diff --git a/tests/generator/generatortrackedvariables.cpp b/tests/generator/generatortrackedvariables.cpp
@@ -452,6 +452,7 @@ void untrack(const libcellml::AnalyserModelPtr &model, const libcellml::Generato
 {
     switch (trackingType) {
     case TrackingType::VARIABLES:
+        generator->trackAllVariables(model); // For coverage.
         generator->untrackAllVariables(model);
 
         break;
@@ -508,17 +509,18 @@ void hodgkinHuxleySquidAxonModel1952CodeGeneration(bool ode, TrackingType tracki
 
     EXPECT_EQ_FILE_CONTENTS("generator/hodgkin_huxley_squid_axon_model_1952/" + modelType + ".untracked." + variableType + ".py", generator->implementationCode(analyserModel));
 
-    // With some external variables.
+    // With an external variable with a dependency on a constant, computed constant, and algebraic variable.
 
-    auto potassium_channel_n_gate_alpha_n = model->component("potassium_channel_n_gate")->variable("alpha_n");
+    auto membrane_Cm = model->component("membrane")->variable("Cm");
+    auto potassium_channel_EK = model->component("potassium_channel")->variable("E_K");
+    auto sodium_channel_m_gate_alapha_m = model->component("sodium_channel_m_gate")->variable("alpha_m");
     auto external_sodium_channel_i_Na = libcellml::AnalyserExternalVariable::create(model->component("sodium_channel")->variable("i_Na"));
 
-    external_sodium_channel_i_Na->addDependency(potassium_channel_n_gate_alpha_n);
-    external_sodium_channel_i_Na->addDependency(model->component("sodium_channel_h_gate")->variable("h"));
+    external_sodium_channel_i_Na->addDependency(membrane_Cm);
+    external_sodium_channel_i_Na->addDependency(potassium_channel_EK);
+    external_sodium_channel_i_Na->addDependency(sodium_channel_m_gate_alapha_m);
 
-    analyser->addExternalVariable(libcellml::AnalyserExternalVariable::create(model->component("membrane")->variable("V")));
     analyser->addExternalVariable(external_sodium_channel_i_Na);
-    analyser->addExternalVariable(libcellml::AnalyserExternalVariable::create(potassium_channel_n_gate_alpha_n));
 
     analyser->analyseModel(model);
 
@@ -544,65 +546,105 @@ void hodgkinHuxleySquidAxonModel1952CodeGeneration(bool ode, TrackingType tracki
     EXPECT_EQ_FILE_CONTENTS("generator/hodgkin_huxley_squid_axon_model_1952/" + modelType + ".untracked." + variableType + ".with.externals.py", generator->implementationCode(analyserModel));
 }
 
+const std::vector<std::string> noIssues;
+const std::vector<std::string> constantRelatedExternalIssues = {
+    "Variable 'Cm' in component 'membrane' is needed to compute an external variable and cannot therefore be untracked.",
+};
+
 TEST(GeneratorTrackedVariables, hodgkinHuxleySquidAxonModel1952UntrackedConstants)
 {
-    hodgkinHuxleySquidAxonModel1952CodeGeneration(true, TrackingType::CONSTANTS);
+    hodgkinHuxleySquidAxonModel1952CodeGeneration(true, TrackingType::CONSTANTS, noIssues, constantRelatedExternalIssues);
 }
 
+const std::vector<std::string> computedConstantRelatedExternalIssues = {
+    "Variable 'E_K' in component 'potassium_channel' is needed to compute an external variable and cannot therefore be untracked.",
+};
+
 TEST(GeneratorTrackedVariables, hodgkinHuxleySquidAxonModel1952UntrackedComputedConstants)
 {
-    hodgkinHuxleySquidAxonModel1952CodeGeneration(true, TrackingType::COMPUTED_CONSTANTS);
+    hodgkinHuxleySquidAxonModel1952CodeGeneration(true, TrackingType::COMPUTED_CONSTANTS, noIssues, computedConstantRelatedExternalIssues);
 }
 
+const std::vector<std::string> algebraicRelatedExternalIssues = {
+    "Variable 'alpha_m' in component 'sodium_channel_m_gate' is needed to compute an external variable and cannot therefore be untracked.",
+};
+
 TEST(GeneratorTrackedVariables, hodgkinHuxleySquidAxonModel1952UntrackedAlgebraicVariables)
 {
-    hodgkinHuxleySquidAxonModel1952CodeGeneration(true, TrackingType::ALGEBRAIC);
+    hodgkinHuxleySquidAxonModel1952CodeGeneration(true, TrackingType::ALGEBRAIC, noIssues, algebraicRelatedExternalIssues);
 }
 
 TEST(GeneratorTrackedVariables, hodgkinHuxleySquidAxonModel1952UntrackedVariables)
 {
-    hodgkinHuxleySquidAxonModel1952CodeGeneration(true, TrackingType::VARIABLES);
+    const std::vector<std::string> variableRelatedExternalIssues = {
+        "Variable 'Cm' in component 'membrane' is needed to compute an external variable and cannot therefore be untracked.",
+        "Variable 'E_K' in component 'potassium_channel' is needed to compute an external variable and cannot therefore be untracked.",
+        "Variable 'alpha_m' in component 'sodium_channel_m_gate' is needed to compute an external variable and cannot therefore be untracked.",
+    };
+
+    hodgkinHuxleySquidAxonModel1952CodeGeneration(true, TrackingType::VARIABLES, noIssues, variableRelatedExternalIssues);
 }
 
 TEST(GeneratorTrackedVariables, hodgkinHuxleySquidAxonModel1952DaeUntrackedConstants)
 {
-    hodgkinHuxleySquidAxonModel1952CodeGeneration(false, TrackingType::CONSTANTS);
+    hodgkinHuxleySquidAxonModel1952CodeGeneration(false, TrackingType::CONSTANTS, noIssues, constantRelatedExternalIssues);
 }
 
 TEST(GeneratorTrackedVariables, hodgkinHuxleySquidAxonModel1952DaeUntrackedComputedConstants)
 {
-    hodgkinHuxleySquidAxonModel1952CodeGeneration(false, TrackingType::COMPUTED_CONSTANTS);
+    hodgkinHuxleySquidAxonModel1952CodeGeneration(false, TrackingType::COMPUTED_CONSTANTS, noIssues, computedConstantRelatedExternalIssues);
 }
 
-const std::vector<std::string> issues = {
+const std::vector<std::string> daeIssues = {
     "Variable 'i_Stim' in component 'membrane' is computed using an NLA system and cannot therefore be untracked.",
     "Variable 'i_L' in component 'leakage_current' is computed using an NLA system and cannot therefore be untracked.",
     "Variable 'i_K' in component 'potassium_channel' is computed using an NLA system and cannot therefore be untracked.",
     "Variable 'i_Na' in component 'sodium_channel' is computed using an NLA system and cannot therefore be untracked.",
-    "Variable 'alpha_m' in component 'sodium_channel_m_gate' is computed using an NLA system and cannot therefore be untracked.",
+    "Variable 'E_L' in component 'leakage_current' is computed using an NLA system and cannot therefore be untracked.",
+    "Variable 'E_Na' in component 'sodium_channel' is computed using an NLA system and cannot therefore be untracked.",
     "Variable 'beta_m' in component 'sodium_channel_m_gate' is computed using an NLA system and cannot therefore be untracked.",
     "Variable 'alpha_h' in component 'sodium_channel_h_gate' is computed using an NLA system and cannot therefore be untracked.",
     "Variable 'beta_h' in component 'sodium_channel_h_gate' is computed using an NLA system and cannot therefore be untracked.",
     "Variable 'alpha_n' in component 'potassium_channel_n_gate' is computed using an NLA system and cannot therefore be untracked.",
     "Variable 'beta_n' in component 'potassium_channel_n_gate' is computed using an NLA system and cannot therefore be untracked.",
 };
-const std::vector<std::string> externalIssues = {
-    "Variable 'i_Stim' in component 'membrane' is computed using an NLA system and cannot therefore be untracked.",
-    "Variable 'i_L' in component 'leakage_current' is computed using an NLA system and cannot therefore be untracked.",
-    "Variable 'i_K' in component 'potassium_channel' is computed using an NLA system and cannot therefore be untracked.",
-    "Variable 'alpha_m' in component 'sodium_channel_m_gate' is computed using an NLA system and cannot therefore be untracked.",
-    "Variable 'beta_m' in component 'sodium_channel_m_gate' is computed using an NLA system and cannot therefore be untracked.",
-    "Variable 'alpha_h' in component 'sodium_channel_h_gate' is computed using an NLA system and cannot therefore be untracked.",
-    "Variable 'beta_h' in component 'sodium_channel_h_gate' is computed using an NLA system and cannot therefore be untracked.",
-    "Variable 'beta_n' in component 'potassium_channel_n_gate' is computed using an NLA system and cannot therefore be untracked.",
-};
 
 TEST(GeneratorTrackedVariables, hodgkinHuxleySquidAxonModel1952DaeUntrackedAlgebraicVariables)
 {
-    hodgkinHuxleySquidAxonModel1952CodeGeneration(false, TrackingType::ALGEBRAIC, issues, externalIssues);
+    const std::vector<std::string> daeExternalIssues = {
+        "Variable 'i_Stim' in component 'membrane' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'i_L' in component 'leakage_current' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'i_K' in component 'potassium_channel' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'E_L' in component 'leakage_current' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'E_Na' in component 'sodium_channel' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'alpha_m' in component 'sodium_channel_m_gate' is needed to compute an external variable and cannot therefore be untracked.",
+        "Variable 'beta_m' in component 'sodium_channel_m_gate' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'alpha_h' in component 'sodium_channel_h_gate' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'beta_h' in component 'sodium_channel_h_gate' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'alpha_n' in component 'potassium_channel_n_gate' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'beta_n' in component 'potassium_channel_n_gate' is computed using an NLA system and cannot therefore be untracked.",
+    };
+
+    hodgkinHuxleySquidAxonModel1952CodeGeneration(false, TrackingType::ALGEBRAIC, daeIssues, daeExternalIssues);
 }
 
 TEST(GeneratorTrackedVariables, hodgkinHuxleySquidAxonModel1952DaeUntrackedVariables)
 {
-    hodgkinHuxleySquidAxonModel1952CodeGeneration(false, TrackingType::VARIABLES, issues, externalIssues);
+    const std::vector<std::string> daeExternalIssues = {
+        "Variable 'Cm' in component 'membrane' is needed to compute an external variable and cannot therefore be untracked.",
+        "Variable 'E_K' in component 'potassium_channel' is needed to compute an external variable and cannot therefore be untracked.",
+        "Variable 'i_Stim' in component 'membrane' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'i_L' in component 'leakage_current' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'i_K' in component 'potassium_channel' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'E_L' in component 'leakage_current' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'E_Na' in component 'sodium_channel' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'alpha_m' in component 'sodium_channel_m_gate' is needed to compute an external variable and cannot therefore be untracked.",
+        "Variable 'beta_m' in component 'sodium_channel_m_gate' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'alpha_h' in component 'sodium_channel_h_gate' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'beta_h' in component 'sodium_channel_h_gate' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'alpha_n' in component 'potassium_channel_n_gate' is computed using an NLA system and cannot therefore be untracked.",
+        "Variable 'beta_n' in component 'potassium_channel_n_gate' is computed using an NLA system and cannot therefore be untracked.",
+    };
+
+    hodgkinHuxleySquidAxonModel1952CodeGeneration(false, TrackingType::VARIABLES, daeIssues, daeExternalIssues);
 }
diff --git a/tests/resources/generator/hodgkin_huxley_squid_axon_model_1952/model.dae.wo.cc.cellml b/tests/resources/generator/hodgkin_huxley_squid_axon_model_1952/model.dae.wo.cc.cellml
@@ -119,12 +119,16 @@
         <math xmlns="http://www.w3.org/1998/Math/MathML">
             <apply>
                 <eq/>
-                <ci>E_Na</ci>
                 <apply>
                     <minus/>
-                    <ci>E_R</ci>
-                    <cn cellml:units="millivolt">115</cn>
+                    <ci>E_Na</ci>
+                    <apply>
+                        <minus/>
+                        <ci>E_R</ci>
+                        <cn cellml:units="millivolt">115</cn>
+                    </apply>
                 </apply>
+                <cn cellml:units="millivolt">0</cn>
             </apply>
             <apply>
                 <eq/>
@@ -160,39 +164,35 @@
         <math xmlns="http://www.w3.org/1998/Math/MathML">
             <apply>
                 <eq/>
+                <ci>alpha_m</ci>
                 <apply>
-                    <minus/>
-                    <ci>alpha_m</ci>
+                    <divide/>
                     <apply>
-                        <divide/>
+                        <times/>
+                        <cn cellml:units="per_millivolt_millisecond">0.1</cn>
                         <apply>
-                            <times/>
-                            <cn cellml:units="per_millivolt_millisecond">0.1</cn>
-                            <apply>
-                                <plus/>
-                                <ci>V</ci>
-                                <cn cellml:units="millivolt">25</cn>
-                            </apply>
+                            <plus/>
+                            <ci>V</ci>
+                            <cn cellml:units="millivolt">25</cn>
                         </apply>
+                    </apply>
+                    <apply>
+                        <minus/>
                         <apply>
-                            <minus/>
+                            <exp/>
                             <apply>
-                                <exp/>
+                                <divide/>
                                 <apply>
-                                    <divide/>
-                                    <apply>
-                                        <plus/>
-                                        <ci>V</ci>
-                                        <cn cellml:units="millivolt">25</cn>
-                                    </apply>
-                                    <cn cellml:units="millivolt">10</cn>
+                                    <plus/>
+                                    <ci>V</ci>
+                                    <cn cellml:units="millivolt">25</cn>
                                 </apply>
+                                <cn cellml:units="millivolt">10</cn>
                             </apply>
-                            <cn cellml:units="dimensionless">1</cn>
                         </apply>
+                        <cn cellml:units="dimensionless">1</cn>
                     </apply>
                 </apply>
-                <cn cellml:units="per_millisecond">0</cn>
             </apply>
             <apply>
                 <eq/>
@@ -483,12 +483,16 @@
         <math xmlns="http://www.w3.org/1998/Math/MathML">
             <apply>
                 <eq/>
-                <ci>E_L</ci>
                 <apply>
                     <minus/>
-                    <ci>E_R</ci>
-                    <cn cellml:units="millivolt">10.613</cn>
+                    <ci>E_L</ci>
+                    <apply>
+                        <minus/>
+                        <ci>E_R</ci>
+                        <cn cellml:units="millivolt">10.613</cn>
+                    </apply>
                 </apply>
+                <cn cellml:units="millivolt">0</cn>
             </apply>
             <apply>
                 <eq/>