diff --git a/src/analyser.cpp b/src/analyser.cpp index 8b7c869481..969e1230ad 100644 --- a/src/analyser.cpp +++ b/src/analyser.cpp @@ -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. // // +--------+ // | + | @@ -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. @@ -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: diff --git a/tests/analyser/analyserunits.cpp b/tests/analyser/analyserunits.cpp index 3bdf8954b1..cfa856d8d0 100644 --- a/tests/analyser/analyserunits.cpp +++ b/tests/analyser/analyserunits.cpp @@ -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'.", @@ -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(); diff --git a/tests/resources/analyser/units/power_values.cellml b/tests/resources/analyser/units/power_values.cellml index b84109cc34..86a0b5b5c2 100644 --- a/tests/resources/analyser/units/power_values.cellml +++ b/tests/resources/analyser/units/power_values.cellml @@ -11,7 +11,9 @@ + + @@ -51,6 +53,7 @@ + @@ -88,6 +91,9 @@ + + + @@ -594,6 +600,46 @@ + + + + + x + + + + t + + x + + + eqnDiff2 + + + + + + x + + + + t + + x + + + + + xx + + + + t + + xx + + + eqnSin @@ -1035,6 +1081,89 @@ + + + eqnCoverage + + + + + + + + + x + + + y + u + + + + + x + u + + + + + + x + x + + + + + + eqnCoverage2 + + + + + + + + x + + + y + u + + + + + x + u + + + + + + x + x + + + + + + eqnCoverage3 + + + + + + + x + u + + + + + x + x + + +