Fix error in Analyser units checking

src/analyser.cpp

@@ -1398,7 +1398,7 @@ void Analyser::AnalyserImpl::updateUnitsMultiplier(const ModelPtr &model,
                 units->unitAttributes(i, reference, prefix, exponent, multiplier, id);
 
                 if (isStandardUnitName(reference)) {
-                    newUnitsMultiplier += unitsMultiplier + (standardMultiplierList.at(reference) + std::log10(multiplier) + convertPrefixToInt(prefix)) * exponent * unitsExponent;
+                    newUnitsMultiplier += unitsMultiplier + (std::log10(multiplier) + (standardMultiplierList.at(reference) + convertPrefixToInt(prefix)) * exponent) * unitsExponent;
                 } else {
                     updateUnitsMultiplier(model, reference, newUnitsMultiplier,
                                           exponent * unitsExponent,

src/units.cpp

@@ -266,7 +266,7 @@ bool updateUnitMultiplier(const UnitsPtr &units, int direction, double &multipli
                     return false;
                 }
                 // Make the direction positive on all branches, direction is only applied at the end.
-                localMultiplier += mult + branchMult * exp + prefixMult;
+                localMultiplier += mult + (branchMult + prefixMult) * exp;
             }
         }
         multiplier += localMultiplier * direction;

tests/analyser/analyser.cpp

@@ -1049,7 +1049,6 @@ TEST(Analyser, unsuitablyConstrainedNlaSystem)
     auto analyser = libcellml::Analyser::create();
 
     analyser->analyseModel(model);
-    printIssues(analyser);
 
     EXPECT_EQ_ISSUES_CELLMLELEMENTTYPES_LEVELS_REFERENCERULES_URLS(expectedIssues,
                                                                    expectedCellmlElementTypes(expectedIssues.size(), libcellml::CellmlElementType::VARIABLE),

tests/analyser/analyserunits.cpp

@@ -764,10 +764,10 @@ TEST(AnalyserUnits, complexUnits)
     EXPECT_EQ(size_t(0), parser->issueCount());
 
     const std::vector<std::string> expectedIssues = {
-        "The units in 'a = 1.0' in component 'main' are not equivalent. 'a' is in 'apple' (i.e. '10^-27 x second^3') while '1.0' is in 'orange' (i.e. '10^-9 x second^7').",
-        "The units in 'b = 1.0/1.0' in component 'main' are not equivalent. 'b' is in 'apple' (i.e. '10^-27 x second^3') while '1.0/1.0' is in 'orange x second_2^-1' (i.e. '10^-9 x second^5').",
-        "The units in 'c = 1.0/(1.0/1.0)' in component 'main' are not equivalent. 'c' is in 'apple' (i.e. '10^-27 x second^3') while '1.0/(1.0/1.0)' is in 'atto_second x orange x second_2^-1' (i.e. '10^-27 x second^6').",
-        "The units in 'd = 1.0/(1.0/1.0)/1.0' in component 'main' are not equivalent. 'd' is in 'apple' (i.e. '10^-27 x second^3') while '1.0/(1.0/1.0)/1.0' is in 'atto_second x orange x second_2^-2' (i.e. '10^-27 x second^4').",
+        "The units in 'a = 1.0' in component 'main' are not equivalent. 'a' is in 'apple' (i.e. '10^-21 x second^3') while '1.0' is in 'orange' (i.e. '10^-9 x second^7').",
+        "The units in 'b = 1.0/1.0' in component 'main' are not equivalent. 'b' is in 'apple' (i.e. '10^-21 x second^3') while '1.0/1.0' is in 'orange x second_2^-1' (i.e. '10^-9 x second^5').",
+        "The units in 'c = 1.0/(1.0/1.0)' in component 'main' are not equivalent. 'c' is in 'apple' (i.e. '10^-21 x second^3') while '1.0/(1.0/1.0)' is in 'atto_second x orange x second_2^-1' (i.e. '10^-27 x second^6').",
+        "The units in 'd = 1.0/(1.0/1.0)/1.0' in component 'main' are not equivalent. 'd' is in 'apple' (i.e. '10^-21 x second^3') while '1.0/(1.0/1.0)/1.0' is in 'atto_second x orange x second_2^-2' (i.e. '10^-27 x second^4').",
     };
 
     auto analyser = libcellml::Analyser::create();

tests/resources/analyser/units/complex_units.cellml

@@ -6,6 +6,9 @@
     <units name="atto_second">
         <unit prefix="atto" units="second"/>
     </units>
+    <units name="pico_second">
+        <unit prefix="pico" units="second"/>
+    </units>
     <units name="apple">
         <unit exponent="3" multiplier="0.001" prefix="micro" units="second"/>
     </units>
@@ -76,7 +79,7 @@
                         <apply>
                             <divide/>
                             <cn cellml:units="second_2">1</cn>
-                            <cn cellml:units="atto_second">1</cn>
+                            <cn cellml:units="pico_second">1</cn>
                         </apply>
                     </apply>
                     <apply>

tests/resources/units_exponent.cellml

@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<model xmlns="http://www.cellml.org/cellml/2.0#" name="per_minute">
+  <units name="minute">
+    <unit units="second" multiplier="60"/>
+  </units>
+  <!-- 1/60 second^-1, the multiplier is outside of the exponent -->
+  <units name="per_minute">
+    <unit units="second" exponent="-1" multiplier="0.01666666666666667"/>
+  </units>
+  <!-- the same units, by reference -->
+  <units name="per_minute_by_reference">
+    <unit units="minute" exponent="-1"/>
+  </units>
+  <component name="main">
+    <variable name="t" units="minute"/>
+    <variable name="x" units="mole" initial_value="1"/>
+    <variable name="k" units="per_minute" initial_value="0.1"/>
+    <math xmlns="http://www.w3.org/1998/Math/MathML">
+      <apply><eq/>
+        <apply><diff/><bvar><ci>t</ci></bvar><ci>x</ci></apply>
+        <apply><times/><ci>k</ci><ci>x</ci></apply>
+      </apply>
+    </math>
+  </component>
+</model>
+

tests/units/units.cpp

@@ -1208,16 +1208,16 @@ TEST(Units, dimensionlessScalingFactor)
     m->addUnits(u5);
     m->addUnits(u6);
 
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u1, u2));
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u2, u1));
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u3, u4));
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u4, u3));
+    EXPECT_EQ(1.0e+06, libcellml::Units::scalingFactor(u1, u2));
+    EXPECT_EQ(1.0e-06, libcellml::Units::scalingFactor(u2, u1));
+    EXPECT_EQ(1.0e+06, libcellml::Units::scalingFactor(u3, u4));
+    EXPECT_EQ(1.0e-06, libcellml::Units::scalingFactor(u4, u3));
     EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u1, u3));
     EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u2, u4));
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u1, u4));
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u2, u3));
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u5, u6));
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u6, u5));
+    EXPECT_EQ(1.0e+06, libcellml::Units::scalingFactor(u1, u4));
+    EXPECT_EQ(1.0e-06, libcellml::Units::scalingFactor(u2, u3));
+    EXPECT_EQ(1.0e+06, libcellml::Units::scalingFactor(u5, u6));
+    EXPECT_EQ(1.0e-06, libcellml::Units::scalingFactor(u6, u5));
     EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u1, u5));
 }
 
