diff --git a/src/analyser.cpp b/src/analyser.cpp index 401936d6f0..78dfdedf8f 100644 --- a/src/analyser.cpp +++ b/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, diff --git a/src/units.cpp b/src/units.cpp index 2cddc08111..25f5a6b935 100644 --- a/src/units.cpp +++ b/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; diff --git a/tests/analyser/analyser.cpp b/tests/analyser/analyser.cpp index ecc9db714b..4388206c18 100644 --- a/tests/analyser/analyser.cpp +++ b/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), diff --git a/tests/analyser/analyserunits.cpp b/tests/analyser/analyserunits.cpp index 0c8cc48b9d..66a3f53541 100644 --- a/tests/analyser/analyserunits.cpp +++ b/tests/analyser/analyserunits.cpp @@ -764,10 +764,10 @@ TEST(AnalyserUnits, complexUnits) EXPECT_EQ(size_t(0), parser->issueCount()); const std::vector 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(); diff --git a/tests/resources/analyser/units/complex_units.cellml b/tests/resources/analyser/units/complex_units.cellml index f4bd83d15c..87cf7cca52 100644 --- a/tests/resources/analyser/units/complex_units.cellml +++ b/tests/resources/analyser/units/complex_units.cellml @@ -6,6 +6,9 @@ + + + @@ -76,7 +79,7 @@ 1 - 1 + 1 diff --git a/tests/resources/units_exponent.cellml b/tests/resources/units_exponent.cellml new file mode 100644 index 0000000000..227f8f4a00 --- /dev/null +++ b/tests/resources/units_exponent.cellml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + tx + kx + + + + + diff --git a/tests/units/units.cpp b/tests/units/units.cpp index ff3434f5cd..315827e992 100644 --- a/tests/units/units.cpp +++ b/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)); +}