From 49ff856ef13bebea2c683c7f6715fea258dbf9bb Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 22 Sep 2026 12:55:14 +1200 Subject: [PATCH 1/7] Add test to show error with unit checking. --- tests/resources/units_exponent.cellml | 26 ++++++++++++++++++++++++++ tests/units/units.cpp | 19 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 tests/resources/units_exponent.cellml diff --git a/tests/resources/units_exponent.cellml b/tests/resources/units_exponent.cellml new file mode 100644 index 0000000000..a4f2f8b618 --- /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..614912d3b3 100644 --- a/tests/units/units.cpp +++ b/tests/units/units.cpp @@ -3486,3 +3486,22 @@ 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); + + Debug() << a->issue(0)->description(); + EXPECT_EQ(size_t(0), a->issueCount()); + } From 17f3ac223b4db9b5535ec4beab728823e7c8a85d Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 22 Sep 2026 14:52:48 +1200 Subject: [PATCH 2/7] Improve accuracy of 1/60th approximation. --- tests/resources/units_exponent.cellml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/resources/units_exponent.cellml b/tests/resources/units_exponent.cellml index a4f2f8b618..227f8f4a00 100644 --- a/tests/resources/units_exponent.cellml +++ b/tests/resources/units_exponent.cellml @@ -5,7 +5,7 @@ - + From e84505e7324bea6c4d2d87eeb9ce895ad9e09514 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 22 Sep 2026 14:58:36 +1200 Subject: [PATCH 3/7] Add non-standard units exponent test. --- tests/units/units.cpp | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/tests/units/units.cpp b/tests/units/units.cpp index 614912d3b3..a1c69bb476 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)); @@ -3502,6 +3502,25 @@ TEST(BugFixingUnits, multiplierOfUnitToExponent) auto a = libcellml::Analyser::create(); a->analyseModel(m); - Debug() << a->issue(0)->description(); 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)); + } From 1ca9871ec1ee567fd86dbd57b31f214a51dbaaa2 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 22 Sep 2026 14:59:13 +1200 Subject: [PATCH 4/7] Remove print statement from unsuitablyConstrainedNlaSystem test. --- tests/analyser/analyser.cpp | 1 - 1 file changed, 1 deletion(-) 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), From c3c14be567ea877ad8bf1967c7388bc771a375ab Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 22 Sep 2026 14:59:41 +1200 Subject: [PATCH 5/7] Fix up application of exponent to multiplier to conform to the standard. --- src/analyser.cpp | 2 +- src/units.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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; From bce92a27a23d030dc474eaf383ddb559572b27a6 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 22 Sep 2026 15:01:43 +1200 Subject: [PATCH 6/7] Fix formatting errors. --- tests/units/units.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/units/units.cpp b/tests/units/units.cpp index a1c69bb476..315827e992 100644 --- a/tests/units/units.cpp +++ b/tests/units/units.cpp @@ -3503,10 +3503,10 @@ TEST(BugFixingUnits, multiplierOfUnitToExponent) 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); @@ -3523,4 +3523,4 @@ TEST(BugFixingUnits, nonStandardUnitsExponent) model->addUnits(oneMillionSquareMetres); EXPECT_TRUE(libcellml::Units::equivalent(squareNonStandardKilometre, oneMillionSquareMetres)); - } +} From d4aee74fd1ae594e30b118196dfa726314767796 Mon Sep 17 00:00:00 2001 From: Hugh Sorby Date: Tue, 22 Sep 2026 15:26:18 +1200 Subject: [PATCH 7/7] Adjust complexUnits test. --- tests/analyser/analyserunits.cpp | 8 ++++---- tests/resources/analyser/units/complex_units.cellml | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) 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