@@ -1310,8 +1310,8 @@ TEST(Units, complicatedMultiplicationFactorUnits)
     model->addUnits(incredible_pile_of_square_apples);
     model->addUnits(bunch_of_bananas);
 
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u1, u2));
-    EXPECT_EQ(1.0, libcellml::Units::scalingFactor(u3, u4));
+    EXPECT_EQ(1.0e+06, libcellml::Units::scalingFactor(u1, u2));
+    EXPECT_EQ(1.0e-03, libcellml::Units::scalingFactor(u3, u4));
     EXPECT_EQ(1e-08, libcellml::Units::scalingFactor(incredible_pile_of_square_apples, square_apple));
     // Incompatible units so we return a scaling factor of 0.0.
     EXPECT_EQ(0.0, libcellml::Units::scalingFactor(incredible_pile_of_square_apples, bunch_of_bananas));
@@ -3486,3 +3486,41 @@ TEST(Units, oneLitreEqualToOneThousandCentiMetreCubedAllCubed)
 
     EXPECT_TRUE(libcellml::Units::equivalent(oneLitreCubed, oneThousandCentiMetreCubedCubed));
 }
+
+TEST(BugFixingUnits, multiplierOfUnitToExponent)
+{
+    libcellml::ParserPtr p = libcellml::Parser::create();
+    auto m = p->parseModel(fileContents("units_exponent.cellml"));
+
+    EXPECT_EQ(size_t(0), p->issueCount());
+
+    auto v = libcellml::Validator::create();
+    v->validateModel(m);
+
+    EXPECT_EQ(size_t(0), v->issueCount());
+
+    auto a = libcellml::Analyser::create();
+    a->analyseModel(m);
+
+    EXPECT_EQ(size_t(0), a->issueCount());
+}
+
+TEST(BugFixingUnits, nonStandardUnitsExponent)
+{
+    auto model = libcellml::Model::create();
+    auto nonStandardKilometre = libcellml::Units::create("b");
+    nonStandardKilometre->addUnit("metre", libcellml::Units::Prefix::KILO);
+
+    auto squareNonStandardKilometre = libcellml::Units::create("squareNonStandardKiloetre");
+    squareNonStandardKilometre->addUnit("b", 2);
+
+    auto oneMillionSquareMetres = libcellml::Units::create("oneMillionSquareMetres");
+    oneMillionSquareMetres->addUnit("metre", 2.0);
+    oneMillionSquareMetres->addUnit("dimensionless", libcellml::Units::Prefix::MEGA);
+
+    model->addUnits(nonStandardKilometre);
+    model->addUnits(squareNonStandardKilometre);
+    model->addUnits(oneMillionSquareMetres);
+
+    EXPECT_TRUE(libcellml::Units::equivalent(squareNonStandardKilometre, oneMillionSquareMetres));
+}