From bf9653d51035c1440084f6b623467b13d17772fa Mon Sep 17 00:00:00 2001 From: Ruben Dashyan Date: Tue, 24 Mar 2020 16:17:04 +0100 Subject: [PATCH] [poincare/expression] Turn getUnit into extractUnits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addition::shallowReduce factors the unit across its terms. Previously expressions of the form 1_m+π_m were reduced to undef. --- poincare/include/poincare/division.h | 2 +- poincare/include/poincare/empty_expression.h | 2 +- poincare/include/poincare/expression.h | 2 +- poincare/include/poincare/expression_node.h | 2 +- poincare/include/poincare/function.h | 3 --- poincare/include/poincare/multiplication.h | 4 ++-- poincare/include/poincare/nth_root.h | 2 +- poincare/include/poincare/parenthesis.h | 2 +- poincare/include/poincare/power.h | 3 +-- poincare/include/poincare/subtraction.h | 2 +- poincare/include/poincare/unit.h | 3 +-- poincare/include/poincare/unit_convert.h | 2 +- poincare/src/addition.cpp | 21 +++++++++++++++++-- poincare/src/expression_node.cpp | 2 +- poincare/src/multiplication.cpp | 22 ++++++++++++++------ poincare/src/power.cpp | 16 ++++++-------- poincare/src/unit.cpp | 4 ++-- poincare/src/unit_convert.cpp | 4 ++-- poincare/test/expression_properties.cpp | 4 ++-- poincare/test/simplification.cpp | 1 + 20 files changed, 61 insertions(+), 42 deletions(-) diff --git a/poincare/include/poincare/division.h b/poincare/include/poincare/division.h index 262f4a418..602f1184d 100644 --- a/poincare/include/poincare/division.h +++ b/poincare/include/poincare/division.h @@ -25,7 +25,7 @@ public: // Properties Type type() const override { return Type::Division; } int polynomialDegree(Context * context, const char * symbolName) const override; - Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); } + Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); } // Approximation virtual Evaluation approximate(SinglePrecision p, Context * context, Preferences::ComplexFormat complexFormat, Preferences::AngleUnit angleUnit) const override { diff --git a/poincare/include/poincare/empty_expression.h b/poincare/include/poincare/empty_expression.h index 3814f3e91..3a64cac18 100644 --- a/poincare/include/poincare/empty_expression.h +++ b/poincare/include/poincare/empty_expression.h @@ -22,7 +22,7 @@ public: // Properties Type type() const override { return Type::EmptyExpression; } int serialize(char * buffer, int bufferSize, Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override; - Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); } + Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); } // Simplification LayoutShape leftLayoutShape() const override { diff --git a/poincare/include/poincare/expression.h b/poincare/include/poincare/expression.h index bce4b2490..c25545bca 100644 --- a/poincare/include/poincare/expression.h +++ b/poincare/include/poincare/expression.h @@ -199,7 +199,7 @@ public: Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) { return node()->replaceSymbolWithExpression(symbol, expression); } /* Units */ - Expression getUnit() const { return node()->getUnit(); } + Expression extractUnits() { return node()->extractUnits(); } bool hasUnit() const; /* Complex */ diff --git a/poincare/include/poincare/expression_node.h b/poincare/include/poincare/expression_node.h index 21b19004d..896a72c28 100644 --- a/poincare/include/poincare/expression_node.h +++ b/poincare/include/poincare/expression_node.h @@ -178,7 +178,7 @@ public: virtual float characteristicXRange(Context * context, Preferences::AngleUnit angleUnit) const; bool isOfType(Type * types, int length) const; - virtual Expression getUnit() const; // Only reduced nodes should answer + virtual Expression extractUnits(); // Only reduced nodes should answer /* Simplification */ /* SimplificationOrder returns: diff --git a/poincare/include/poincare/function.h b/poincare/include/poincare/function.h index bb2d2f92c..cc64a531b 100644 --- a/poincare/include/poincare/function.h +++ b/poincare/include/poincare/function.h @@ -28,9 +28,6 @@ public: int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const override; int getVariables(Context * context, isVariableTest isVariable, char * variables, int maxSizeVariable, int nextVariableIndex) const override; float characteristicXRange(Context * context, Preferences::AngleUnit angleUnit) const override; - /* getUnit() is ExpressionNode::getUnit -> - * as the function is reduced, it would have been replaced if it had a - * definition. It thus has no definition, so no unit. */ private: char m_name[0]; // MUST be the last member variable diff --git a/poincare/include/poincare/multiplication.h b/poincare/include/poincare/multiplication.h index cf16e64ba..1021ec9e0 100644 --- a/poincare/include/poincare/multiplication.h +++ b/poincare/include/poincare/multiplication.h @@ -25,7 +25,7 @@ public: int polynomialDegree(Context * context, const char * symbolName) const override; int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const override; bool childAtIndexNeedsUserParentheses(const Expression & child, int childIndex) const override; - Expression getUnit() const override; + Expression extractUnits() override; // Approximation template static Complex compute(const std::complex c, const std::complex d, Preferences::ComplexFormat complexFormat) { return Complex::Builder(c*d); } @@ -76,7 +76,7 @@ public: // Properties int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const; - Expression getUnit() const; + Expression extractUnits(); // Approximation template static void computeOnArrays(T * m, T * n, T * result, int mNumberOfColumns, int mNumberOfRows, int nNumberOfColumns); // Simplification diff --git a/poincare/include/poincare/nth_root.h b/poincare/include/poincare/nth_root.h index d0d1d8e87..5132282f6 100644 --- a/poincare/include/poincare/nth_root.h +++ b/poincare/include/poincare/nth_root.h @@ -20,7 +20,7 @@ public: #endif private: - Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); } + Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); } // Layout Layout createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override; int serialize(char * buffer, int bufferSize, Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override; diff --git a/poincare/include/poincare/parenthesis.h b/poincare/include/poincare/parenthesis.h index 067898502..682c7630a 100644 --- a/poincare/include/poincare/parenthesis.h +++ b/poincare/include/poincare/parenthesis.h @@ -21,7 +21,7 @@ public: // Properties Type type() const override { return Type::Parenthesis; } int polynomialDegree(Context * context, const char * symbolName) const override; - Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); } + Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); } // Layout Layout createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override; diff --git a/poincare/include/poincare/power.h b/poincare/include/poincare/power.h index ad269d15d..d9da121c7 100644 --- a/poincare/include/poincare/power.h +++ b/poincare/include/poincare/power.h @@ -29,7 +29,7 @@ public: Sign sign(Context * context) const override; Expression setSign(Sign s, ReductionContext reductionContext) override; bool childAtIndexNeedsUserParentheses(const Expression & child, int childIndex) const override; - Expression getUnit() const override; + Expression extractUnits() override; int polynomialDegree(Context * context, const char * symbolName) const override; int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const override; @@ -79,7 +79,6 @@ public: int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[]) const; Expression shallowReduce(ExpressionNode::ReductionContext reductionContext); Expression shallowBeautify(ExpressionNode::ReductionContext reductionContext); - Expression getUnit() const; private: constexpr static int k_maxExactPowerMatrix = 100; diff --git a/poincare/include/poincare/subtraction.h b/poincare/include/poincare/subtraction.h index f8703648e..96db82b72 100644 --- a/poincare/include/poincare/subtraction.h +++ b/poincare/include/poincare/subtraction.h @@ -24,7 +24,7 @@ public: Type type() const override { return Type::Subtraction; } int polynomialDegree(Context * context, const char * symbolName) const override; bool childAtIndexNeedsUserParentheses(const Expression & child, int childIndex) const override; - Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); } + Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); } // Approximation template static Complex compute(const std::complex c, const std::complex d, Preferences::ComplexFormat complexFormat) { return Complex::Builder(c - d); } diff --git a/poincare/include/poincare/unit.h b/poincare/include/poincare/unit.h index 97bc4ba87..f3b662e7d 100644 --- a/poincare/include/poincare/unit.h +++ b/poincare/include/poincare/unit.h @@ -151,7 +151,7 @@ public: // Expression Properties Type type() const override { return Type::Unit; } Sign sign(Context * context) const override; - Expression getUnit() const override; + Expression extractUnits() override; /* Layout */ Layout createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override; @@ -725,7 +725,6 @@ public: Unit(const UnitNode * node) : Expression(node) {} static Unit Builder(const Dimension * dimension, const Representative * representative, const Prefix * prefix); - Expression getUnit() const { return clone(); } // Simplification Expression shallowReduce(ExpressionNode::ReductionContext reductionContext); diff --git a/poincare/include/poincare/unit_convert.h b/poincare/include/poincare/unit_convert.h index 70943a9f5..c142cb712 100644 --- a/poincare/include/poincare/unit_convert.h +++ b/poincare/include/poincare/unit_convert.h @@ -20,7 +20,7 @@ public: Type type() const override { return Type::UnitConvert; } private: - Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); } + Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); } // Simplification Expression shallowReduce(ReductionContext reductionContext) override; // Evalutation diff --git a/poincare/src/addition.cpp b/poincare/src/addition.cpp index 501a57a10..4bdec595a 100644 --- a/poincare/src/addition.cpp +++ b/poincare/src/addition.cpp @@ -155,16 +155,33 @@ Expression Addition::shallowReduce(ExpressionNode::ReductionContext reductionCon /* Step 2: Handle the units. All children should have the same unit, otherwise * the result is not homogeneous. */ { - Expression unit = childAtIndex(0).getUnit(); + Expression unit = childAtIndex(0).extractUnits(); const bool hasUnit = !unit.isUninitialized(); for (int i = 1; i < childrenCount; i++) { - Expression otherUnit = childAtIndex(i).getUnit(); + Expression otherUnit = childAtIndex(i).extractUnits(); if (hasUnit == otherUnit.isUninitialized() || (hasUnit && !unit.isIdenticalTo(otherUnit))) { return replaceWithUndefinedInPlace(); } } + if (hasUnit) { + for (int i = 0; i < childrenCount; i++) { + /* Any unary hierarchy must be squashed here since it has not been + * done in extractUnits. + */ + Expression child = childAtIndex(i); + if (child.type() == ExpressionNode::Type::Multiplication) { + static_cast(child).squashUnaryHierarchyInPlace(); + } + } + Expression addition = shallowReduce(reductionContext); + Multiplication result = Multiplication::Builder(unit); + result.mergeSameTypeChildrenInPlace(); + addition.replaceWithInPlace(result); + result.addChildAtIndexInPlace(addition, 0, 1); + return std::move(result); + } } // Step 3: Sort the children diff --git a/poincare/src/expression_node.cpp b/poincare/src/expression_node.cpp index 1ac78e729..dc87f141f 100644 --- a/poincare/src/expression_node.cpp +++ b/poincare/src/expression_node.cpp @@ -136,7 +136,7 @@ bool ExpressionNode::isOfType(Type * types, int length) const { return false; } -Expression ExpressionNode::getUnit() const { +Expression ExpressionNode::extractUnits() { return Expression(); } diff --git a/poincare/src/multiplication.cpp b/poincare/src/multiplication.cpp index c482edb56..419db00bf 100644 --- a/poincare/src/multiplication.cpp +++ b/poincare/src/multiplication.cpp @@ -63,8 +63,8 @@ bool MultiplicationNode::childAtIndexNeedsUserParentheses(const Expression & chi return child.isOfType(types, 2); } -Expression MultiplicationNode::getUnit() const { - return Multiplication(this).getUnit(); +Expression MultiplicationNode::extractUnits() { + return Multiplication(this).extractUnits(); } template @@ -255,20 +255,30 @@ int Multiplication::getPolynomialCoefficients(Context * context, const char * sy return deg; } -Expression Multiplication::getUnit() const { +Expression Multiplication::extractUnits() { Multiplication result = Multiplication::Builder(); int resultChildrenCount = 0; - const int childrenCount = numberOfChildren(); - for (int i = 0; i < childrenCount; i++) { - Expression currentUnit = childAtIndex(i).getUnit(); + for (int i = 0; i < numberOfChildren(); i++) { + Expression currentUnit = childAtIndex(i).extractUnits(); if (!currentUnit.isUninitialized()) { + assert(childAtIndex(i) == currentUnit); result.addChildAtIndexInPlace(currentUnit, resultChildrenCount, resultChildrenCount); resultChildrenCount++; + removeChildAtIndexInPlace(i--); } } if (resultChildrenCount == 0) { return Expression(); } + /* squashUnaryHierarchyInPlace(); + * That would make 'this' invalid, so we would rather keep any unary + * hierarchy as it is and handle it later. + * TODO ? + * A possible solution would be that the extractUnits method becomes + * Expression extractUnits(Expression & units) + * returning the Expression that is left after extracting the units + * and setting the units reference instead of returning the units. + */ return result.squashUnaryHierarchyInPlace(); } diff --git a/poincare/src/power.cpp b/poincare/src/power.cpp index b8f096678..576820dc9 100644 --- a/poincare/src/power.cpp +++ b/poincare/src/power.cpp @@ -81,8 +81,12 @@ int PowerNode::polynomialDegree(Context * context, const char * symbolName) cons return -1; } -Expression PowerNode::getUnit() const { - return Power(this).getUnit(); +Expression PowerNode::extractUnits() { + if (!childAtIndex(0)->extractUnits().isUninitialized()) { + assert(childAtIndex(0)->type() == ExpressionNode::Type::Unit); + return Power(this); + } + return ExpressionNode::extractUnits(); } int PowerNode::getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const { @@ -985,14 +989,6 @@ Expression Power::shallowBeautify(ExpressionNode::ReductionContext reductionCont return *this; } -Expression Power::getUnit() const { - Expression baseUnit = childAtIndex(0).getUnit(); - if (baseUnit.isUninitialized()) { - return baseUnit; - } - return Power::Builder(baseUnit, childAtIndex(1).clone()); -} - // Private // Simplification diff --git a/poincare/src/unit.cpp b/poincare/src/unit.cpp index 344654270..7c7a141cf 100644 --- a/poincare/src/unit.cpp +++ b/poincare/src/unit.cpp @@ -164,8 +164,8 @@ ExpressionNode::Sign UnitNode::sign(Context * context) const { return Sign::Positive; } -Expression UnitNode::getUnit() const { - return Unit(this).getUnit(); +Expression UnitNode::extractUnits() { + return Unit(this); } int UnitNode::simplificationOrderSameType(const ExpressionNode * e, bool ascending, bool canBeInterrupted, bool ignoreParentheses) const { diff --git a/poincare/src/unit_convert.cpp b/poincare/src/unit_convert.cpp index a4d4fedd5..8a448af0e 100644 --- a/poincare/src/unit_convert.cpp +++ b/poincare/src/unit_convert.cpp @@ -41,7 +41,7 @@ Expression UnitConvert::shallowReduce(ExpressionNode::ReductionContext reduction reductionContext.angleUnit(), reductionContext.target(), ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithUndefinedAndReplaceUnits); - Expression unit = childAtIndex(1).clone().reduce(reductionContextWithUnits).getUnit(); + Expression unit = childAtIndex(1).clone().reduce(reductionContextWithUnits).extractUnits(); if (unit.isUninitialized()) { // There is no unit on the right return replaceWithUndefinedInPlace(); @@ -54,7 +54,7 @@ Expression UnitConvert::shallowReduce(ExpressionNode::ReductionContext reduction reductionContext.angleUnit(), reductionContext.target(), ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithUndefinedAndDoNotReplaceUnits); - Expression finalUnit = childAtIndex(1).reduce(reductionContextWithoutUnits).getUnit(); + Expression finalUnit = childAtIndex(1).reduce(reductionContextWithoutUnits).extractUnits(); // Divide the left member by the new unit Expression division = Division::Builder(childAtIndex(0), finalUnit.clone()); diff --git a/poincare/test/expression_properties.cpp b/poincare/test/expression_properties.cpp index 854888d5b..4bfd28073 100644 --- a/poincare/test/expression_properties.cpp +++ b/poincare/test/expression_properties.cpp @@ -374,10 +374,10 @@ void assert_reduced_expression_unit(const char * expression, const char * unit, ExpressionNode::ReductionContext redContext(&globalContext, Real, Degree, SystemForApproximation, symbolicComutation); Expression e = parse_expression(expression, &globalContext, false); e = e.reduce(redContext); - Expression u1 = e.getUnit(); + Expression u1 = e.extractUnits(); Expression u2 = parse_expression(unit, &globalContext, false); u2 = u2.reduce(redContext); - u2 = u2.getUnit(); + u2 = u2.extractUnits(); quiz_assert_print_if_failure(u1.isUninitialized() == u2.isUninitialized() && (u1.isUninitialized() || u1.isIdenticalTo(u2)), expression); } diff --git a/poincare/test/simplification.cpp b/poincare/test/simplification.cpp index e4bb5fd41..04359e9d8 100644 --- a/poincare/test/simplification.cpp +++ b/poincare/test/simplification.cpp @@ -414,6 +414,7 @@ QUIZ_CASE(poincare_simplification_units) { /* Valid expressions */ assert_parsed_expression_simplify_to("-2×_A", "-2×_A"); assert_parsed_expression_simplify_to("cos(1_s/1_s)", "cos(1)"); + assert_parsed_expression_simplify_to("1_m+π_m+√(2)_m-cos(15)_m", "6.3154941288217×_m"); } QUIZ_CASE(poincare_simplification_power) {