Merge pull request #1241 from agarny/issue1240

Code generation: unit scaling for rate computation is the other way round
cellml · Jul 3, 2024 · cf540fa · cf540fa
2 parents 66d2f00 + 48c7137
commit cf540fa
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 28 deletions.
diff --git a/.codespellexclude b/.codespellexclude
@@ -13,3 +13,5 @@
     if (index < pFunc()->mErrors.size()) {
         issue = pFunc()->mIssues.at(pFunc()->mErrors.at(index));
     return pFunc()->mErrors.size();
+identity and expression, level of experience, education, socio-economic status,
+        "W3C MathML DTD error: Element apply content does not follow the DTD, expecting (csymbol | ci | cn | apply | reln | lambda | condition | declare | sep | semantics | annotation | annotation-xml | integers | reals | rationals | naturalnumbers | complexes | primes | exponentiale | imaginaryi | notanumber | true | false | emptyset | pi | eulergamma | infinity | interval | list | matrix | matrixrow | set | vector | piecewise | lowlimit | uplimit | bvar | degree | logbase | momentabout | domainofapplication | inverse | ident | domain | codomain | image | abs | conjugate | exp | factorial | arg | real | imaginary | floor | ceiling | not | ln | sin | cos | tan | sec | csc | cot | sinh | cosh | tanh | sech | csch | coth | arcsin | arccos | arctan | arccosh | arccot | arccoth | arccsc | arccsch | arcsec | arcsech | arcsinh | arctanh | determinant | transpose | card | quotient | divide | power | rem | implies | vectorproduct | scalarproduct | outerproduct | setdiff | fn | compose | plus | times | max | min | gcd | lcm | and | or | xor | union | intersect | cartesianproduct | mean | sdev | variance | median | mode | selector | root | minus | log | int | diff | partialdiff | divergence | grad | curl | laplacian | sum | product | limit | moment | exists | forall | neq | factorof | in | notin | notsubset | notprsubset | tendsto | eq | leq | lt | geq | gt | equivalent | approx | subset | prsubset | mi | mn | mo | mtext | ms | mspace | mrow | mfrac | msqrt | mroot | menclose | mstyle | merror | mpadded | mphantom | mfenced | msub | msup | msubsup | munder | mover | munderover | mmultiscripts | mtable | mtr | mlabeledtr | mtd | maligngroup | malignmark | maction)*, got (CDATA bvar ).",
diff --git a/src/analyser.cpp b/src/analyser.cpp
@@ -2453,7 +2453,7 @@ void Analyser::AnalyserImpl::scaleEquationAst(const AnalyserEquationAstPtr &ast)
                 auto astGrandparent = astParent->parent();
 
                 if (astGrandparent->mPimpl->mType == AnalyserEquationAst::Type::EQUALITY) {
-                    scaleAst(astGrandparent->mPimpl->mOwnedRightChild, astGrandparent, 1.0 / scalingFactor);
+                    scaleAst(astGrandparent->mPimpl->mOwnedRightChild, astGrandparent, scalingFactor);
                 } else {
                     scaleAst(astParent, astGrandparent, 1.0 / scalingFactor);
                 }

diff --git a/tests/resources/generator/cellml_unit_scaling_voi_indirect/model.c b/tests/resources/generator/cellml_unit_scaling_voi_indirect/model.c
@@ -8,14 +8,15 @@
 const char VERSION[] = "0.5.0";
 const char LIBCELLML_VERSION[] = "0.5.0";
 
-const size_t STATE_COUNT = 2;
+const size_t STATE_COUNT = 3;
 const size_t VARIABLE_COUNT = 0;
 
-const VariableInfo VOI_INFO = {"t", "ms", "environment", VARIABLE_OF_INTEGRATION};
+const VariableInfo VOI_INFO = {"t", "second", "environment", VARIABLE_OF_INTEGRATION};
 
 const VariableInfo STATE_INFO[] = {
-    {"x", "mM", "t_in_ms", STATE},
-    {"x", "mM", "t_in_s", STATE}
+    {"x", "metre", "t_in_s", STATE},
+    {"x", "metre", "t_in_ms", STATE},
+    {"x", "metre", "t_in_ks", STATE}
 };
 
 const VariableInfo VARIABLE_INFO[] = {
@@ -52,6 +53,7 @@ void initialiseVariables(double *states, double *rates, double *variables)
 {
     states[0] = 3.0;
     states[1] = 7.0;
+    states[2] = 11.0;
 }
 
 void computeComputedConstants(double *variables)
@@ -62,6 +64,7 @@ void computeRates(double voi, double *states, double *rates, double *variables)
 {
     rates[0] = 5.0;
     rates[1] = 1000.0*9.0;
+    rates[2] = 0.001*13.0;
 }
 
 void computeVariables(double voi, double *states, double *rates, double *variables)

diff --git a/tests/resources/generator/cellml_unit_scaling_voi_indirect/model.cellml b/tests/resources/generator/cellml_unit_scaling_voi_indirect/model.cellml
@@ -1,29 +1,50 @@
 <?xml version='1.0' encoding='UTF-8'?>
 <model name="my_model" xmlns="http://www.cellml.org/cellml/2.0#" xmlns:cellml="http://www.cellml.org/cellml/2.0#">
-    <!-- Define the VOI as being time (expressed in milliseconds). Then:
- - Use the VOI as-is (i.e. time expressed in milliseconds) in a component to compute an ODE; and
- - Use a scaled version of the VOI (here, time expressed in seconds) in a component to compute another ODE. -->
+    <!-- Define the VOI as being time (expressed in seconds). Then:
+ - Use the VOI as-is (i.e. time expressed in seconds) in a component to compute an ODE;
+ - Use a scaled version of the VOI (here, time expressed in milliseconds) in a component to compute another ODE
+ - Use a scaled version of the VOI (here, time expressed in kiloseconds) in a component to compute another ODE. -->
     <units name="ms">
         <unit prefix="milli" units="second"/>
     </units>
-    <units name="mM">
-        <unit prefix="milli" units="mole"/>
-        <unit exponent="-1" units="litre"/>
+    <units name="ks">
+        <unit prefix="kilo" units="second"/>
     </units>
-    <units name="mM_per_s">
-        <unit units="mM"/>
+    <units name="m_per_s">
+        <unit units="metre"/>
         <unit exponent="-1" units="second"/>
     </units>
-    <units name="mM_per_ms">
-        <unit units="mM"/>
+    <units name="m_per_ms">
+        <unit units="metre"/>
         <unit exponent="-1" units="ms"/>
     </units>
+    <units name="m_per_ks">
+        <unit units="metre"/>
+        <unit exponent="-1" units="ks"/>
+    </units>
     <component name="environment">
-        <variable interface="public" name="t" units="ms"/>
+        <variable interface="public" name="t" units="second"/>
+    </component>
+    <component name="t_in_s">
+        <variable interface="public" name="t" units="second"/>
+        <variable initial_value="3" name="x" units="metre"/>
+        <math xmlns="http://www.w3.org/1998/Math/MathML">
+            <apply>
+                <eq/>
+                <apply>
+                    <diff/>
+                    <bvar>
+                        <ci>t</ci>
+                    </bvar>
+                    <ci>x</ci>
+                </apply>
+                <cn cellml:units="m_per_s">5</cn>
+            </apply>
+        </math>
     </component>
     <component name="t_in_ms">
         <variable interface="public" name="t" units="ms"/>
-        <variable initial_value="3" name="x" units="mM"/>
+        <variable initial_value="7" name="x" units="metre"/>
         <math xmlns="http://www.w3.org/1998/Math/MathML">
             <apply>
                 <eq/>
@@ -34,13 +55,13 @@
                     </bvar>
                     <ci>x</ci>
                 </apply>
-                <cn cellml:units="mM_per_ms">5</cn>
+                <cn cellml:units="m_per_ms">9</cn>
             </apply>
         </math>
     </component>
-    <component name="t_in_s">
-        <variable interface="public" name="t" units="second"/>
-        <variable initial_value="7" name="x" units="mM"/>
+    <component name="t_in_ks">
+        <variable interface="public" name="t" units="ks"/>
+        <variable initial_value="11" name="x" units="metre"/>
         <math xmlns="http://www.w3.org/1998/Math/MathML">
             <apply>
                 <eq/>
@@ -51,14 +72,17 @@
                     </bvar>
                     <ci>x</ci>
                 </apply>
-                <cn cellml:units="mM_per_s">9</cn>
+                <cn cellml:units="m_per_ks">13</cn>
             </apply>
         </math>
     </component>
+    <connection component_1="environment" component_2="t_in_s">
+        <map_variables variable_1="t" variable_2="t"/>
+    </connection>
     <connection component_1="environment" component_2="t_in_ms">
         <map_variables variable_1="t" variable_2="t"/>
     </connection>
-    <connection component_1="environment" component_2="t_in_s">
+    <connection component_1="environment" component_2="t_in_ks">
         <map_variables variable_1="t" variable_2="t"/>
     </connection>
 </model>
diff --git a/tests/resources/generator/cellml_unit_scaling_voi_indirect/model.h b/tests/resources/generator/cellml_unit_scaling_voi_indirect/model.h
@@ -20,7 +20,7 @@ typedef enum {
 
 typedef struct {
     char name[2];
-    char units[3];
+    char units[7];
     char component[12];
     VariableType type;
 } VariableInfo;

diff --git a/tests/resources/generator/cellml_unit_scaling_voi_indirect/model.py b/tests/resources/generator/cellml_unit_scaling_voi_indirect/model.py
@@ -7,7 +7,7 @@
 __version__ = "0.4.0"
 LIBCELLML_VERSION = "0.5.0"
 
-STATE_COUNT = 2
+STATE_COUNT = 3
 VARIABLE_COUNT = 0
 
 
@@ -19,11 +19,12 @@ class VariableType(Enum):
     ALGEBRAIC = 4
 
 
-VOI_INFO = {"name": "t", "units": "ms", "component": "environment", "type": VariableType.VARIABLE_OF_INTEGRATION}
+VOI_INFO = {"name": "t", "units": "second", "component": "environment", "type": VariableType.VARIABLE_OF_INTEGRATION}
 
 STATE_INFO = [
-    {"name": "x", "units": "mM", "component": "t_in_ms", "type": VariableType.STATE},
-    {"name": "x", "units": "mM", "component": "t_in_s", "type": VariableType.STATE}
+    {"name": "x", "units": "metre", "component": "t_in_s", "type": VariableType.STATE},
+    {"name": "x", "units": "metre", "component": "t_in_ms", "type": VariableType.STATE},
+    {"name": "x", "units": "metre", "component": "t_in_ks", "type": VariableType.STATE}
 ]
 
 VARIABLE_INFO = [
@@ -41,6 +42,7 @@ def create_variables_array():
 def initialise_variables(states, rates, variables):
     states[0] = 3.0
     states[1] = 7.0
+    states[2] = 11.0
 
 
 def compute_computed_constants(variables):
@@ -50,6 +52,7 @@ def compute_computed_constants(variables):
 def compute_rates(voi, states, rates, variables):
     rates[0] = 5.0
     rates[1] = 1000.0*9.0
+    rates[2] = 0.001*13.0
 
 
 def compute_variables(voi, states, rates, variables):