Fixed an issue with using the wrong name for a (connected) variable.

src/api/libcellml/generator.h

@@ -78,8 +78,7 @@ class LIBCELLML_EXPORT GeneratorVariable
      * @c Component in which the @c Variable is first defined (in the case of
      * the variable of integration), initialised (in the case of a constant) or
      * actually computed (in the case of a state, computed constant or algebraic
-     * variable). It may or may not be the same @c Component as the parent
-     * component of the @c Variable.
+     * variable).
      *
      * @return The @c Component.
      */

src/generator.cpp

@@ -120,14 +120,17 @@ struct GeneratorInternalVariable
     size_t mIndex = MAX_SIZE_T;
     Type mType = Type::UNKNOWN;
 
+    VariablePtr mInitialValueVariable;
+
     VariablePtr mVariable;
     ComponentPtr mComponent;
 
     GeneratorEquationWeakPtr mEquation;
 
     explicit GeneratorInternalVariable(const VariablePtr &variable);
 
-    void setVariable(const VariablePtr &variable);
+    void setVariable(const VariablePtr &variable,
+                     bool checkInitialValue = true);
 
     void makeVoi();
     void makeState();
@@ -136,22 +139,25 @@ struct GeneratorInternalVariable
 using GeneratorInternalVariablePtr = std::shared_ptr<GeneratorInternalVariable>;
 
 GeneratorInternalVariable::GeneratorInternalVariable(const VariablePtr &variable)
-    : mComponent(std::dynamic_pointer_cast<Component>(variable->parent()))
 {
     setVariable(variable);
 }
 
