From 51f1cdb076c16c49643d7965bd637e46c1247fef Mon Sep 17 00:00:00 2001 From: Hugo Saint-Vignes Date: Tue, 16 Jun 2020 16:59:16 +0200 Subject: [PATCH] [poincare] Handle rational unit exponents Change-Id: Id710702dbed19d34992da90978d5823d68abb80a --- liba/include/stdint.h | 3 ++ poincare/include/poincare/unit.h | 4 +-- poincare/src/multiplication.cpp | 4 +++ poincare/src/power.cpp | 8 ++--- poincare/src/square_root.cpp | 1 - poincare/src/unit.cpp | 54 ++++++++++++++++++-------------- poincare/test/simplification.cpp | 4 +-- 7 files changed, 46 insertions(+), 32 deletions(-) diff --git a/liba/include/stdint.h b/liba/include/stdint.h index 1096e5ab1..73a2be5e8 100644 --- a/liba/include/stdint.h +++ b/liba/include/stdint.h @@ -26,6 +26,9 @@ typedef int64_t int_fast64_t; typedef uint8_t uint_least8_t; +#define INT8_MAX 0x7f +#define INT8_MIN (-INT8_MAX-1) + #define UINT8_MAX 0xff #define UINT16_MAX 0xffff diff --git a/poincare/include/poincare/unit.h b/poincare/include/poincare/unit.h index 1d8a172dd..9ab3c0929 100644 --- a/poincare/include/poincare/unit.h +++ b/poincare/include/poincare/unit.h @@ -72,7 +72,7 @@ public: bool canParse(const char * symbol, size_t length, const Prefix * * prefix) const; int serialize(char * buffer, int bufferSize, const Prefix * prefix) const; - const Prefix * bestPrefixForValue(double & value, const int exponent) const; + const Prefix * bestPrefixForValue(double & value, const double exponent) const; private: const char * m_rootSymbol; const char * m_definition; @@ -793,7 +793,7 @@ private: UnitNode * node() const { return static_cast(Expression::node()); } bool isSI() const; static void ChooseBestMultipleForValue(Expression * units, double * value, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext); - void chooseBestMultipleForValue(double * value, const int exponent, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext); + void chooseBestMultipleForValue(double * value, const double exponent, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext); Expression removeUnit(Expression * unit); }; diff --git a/poincare/src/multiplication.cpp b/poincare/src/multiplication.cpp index 5a44c1a71..5df6d9d98 100644 --- a/poincare/src/multiplication.cpp +++ b/poincare/src/multiplication.cpp @@ -391,6 +391,10 @@ Expression Multiplication::shallowBeautify(ExpressionNode::ReductionContext redu * - Repeat those steps until no more simplification is possible. */ Multiplication unitsAccu = Multiplication::Builder(); + /* If exponents are not integers, FromBaseUnits will return the closest + * representation of units with base units and integer exponents. + * It cause no problem because once the best derived units are found, + * units is divided then multiplied by them. */ Unit::Dimension::Vector unitsExponents = Unit::Dimension::Vector::FromBaseUnits(units); Unit::Dimension::Vector::Metrics unitsMetrics = unitsExponents.metrics(); Unit::Dimension::Vector bestRemainderExponents; diff --git a/poincare/src/power.cpp b/poincare/src/power.cpp index 4b8f2f617..3ecb7587e 100644 --- a/poincare/src/power.cpp +++ b/poincare/src/power.cpp @@ -418,8 +418,8 @@ Expression Power::shallowReduce(ExpressionNode::ReductionContext reductionContex } assert(index == childAtIndex(1)); if (base.hasUnit()) { - if (index.type() != ExpressionNode::Type::Rational || !static_cast(index).isInteger()) { - // The exponent must be an Integer + if (index.type() != ExpressionNode::Type::Rational) { + // The exponent must be an Rational return replaceWithUndefinedInPlace(); } } @@ -995,8 +995,8 @@ Expression Power::shallowBeautify(ExpressionNode::ReductionContext reductionCont p.shallowReduce(reductionContext); return d.shallowBeautify(reductionContext); } - // Step 2: Turn a^(1/n) into root(a, n) - if (childAtIndex(1).type() == ExpressionNode::Type::Rational && childAtIndex(1).convert().signedIntegerNumerator().isOne()) { + // Step 2: Turn a^(1/n) into root(a, n), unless base is a unit + if (childAtIndex(1).type() == ExpressionNode::Type::Rational && childAtIndex(1).convert().signedIntegerNumerator().isOne() && childAtIndex(0).type() != ExpressionNode::Type::Unit) { Integer index = childAtIndex(1).convert().integerDenominator(); // Special case: a^(1/2) --> sqrt(a) if (index.isEqualTo(Integer(2))) { diff --git a/poincare/src/square_root.cpp b/poincare/src/square_root.cpp index 0c8361d1e..cdefee1a5 100644 --- a/poincare/src/square_root.cpp +++ b/poincare/src/square_root.cpp @@ -40,7 +40,6 @@ Expression SquareRootNode::shallowReduce(ReductionContext reductionContext) { Expression SquareRoot::shallowReduce(ExpressionNode::ReductionContext reductionContext) { { Expression e = Expression::defaultShallowReduce(); - e = e.defaultHandleUnitsInChildren(); if (e.isUndefined()) { return e; } diff --git a/poincare/src/unit.cpp b/poincare/src/unit.cpp index bc6cceb18..c399e507d 100644 --- a/poincare/src/unit.cpp +++ b/poincare/src/unit.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -16,8 +17,6 @@ namespace Poincare { -static inline int absInt(int x) { return x >= 0 ? x : -x; } - int UnitNode::Prefix::serialize(char * buffer, int bufferSize) const { assert(bufferSize >= 0); return std::min(strlcpy(buffer, m_symbol, bufferSize), bufferSize - 1); @@ -57,20 +56,20 @@ int UnitNode::Representative::serialize(char * buffer, int bufferSize, const Pre return length; } -const UnitNode::Prefix * UnitNode::Representative::bestPrefixForValue(double & value, const int exponent) const { +const UnitNode::Prefix * UnitNode::Representative::bestPrefixForValue(double & value, const double exponent) const { if (!isPrefixable()) { return &Unit::EmptyPrefix; } const Prefix * bestPre = nullptr; - unsigned int diff = -1; + double diff = -1.0; /* Find the 'Prefix' with the most adequate 'exponent' for the order of * magnitude of 'value'. */ - const int orderOfMagnitude = IEEE754::exponentBase10(std::fabs(value)); + const double orderOfMagnitude = IEEE754::exponentBase10(std::fabs(value)); for (size_t i = 0; i < m_outputPrefixesLength; i++) { const Prefix * pre = m_outputPrefixes[i]; - unsigned int newDiff = absInt(orderOfMagnitude - pre->exponent() * exponent); - if (newDiff < diff) { + double newDiff = std::abs(orderOfMagnitude - pre->exponent() * exponent); + if (newDiff < diff || diff < 0.0) { diff = newDiff; bestPre = pre; } @@ -112,6 +111,8 @@ Unit::Dimension::Vector::Metrics UnitNode::Dimension::Vector::me template<> Unit::Dimension::Vector UnitNode::Dimension::Vector::FromBaseUnits(const Expression baseUnits) { + /* Returns the vector of Base units with integer exponents. If rational, the + * closest integer will be used. */ Vector vector; int numberOfFactors; int factorIndex = 0; @@ -128,8 +129,20 @@ Unit::Dimension::Vector UnitNode::Dimension::Vector::FromBaseU Integer exponent(1); if (factor.type() == ExpressionNode::Type::Power) { Expression exp = factor.childAtIndex(1); - assert(exp.type() == ExpressionNode::Type::Rational && static_cast(exp).isInteger()); - exponent = static_cast(exp).signedIntegerNumerator(); + assert(exp.type() == ExpressionNode::Type::Rational); + // Using the closest integer to the exponent. + double exponent_double = static_cast(exp).node()->templatedApproximate(); + if (std::fabs(exponent_double) < INT_MAX / 2) { + // Exponent can be safely casted as int + exponent = (int)std::round(exponent_double); + assert(std::fabs(exponent_double - exponent.approximate()) <= 0.5); + } else { + /* Base units vector will ignore this coefficient, that could have been + * casted as int8_t in CanSimplifyUnitProduct, leading to homogeneous, + * but badly formatted units. Any way, the missing exponent won't affect + * CanSimplifyUnitProduct as homogeneity is conserved. */ + exponent = 0; + } factor = factor.childAtIndex(0); } // Fill the vector with the unit's exponent @@ -184,7 +197,7 @@ int UnitNode::simplificationOrderSameType(const ExpressionNode * e, bool ascendi if (dimdiff != 0) { return dimdiff; } - // This works because reprensentatives are ordered in a table + // This works because representatives are ordered in a table const ptrdiff_t repdiff = eNode->representative() - m_representative; if (repdiff != 0) { /* We order representatives in the reverse order as how they're stored in @@ -341,31 +354,25 @@ Expression Unit::shallowBeautify(ExpressionNode::ReductionContext reductionConte void Unit::ChooseBestMultipleForValue(Expression * units, double * value, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext) { // Identify the first Unit factor and its exponent Expression firstFactor = *units; - int exponent = 1; + double exponent = 1.0; if (firstFactor.type() == ExpressionNode::Type::Multiplication) { firstFactor = firstFactor.childAtIndex(0); } if (firstFactor.type() == ExpressionNode::Type::Power) { Expression exp = firstFactor.childAtIndex(1); firstFactor = firstFactor.childAtIndex(0); - assert(exp.type() == ExpressionNode::Type::Rational && static_cast(exp).isInteger()); - Integer expInt = static_cast(exp).signedIntegerNumerator(); - if (expInt.isExtractable()) { - exponent = expInt.extractedInt(); - } else { - // The exponent is too large to be extracted, so do not try to use it. - exponent = 0; - } + assert(exp.type() == ExpressionNode::Type::Rational); + exponent = static_cast(exp).node()->templatedApproximate(); } assert(firstFactor.type() == ExpressionNode::Type::Unit); // Choose its multiple and update value accordingly - if (exponent != 0) { + if (exponent != 0.0) { static_cast(firstFactor).chooseBestMultipleForValue(value, exponent, tuneRepresentative, reductionContext); } } -void Unit::chooseBestMultipleForValue(double * value, const int exponent, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext) { - assert(!std::isnan(*value) && exponent != 0); +void Unit::chooseBestMultipleForValue(double * value, const double exponent, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext) { + assert(!std::isnan(*value) && exponent != 0.0); if (*value == 0 || *value == 1.0 || std::isinf(*value)) { return; } @@ -442,7 +449,8 @@ bool Unit::IsSI(Expression & e) { return true; } if (e.type() == ExpressionNode::Type::Power) { - assert(e.childAtIndex(1).type() == ExpressionNode::Type::Rational && e.childAtIndex(1).convert().isInteger()); + // Rational exponents are accepted in IS system + assert(e.childAtIndex(1).type() == ExpressionNode::Type::Rational); Expression child = e.childAtIndex(0); assert(child.type() == ExpressionNode::Type::Unit); return IsSI(child); diff --git a/poincare/test/simplification.cpp b/poincare/test/simplification.cpp index efba56df3..a85529aee 100644 --- a/poincare/test/simplification.cpp +++ b/poincare/test/simplification.cpp @@ -319,7 +319,8 @@ QUIZ_CASE(poincare_simplification_units) { assert_parsed_expression_simplify_to("log(undef)*_s", "undef"); /* Units with invalid exponent */ - assert_parsed_expression_simplify_to("_s^(1/2)", "undef"); + assert_parsed_expression_simplify_to("_s^(_s)", "undef"); + assert_parsed_expression_simplify_to("_s^(π)", "undef"); /* Inhomogeneous expressions */ assert_parsed_expression_simplify_to("1+_s", "undef"); @@ -416,7 +417,6 @@ QUIZ_CASE(poincare_simplification_units) { assert_parsed_expression_simplify_to("tanh(_s)", "undef"); assert_parsed_expression_simplify_to("trace(_s)", "undef"); assert_parsed_expression_simplify_to("transpose(_s)", "undef"); - assert_parsed_expression_simplify_to("√(_s)", "undef"); /* Valid expressions */ assert_parsed_expression_simplify_to("-2×_A", "-2×_A");