Skip to content

Commit

Permalink
Analyser: make sure that we are dealing with a power function when un…
Browse files Browse the repository at this point in the history 
…its maps are not dimensionless.
  • Loading branch information
@agarny
agarny committed Nov 6, 2024
1 parent fa756b4 commit 99a4beb
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 19 deletions.
58 changes: 39 additions & 19 deletions src/analyser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ void Analyser::AnalyserImpl::analyseNode(const XmlNodePtr &node,
// Basic content elements.

if (node->isMathmlElement("apply")) {
// We may have 2, 3 or more child nodes, e.g.
// We may have 1, 2, 3 or more child nodes, e.g.
//
// +--------+
// | + |
Expand Down Expand Up @@ -744,29 +744,32 @@ void Analyser::AnalyserImpl::analyseNode(const XmlNodePtr &node,
auto childCount = mathmlChildCount(node);

analyseNode(mathmlChildNode(node, 0), ast, astParent, component, equation);
analyseNode(mathmlChildNode(node, 1), ast->mPimpl->mOwnedLeftChild, ast, component, equation);

if (childCount >= 3) {
AnalyserEquationAstPtr astRightChild;
AnalyserEquationAstPtr tempAst;
if (childCount >= 2) {
analyseNode(mathmlChildNode(node, 1), ast->mPimpl->mOwnedLeftChild, ast, component, equation);

analyseNode(mathmlChildNode(node, childCount - 1), astRightChild, nullptr, component, equation);
if (childCount >= 3) {
AnalyserEquationAstPtr astRightChild;
AnalyserEquationAstPtr tempAst;

for (auto i = childCount - 2; i > 1; --i) {
tempAst = AnalyserEquationAst::create();
analyseNode(mathmlChildNode(node, childCount - 1), astRightChild, nullptr, component, equation);

analyseNode(mathmlChildNode(node, 0), tempAst, nullptr, component, equation);
analyseNode(mathmlChildNode(node, i), tempAst->mPimpl->mOwnedLeftChild, tempAst, component, equation);
for (auto i = childCount - 2; i > 1; --i) {
tempAst = AnalyserEquationAst::create();

astRightChild->mPimpl->mParent = tempAst;
analyseNode(mathmlChildNode(node, 0), tempAst, nullptr, component, equation);
analyseNode(mathmlChildNode(node, i), tempAst->mPimpl->mOwnedLeftChild, tempAst, component, equation);

tempAst->mPimpl->mOwnedRightChild = astRightChild;
astRightChild = tempAst;
}
astRightChild->mPimpl->mParent = tempAst;

tempAst->mPimpl->mOwnedRightChild = astRightChild;
astRightChild = tempAst;
}

astRightChild->mPimpl->mParent = ast;
astRightChild->mPimpl->mParent = ast;

ast->mPimpl->mOwnedRightChild = astRightChild;
ast->mPimpl->mOwnedRightChild = astRightChild;
}
}

// Relational and logical operators.
Expand Down Expand Up @@ -2182,9 +2185,26 @@ void Analyser::AnalyserImpl::analyseEquationUnits(const AnalyserEquationAstPtr &
+ expressionUnits(ast->mPimpl->mOwnedRightChild, rightUnitsMaps, rightUserUnitsMaps, rightUnitsMultipliers) + ".";
}
} else if (!isDimensionlessUnitsMaps(unitsMaps)) {
issueDescription = "The units in " + expression(ast) + " may not be equivalent. "
+ expressionUnits(ast->mPimpl->mOwnedLeftChild, unitsMaps, userUnitsMaps, unitsMultipliers) + " while "
+ expression(ast->mPimpl->mOwnedRightChild->mPimpl->mOwnedRightChild, false) + " may result in " + expression(ast->mPimpl->mOwnedRightChild, false) + " having different units.";
auto leftChild = ast->mPimpl->mOwnedLeftChild;
auto rightChild = ast->mPimpl->mOwnedRightChild;

if (leftChild->type() == AnalyserEquationAst::Type::POWER) {
if (rightChild != nullptr) {
if (rightChild->type() == AnalyserEquationAst::Type::POWER) {
issueDescription = "The units in " + expression(ast) + " may not be equivalent. "
+ expression(leftChild->mPimpl->mOwnedRightChild, false) + " may result in " + expression(leftChild, false) + " having different units while "
+ expression(rightChild->mPimpl->mOwnedRightChild, false) + " may result in " + expression(rightChild, false) + " having different units.";
} else {
issueDescription = "The units in " + expression(ast) + " may not be equivalent. "
+ expression(leftChild->mPimpl->mOwnedRightChild, false) + " may result in " + expression(leftChild, false) + " having different units while "
+ expressionUnits(rightChild, unitsMaps, userUnitsMaps, unitsMultipliers) + ".";
}
}
} else if (rightChild->type() == AnalyserEquationAst::Type::POWER) {
issueDescription = "The units in " + expression(ast) + " may not be equivalent. "
+ expressionUnits(leftChild, unitsMaps, userUnitsMaps, unitsMultipliers) + " while "
+ expression(rightChild->mPimpl->mOwnedRightChild, false) + " may result in " + expression(rightChild, false) + " having different units.";
}
}
} break;
case AnalyserEquationAst::Type::PIECEWISE:
Expand Down
6 changes: 6 additions & 0 deletions tests/analyser/analyserunits.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,8 @@ TEST(AnalyserUnits, powerValues)
"The units in 'eqnMax2 = pow(x, max(5.0, 3.0))' in component 'my_component' are not equivalent. 'eqnMax2' is in 'second' while 'pow(x, max(5.0, 3.0))' is in 'second^5'.",
"The units in 'eqnRem = pow(x, rem(3.0, 5.0))' in component 'my_component' are not equivalent. 'eqnRem' is in 'second' while 'pow(x, rem(3.0, 5.0))' is in 'second^3'.",
"The units in 'eqnDiff = pow(x, dx/dt)' in component 'my_component' may not be equivalent. 'eqnDiff' is in 'second' while 'dx/dt' may result in 'pow(x, dx/dt)' having different units.",
"The units in 'pow(x, dx/dt) = eqnDiff2' in component 'my_component' may not be equivalent. 'dx/dt' may result in 'pow(x, dx/dt)' having different units while 'eqnDiff2' is in 'second'.",
"The units in 'pow(x, dx/dt) = pow(xx, dxx/dt)' in component 'my_component' may not be equivalent. 'dx/dt' may result in 'pow(x, dx/dt)' having different units while 'dxx/dt' may result in 'pow(xx, dxx/dt)' having different units.",
"The units in 'eqnSin = pow(x, sin(3.0))' in component 'my_component' are not equivalent. 'eqnSin' is in 'second' while 'pow(x, sin(3.0))' is in 'second^0.14112'.",
"The units in 'eqnCos = pow(x, cos(3.0))' in component 'my_component' are not equivalent. 'eqnCos' is in 'second' while 'pow(x, cos(3.0))' is in 'second^-0.989992'.",
"The units in 'eqnTan = pow(x, tan(3.0))' in component 'my_component' are not equivalent. 'eqnTan' is in 'second' while 'pow(x, tan(3.0))' is in 'second^-0.142547'.",
Expand Down Expand Up @@ -881,6 +883,10 @@ TEST(AnalyserUnits, powerValues)
"The units in 'eqnPi = pow(x, pi)' in component 'my_component' are not equivalent. 'eqnPi' is in 'second' while 'pow(x, pi)' is in 'second^3.14159'.",
"The units in 'eqnInfinity = pow(x, infinity)' in component 'my_component' are not equivalent. 'eqnInfinity' is in 'second' while 'pow(x, infinity)' is in 'second^inf' (i.e. '10^nan x second^inf').",
"The units in 'eqnNotanumber = pow(x, notanumber)' in component 'my_component' are not equivalent. 'eqnNotanumber' is in 'second' while 'pow(x, notanumber)' is in 'second^nan' (i.e. '10^nan x second^nan').",
"The type of variable 'eqnCoverage' in component 'my_component' is unknown.",
"The type of variable 'u' in component 'my_component' is unknown.",
"The type of variable 'eqnCoverage2' in component 'my_component' is unknown.",
"The type of variable 'eqnCoverage3' in component 'my_component' is unknown.",
};

auto analyser = libcellml::Analyser::create();
Expand Down
129 changes: 129 additions & 0 deletions tests/resources/analyser/units/power_values.cellml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
</units>
<component name="my_component">
<variable name="t" units="second"/>
<variable name="u" units="dimensionless"/>
<variable name="x" initial_value="1" units="second"/>
<variable name="xx" initial_value="1" units="second"/>
<variable name="y" initial_value="3" units="dimensionless"/>
<variable name="z" initial_value="1" units="dimensionless"/>
<variable name="eqnEq" units="second"/>
Expand Down Expand Up @@ -51,6 +53,7 @@
<variable name="eqnMax2" units="second"/>
<variable name="eqnRem" units="second"/>
<variable name="eqnDiff" units="second"/>
<variable name="eqnDiff2" units="second"/>
<variable name="eqnSin" units="second"/>
<variable name="eqnCos" units="second"/>
<variable name="eqnTan" units="second"/>
Expand Down Expand Up @@ -88,6 +91,9 @@
<variable name="eqnPi" units="second"/>
<variable name="eqnInfinity" units="second"/>
<variable name="eqnNotanumber" units="second"/>
<variable name="eqnCoverage" units="second"/>
<variable name="eqnCoverage2" units="second"/>
<variable name="eqnCoverage3" units="second"/>
<math xmlns="http://www.w3.org/1998/Math/MathML">
<apply>
<eq/>
Expand Down Expand Up @@ -594,6 +600,46 @@
</apply>
</apply>
</apply>
<apply>
<eq/>
<apply>
<power/>
<ci>x</ci>
<apply>
<diff/>
<bvar>
<ci>t</ci>
</bvar>
<ci>x</ci>
</apply>
</apply>
<ci>eqnDiff2</ci>
</apply>
<apply>
<eq/>
<apply>
<power/>
<ci>x</ci>
<apply>
<diff/>
<bvar>
<ci>t</ci>
</bvar>
<ci>x</ci>
</apply>
</apply>
<apply>
<power/>
<ci>xx</ci>
<apply>
<diff/>
<bvar>
<ci>t</ci>
</bvar>
<ci>xx</ci>
</apply>
</apply>
</apply>
<apply>
<eq/>
<ci>eqnSin</ci>
Expand Down Expand Up @@ -1035,6 +1081,89 @@
<notanumber/>
</apply>
</apply>
<apply>
<eq/>
<ci>eqnCoverage</ci>
<apply>
<times/>
<apply>
<minus/>
<apply>
<divide/>
<apply>
<power/>
<ci>x</ci>
<apply>
<minus/>
<ci>y</ci>
<ci>u</ci>
</apply>
</apply>
<apply>
<power/>
<ci>x</ci>
<ci>u</ci>
</apply>
</apply>
</apply>
<apply>
<minus/>
<ci>x</ci>
<ci>x</ci>
</apply>
</apply>
</apply>
<apply>
<eq/>
<ci>eqnCoverage2</ci>
<apply>
<times/>
<apply>
<apply>
<divide/>
<apply>
<power/>
<ci>x</ci>
<apply>
<minus/>
<ci>y</ci>
<ci>u</ci>
</apply>
</apply>
<apply>
<power/>
<ci>x</ci>
<ci>u</ci>
</apply>
</apply>
</apply>
<apply>
<minus/>
<ci>x</ci>
<ci>x</ci>
</apply>
</apply>
</apply>
<apply>
<eq/>
<ci>eqnCoverage3</ci>
<apply>
<times/>
<apply>
<minus/>
<apply>
<power/>
<ci>x</ci>
<ci>u</ci>
</apply>
</apply>
<apply>
<minus/>
<ci>x</ci>
<ci>x</ci>
</apply>
</apply>
</apply>
</math>
</component>
</model>

0 comments on commit 99a4beb

Please sign in to comment.