-void GeneratorInternalVariable::setVariable(const VariablePtr &variable)
+void GeneratorInternalVariable::setVariable(const VariablePtr &variable,
+                                            bool checkInitialValue)
 {
-    mVariable = variable;
-
-    if (!variable->initialValue().empty()) {
+    if (checkInitialValue && !variable->initialValue().empty()) {
         // The variable has an initial value, so it can either be a constant or
         // a state. By default, we consider it to be a constant and, if we find
         // an ODE for that variable, we will know that it was actually a state.
 
         mType = Type::CONSTANT;
+
+        mInitialValueVariable = variable;
     }
+
+    mVariable = variable;
+    mComponent = std::dynamic_pointer_cast<Component>(variable->parent());
 }
 
 void GeneratorInternalVariable::makeVoi()
@@ -427,6 +433,15 @@ bool GeneratorEquation::knownOdeVariable(const GeneratorInternalVariablePtr &ode
            || (odeVariable->mType == GeneratorInternalVariable::Type::VARIABLE_OF_INTEGRATION);
 }
 
+bool sameOrEquivalentVariable(const VariablePtr &variable1,
+                              const VariablePtr &variable2)
+{
+    // Return whether the given variables are the same or are equivalent (be it
+    // directly or indirectly).
+
+    return (variable1 == variable2) || variable1->hasEquivalentVariable(variable2, true);
+}
+
 bool GeneratorEquation::check(size_t &equationOrder, size_t &stateIndex,
                               size_t &variableIndex)
 {
@@ -451,7 +466,7 @@ bool GeneratorEquation::check(size_t &equationOrder, size_t &stateIndex,
     }
 
     // Determine, from the (new) known (ODE) variables, whether the equation is
-    // truly constant or variable-based constant.
+    // truly a constant or a variable-based constant.
 
     mComputedTrueConstant = mComputedTrueConstant
                             && !containsNonUnknownVariables(mVariables)
@@ -490,18 +505,26 @@ bool GeneratorEquation::check(size_t &equationOrder, size_t &stateIndex,
     mVariables.remove_if(knownVariable);
     mOdeVariables.remove_if(knownOdeVariable);
 
-    // If there is one (ODE) variable left then update its component (to be sure
-    // that it's the same as the one in which the equation is), its type (if it
-    // is currently unknown), determine its index and determine the type of our
-    // equation and set its order, if the (ODE) variable is a state, computed
-    // constant or algebraic variable.
+    // If there is one (ODE) variable left then update its viariable (to be the
+    // corresponding one in the component in which the equation is), its type
+    // (if it is currently unknown), determine its index and determine the type
+    // of our equation and set its order, if the (ODE) variable is a state,
+    // computed constant or algebraic variable.
 
     bool relevantCheck = false;
 
     if (mVariables.size() + mOdeVariables.size() == 1) {
         GeneratorInternalVariablePtr variable = (mVariables.size() == 1) ? mVariables.front() : mOdeVariables.front();
 
-        variable->mComponent = mComponent;
+        for (size_t i = 0; i < mComponent->variableCount(); ++i) {
+            VariablePtr localVariable = mComponent->variable(i);
+
+            if (sameOrEquivalentVariable(variable->mVariable, localVariable)) {
+                variable->setVariable(localVariable, false);
+
+                break;
+            }
+        }
 
         if (variable->mType == GeneratorInternalVariable::Type::UNKNOWN) {
             variable->mType = mComputedTrueConstant ?
@@ -601,9 +624,6 @@ struct Generator::GeneratorImpl
     static bool compareEquationsByVariable(const GeneratorEquationPtr &equation1,
                                            const GeneratorEquationPtr &equation2);
 
-    bool sameOrEquivalentVariable(const VariablePtr &variable1,
-                                  const VariablePtr &variable2);
-
     GeneratorVariablePtr variableFirstOccurrence(const VariablePtr &variable,
                                                  const ComponentPtr &component);
 
@@ -773,15 +793,6 @@ GeneratorInternalVariablePtr Generator::GeneratorImpl::generatorVariable(const V
     return internalVariable;
 }
 
-bool Generator::GeneratorImpl::sameOrEquivalentVariable(const VariablePtr &variable1,
-                                                        const VariablePtr &variable2)
-{
-    // Return whether the given variables are the same or are equivalent (be it
-    // directly or indirectly).
-
-    return (variable1 == variable2) || variable1->hasEquivalentVariable(variable2, true);
-}
-
 GeneratorVariablePtr Generator::GeneratorImpl::variableFirstOccurrence(const VariablePtr &variable,
                                                                        const ComponentPtr &component)
 {
@@ -1213,7 +1224,7 @@ void Generator::GeneratorImpl::processComponent(const ComponentPtr &component)
                    && !variable->initialValue().empty()
                    && !generatorVariable->mVariable->initialValue().empty()) {
             ModelPtr model = owningModel(component);
-            ComponentPtr trackedVariableComponent = std::dynamic_pointer_cast<Component>(generatorVariable->mVariable->parent());
+            ComponentPtr trackedVariableComponent = generatorVariable->mComponent;
             ModelPtr trackedVariableModel = owningModel(trackedVariableComponent);
             IssuePtr issue = Issue::create();
 
@@ -1356,21 +1367,11 @@ void Generator::GeneratorImpl::processEquationAst(const GeneratorEquationAstPtr
 bool Generator::GeneratorImpl::compareVariablesByName(const GeneratorInternalVariablePtr &variable1,
                                                       const GeneratorInternalVariablePtr &variable2)
 {
-    // TODO: we can't currently instatiate imports, which means that we can't
-    //       have variables in different models. This also means that we can't
-    //       have code to check for the name of a model since this would fail
-    //       coverage test. So, once we can instantiate imports, we will need to
-    //       account for the name of a model.
-    VariablePtr realVariable1 = variable1->mVariable;
-    VariablePtr realVariable2 = variable2->mVariable;
-    ComponentPtr realComponent1 = std::dynamic_pointer_cast<Component>(realVariable1->parent());
-    ComponentPtr realComponent2 = std::dynamic_pointer_cast<Component>(realVariable2->parent());
-
-    if (realComponent1->name() == realComponent2->name()) {
-        return realVariable1->name() < realVariable2->name();
+    if (variable1->mComponent->name() == variable2->mComponent->name()) {
+        return variable1->mVariable->name() < variable2->mVariable->name();
     }
 
-    return realComponent1->name() < realComponent2->name();
+    return variable1->mComponent->name() < variable2->mComponent->name();
 }
 
 bool Generator::GeneratorImpl::compareVariablesByTypeAndIndex(const GeneratorInternalVariablePtr &variable1,
@@ -1502,11 +1503,10 @@ void Generator::GeneratorImpl::processModel(const ModelPtr &model)
 
             if (!issueType.empty()) {
                 IssuePtr issue = Issue::create();
-                VariablePtr realVariable = internalVariable->mVariable;
-                ComponentPtr realComponent = std::dynamic_pointer_cast<Component>(realVariable->parent());
+                ComponentPtr realComponent = internalVariable->mComponent;
                 ModelPtr realModel = owningModel(realComponent);
 
-                issue->setDescription("Variable '" + realVariable->name()
+                issue->setDescription("Variable '" + internalVariable->mVariable->name()
                                       + "' in component '" + realComponent->name()
                                       + "' of model '" + realModel->name() + "' " + issueType + ".");
                 issue->setCause(Issue::Cause::GENERATOR);
@@ -3300,7 +3300,7 @@ std::string Generator::GeneratorImpl::generateCode(const GeneratorEquationAstPtr
 
 std::string Generator::GeneratorImpl::generateInitializationCode(const GeneratorInternalVariablePtr &variable)
 {
-    return mProfile->indentString() + generateVariableNameCode(variable->mVariable) + " = " + generateDoubleCode(variable->mVariable->initialValue()) + mProfile->commandSeparatorString() + "\n";
+    return mProfile->indentString() + generateVariableNameCode(variable->mVariable) + " = " + generateDoubleCode(variable->mInitialValueVariable->initialValue()) + mProfile->commandSeparatorString() + "\n";
 }
 
 std::string Generator::GeneratorImpl::generateEquationCode(const GeneratorEquationPtr &equation,

tests/generator/generator.cpp

@@ -1136,6 +1136,48 @@ TEST(Generator, nobleModel1962)
     EXPECT_EQ(fileContents("generator/noble_model_1962/model.py"), generator->implementationCode());
 }
 
+TEST(Generator, sineImports)
+{
+    libcellml::ParserPtr parser = libcellml::Parser::create();
+    libcellml::ModelPtr model = parser->parseModel(fileContents("sine_approximations_import.xml"));
+
+    EXPECT_EQ(size_t(0), parser->issueCount());
+    EXPECT_TRUE(model->hasUnresolvedImports());
+
+    model->resolveImports(resourcePath());
+
+    EXPECT_FALSE(model->hasUnresolvedImports());
+
+    model->flatten();
+
+    libcellml::GeneratorPtr generator = libcellml::Generator::create();
+
+    generator->processModel(model);
+
+    EXPECT_EQ(size_t(0), generator->issueCount());
+
+    EXPECT_EQ(libcellml::Generator::ModelType::ODE, generator->modelType());
+
+    EXPECT_EQ(size_t(1), generator->stateCount());
+    EXPECT_EQ(size_t(10), generator->variableCount());
+
+    EXPECT_NE(nullptr, generator->voi());
+    EXPECT_NE(nullptr, generator->state(0));
+    EXPECT_EQ(nullptr, generator->state(generator->stateCount()));
+    EXPECT_NE(nullptr, generator->variable(0));
+    EXPECT_EQ(nullptr, generator->variable(generator->variableCount()));
+
+    EXPECT_EQ(fileContents("generator/sine_model_imports/model.h"), generator->interfaceCode());
+    EXPECT_EQ(fileContents("generator/sine_model_imports/model.c"), generator->implementationCode());
+
+    libcellml::GeneratorProfilePtr profile = libcellml::GeneratorProfile::create(libcellml::GeneratorProfile::Profile::PYTHON);
+
+    generator->setProfile(profile);
+
+    EXPECT_EQ(EMPTY_STRING, generator->interfaceCode());
+    EXPECT_EQ(fileContents("generator/sine_model_imports/model.py"), generator->implementationCode());
+}
+
 TEST(Generator, coverage)
 {
     libcellml::ParserPtr parser = libcellml::Parser::create();

tests/printer/printer.cpp

@@ -193,6 +193,7 @@ TEST(Printer, printModelWithImports)
         "    <variable name=\"sin2\" units=\"dimensionless\" interface=\"public_and_private\" id=\"deriv_approx\"/>\n"
         "    <variable name=\"deriv_approx_initial_value\" units=\"dimensionless\" initial_value=\"0\" interface=\"public_and_private\" id=\"deriv_approx_initial_value\"/>\n"
         "    <variable name=\"sin3\" units=\"dimensionless\" interface=\"public_and_private\" id=\"parabolic_approx\"/>\n"
+        "    <variable name=\"C\" units=\"dimensionless\" initial_value=\"0.75\" interface=\"public_and_private\"/>\n"
         "  </component>\n"
         "  <connection component_1=\"main\" component_2=\"actual_sin\">\n"
         "    <map_variables variable_1=\"x\" variable_2=\"x\"/>\n"
@@ -206,6 +207,7 @@ TEST(Printer, printModelWithImports)
         "  <connection component_1=\"main\" component_2=\"parabolic_approx_sin\">\n"
         "    <map_variables variable_1=\"x\" variable_2=\"x\"/>\n"
         "    <map_variables variable_1=\"sin3\" variable_2=\"sin\"/>\n"
+        "    <map_variables variable_1=\"C\" variable_2=\"C\"/>\n"
         "  </connection>\n"
         "  <encapsulation>\n"
         "    <component_ref component=\"main\">\n"

tests/resources/deriv_approx_sin.xml

@@ -3,7 +3,7 @@
 
   <component name="sin" id="sin">
     <variable id="x" units="dimensionless" name="x" interface="public_and_private"/>
-    <variable id="sin" units="dimensionless" name="sin" initial_value="sin_initial_value" interface="public_and_private"/>
+    <variable id="sin" units="dimensionless" name="sin" initial_value="0.0" interface="public_and_private"/>
     <variable units="dimensionless" name="sin_initial_value" interface="public_and_private"/>
     <math xmlns="http://www.w3.org/1998/Math/MathML">
       <apply><eq/>

tests/resources/generator/cellml_mappings_and_encapsulations/model.cellml

@@ -10,7 +10,7 @@
  - Check a variable source can have no maths in its own component
    (see circle_x_source)
  - Check we can export a variable both sideways and upwards
-   (see circle_x_sibling)    
+   (see circle_x_sibling)
  - Check connection handling using a purely pass-through component
    (see circle_y)
  - Check connecting a private=in, public=out (y) variable

tests/resources/generator/fabbri_fantini_wilders_severi_human_san_model_2017/model.c

@@ -90,7 +90,7 @@ const VariableInfoWithType VARIABLE_INFO[] = {
     {"Ko", "millimolar", "Ionic_values", CONSTANT},
     {"Nao", "millimolar", "Ionic_values", CONSTANT},
     {"C", "microF", "Membrane", CONSTANT},
-    {"F", "coulomb_per_mole", "Nai_concentration", CONSTANT},
+    {"F", "coulomb_per_mole", "Membrane", CONSTANT},
     {"R", "joule_per_kilomole_kelvin", "Membrane", CONSTANT},
     {"T", "kelvin", "Membrane", CONSTANT},
     {"clamp_mode", "dimensionless", "Membrane", CONSTANT},

tests/resources/generator/fabbri_fantini_wilders_severi_human_san_model_2017/model.py

@@ -96,7 +96,7 @@ class VariableType(Enum):
     {"name": "Ko", "units": "millimolar", "component": "Ionic_values", "type": VariableType.CONSTANT},
     {"name": "Nao", "units": "millimolar", "component": "Ionic_values", "type": VariableType.CONSTANT},
     {"name": "C", "units": "microF", "component": "Membrane", "type": VariableType.CONSTANT},
-    {"name": "F", "units": "coulomb_per_mole", "component": "Nai_concentration", "type": VariableType.CONSTANT},
+    {"name": "F", "units": "coulomb_per_mole", "component": "Membrane", "type": VariableType.CONSTANT},
     {"name": "R", "units": "joule_per_kilomole_kelvin", "component": "Membrane", "type": VariableType.CONSTANT},
     {"name": "T", "units": "kelvin", "component": "Membrane", "type": VariableType.CONSTANT},
     {"name": "clamp_mode", "units": "dimensionless", "component": "Membrane", "type": VariableType.CONSTANT},

tests/resources/generator/garny_kohl_hunter_boyett_noble_rabbit_san_model_2003/model.c

@@ -76,12 +76,12 @@ const VariableInfoWithType VARIABLE_INFO[] = {
     {"g_f_Na_Periphery_0DCapable", "microS", "hyperpolarisation_activated_current", CONSTANT},
     {"g_f_Na_Periphery_1DCapable", "microS", "hyperpolarisation_activated_current", CONSTANT},
     {"g_f_Na_Periphery_Published", "microS", "hyperpolarisation_activated_current", CONSTANT},
-    {"Ca_i", "millimolar", "sodium_calcium_exchanger", CONSTANT},
-    {"Ca_o", "millimolar", "sodium_calcium_exchanger", CONSTANT},
+    {"Ca_i", "millimolar", "ionic_concentrations", CONSTANT},
+    {"Ca_o", "millimolar", "ionic_concentrations", CONSTANT},
     {"K_i", "millimolar", "ionic_concentrations", CONSTANT},
-    {"K_o", "millimolar", "sodium_potassium_pump", CONSTANT},
-    {"Na_i", "millimolar", "sodium_calcium_exchanger", CONSTANT},
-    {"Na_o", "millimolar", "sodium_calcium_exchanger", CONSTANT},
+    {"K_o", "millimolar", "ionic_concentrations", CONSTANT},
+    {"Na_i", "millimolar", "ionic_concentrations", CONSTANT},
+    {"Na_o", "millimolar", "ionic_concentrations", CONSTANT},
     {"CmCentre", "microF", "membrane", CONSTANT},
     {"CmPeriphery", "microF", "membrane", CONSTANT},
     {"F", "coulomb_per_mole", "membrane", CONSTANT},

tests/resources/generator/garny_kohl_hunter_boyett_noble_rabbit_san_model_2003/model.py

@@ -82,12 +82,12 @@ class VariableType(Enum):
     {"name": "g_f_Na_Periphery_0DCapable", "units": "microS", "component": "hyperpolarisation_activated_current", "type": VariableType.CONSTANT},
     {"name": "g_f_Na_Periphery_1DCapable", "units": "microS", "component": "hyperpolarisation_activated_current", "type": VariableType.CONSTANT},
     {"name": "g_f_Na_Periphery_Published", "units": "microS", "component": "hyperpolarisation_activated_current", "type": VariableType.CONSTANT},
-    {"name": "Ca_i", "units": "millimolar", "component": "sodium_calcium_exchanger", "type": VariableType.CONSTANT},
-    {"name": "Ca_o", "units": "millimolar", "component": "sodium_calcium_exchanger", "type": VariableType.CONSTANT},
+    {"name": "Ca_i", "units": "millimolar", "component": "ionic_concentrations", "type": VariableType.CONSTANT},
+    {"name": "Ca_o", "units": "millimolar", "component": "ionic_concentrations", "type": VariableType.CONSTANT},
     {"name": "K_i", "units": "millimolar", "component": "ionic_concentrations", "type": VariableType.CONSTANT},
-    {"name": "K_o", "units": "millimolar", "component": "sodium_potassium_pump", "type": VariableType.CONSTANT},
-    {"name": "Na_i", "units": "millimolar", "component": "sodium_calcium_exchanger", "type": VariableType.CONSTANT},
-    {"name": "Na_o", "units": "millimolar", "component": "sodium_calcium_exchanger", "type": VariableType.CONSTANT},
+    {"name": "K_o", "units": "millimolar", "component": "ionic_concentrations", "type": VariableType.CONSTANT},
+    {"name": "Na_i", "units": "millimolar", "component": "ionic_concentrations", "type": VariableType.CONSTANT},
+    {"name": "Na_o", "units": "millimolar", "component": "ionic_concentrations", "type": VariableType.CONSTANT},
     {"name": "CmCentre", "units": "microF", "component": "membrane", "type": VariableType.CONSTANT},
     {"name": "CmPeriphery", "units": "microF", "component": "membrane", "type": VariableType.CONSTANT},
     {"name": "F", "units": "coulomb_per_mole", "component": "membrane", "type": VariableType.CONSTANT},