From a5a57c40765e90b7f66e2e71459813ba3861e088 Mon Sep 17 00:00:00 2001 From: Hugo Saint-Vignes Date: Tue, 16 Jun 2020 17:10:00 +0200 Subject: [PATCH] [poincare] Improve prefixes for mass and inductance unities Change-Id: Ic9eb7b5adff7b172452b4c73bd7ddc5c59761219 --- poincare/include/poincare/unit.h | 15 ++++++---- poincare/src/unit.cpp | 48 ++++++++++++++++++++++---------- poincare/test/simplification.cpp | 13 +++++++-- 3 files changed, 52 insertions(+), 24 deletions(-) diff --git a/poincare/include/poincare/unit.h b/poincare/include/poincare/unit.h index d82bbc8f8..feb6f6d9a 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 double exponent) const; + const Prefix * bestPrefixForValue(double & value, const float exponent) const; private: const char * m_rootSymbol; const char * m_definition; @@ -304,9 +304,12 @@ public: NoPrefix), }, MassRepresentatives[] = { - Representative("g", nullptr, + Representative("kg", nullptr, + Representative::Prefixable::No, + NoPrefix), + Representative("g", "0.001_kg", Representative::Prefixable::Yes, - LongScalePrefixes), + NegativeLongScalePrefixes), Representative("t", "1000_kg", Representative::Prefixable::Yes, NoPrefix), @@ -406,7 +409,7 @@ public: InductanceRepresentatives[] = { Representative("H", "_kg*_m^2*_s^-2*_A^-2", Representative::Prefixable::Yes, - NoPrefix), + LongScalePrefixes), }, CatalyticActivityRepresentatives[] = { Representative("kat", "_mol*_s^-1", @@ -478,7 +481,7 @@ public: .luminuousIntensity = 0, }, MassRepresentatives, - &KiloPrefix + &EmptyPrefix ), Dimension( Dimension::Vector { @@ -795,7 +798,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 double exponent, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext); + void chooseBestMultipleForValue(double * value, const float exponent, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext); Expression removeUnit(Expression * unit); }; diff --git a/poincare/src/unit.cpp b/poincare/src/unit.cpp index c399e507d..f9d01741d 100644 --- a/poincare/src/unit.cpp +++ b/poincare/src/unit.cpp @@ -56,21 +56,39 @@ int UnitNode::Representative::serialize(char * buffer, int bufferSize, const Pre return length; } -const UnitNode::Prefix * UnitNode::Representative::bestPrefixForValue(double & value, const double exponent) const { +static bool compareMagnitudeOrders(float order, float otherOrder) { + /* Precision can be lost (with a year conversion for instance), so the order + * value is rounded */ + if (std::abs(order) < Expression::Epsilon()) { + order = 0.0; + } + if (std::abs(otherOrder) < Expression::Epsilon()) { + otherOrder = 0.0; + } + if (std::abs(std::abs(order) - std::abs(otherOrder)) < 3.0 && order * otherOrder < 0.0) { + /* If the two values are close, and their sign are opposed, the positive + * order is preferred */ + return (order >= 0.0); + } + // Otherwise, the closest order to 0 is preferred + return (std::abs(order) < std::abs(otherOrder)); +} + +const UnitNode::Prefix * UnitNode::Representative::bestPrefixForValue(double & value, const float exponent) const { if (!isPrefixable()) { return &Unit::EmptyPrefix; } + float bestOrder; const Prefix * bestPre = nullptr; - double diff = -1.0; /* Find the 'Prefix' with the most adequate 'exponent' for the order of * magnitude of 'value'. */ - const double orderOfMagnitude = IEEE754::exponentBase10(std::fabs(value)); + const float orderOfMagnitude = std::log10(std::fabs(value)); for (size_t i = 0; i < m_outputPrefixesLength; i++) { const Prefix * pre = m_outputPrefixes[i]; - double newDiff = std::abs(orderOfMagnitude - pre->exponent() * exponent); - if (newDiff < diff || diff < 0.0) { - diff = newDiff; + float order = orderOfMagnitude - pre->exponent() * exponent; + if (bestPre == nullptr || compareMagnitudeOrders(order, bestOrder)) { + bestOrder = order; bestPre = pre; } } @@ -131,11 +149,11 @@ Unit::Dimension::Vector UnitNode::Dimension::Vector::FromBaseU Expression exp = factor.childAtIndex(1); 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) { + float exponent_float = static_cast(exp).node()->templatedApproximate(); + if (std::abs(exponent_float) < 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); + exponent = (int)std::round(exponent_float); + assert(std::abs(exponent_float - 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, @@ -354,7 +372,7 @@ 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; - double exponent = 1.0; + float exponent = 1.0; if (firstFactor.type() == ExpressionNode::Type::Multiplication) { firstFactor = firstFactor.childAtIndex(0); } @@ -362,7 +380,7 @@ void Unit::ChooseBestMultipleForValue(Expression * units, double * value, bool t Expression exp = firstFactor.childAtIndex(1); firstFactor = firstFactor.childAtIndex(0); assert(exp.type() == ExpressionNode::Type::Rational); - exponent = static_cast(exp).node()->templatedApproximate(); + exponent = static_cast(exp).node()->templatedApproximate(); } assert(firstFactor.type() == ExpressionNode::Type::Unit); // Choose its multiple and update value accordingly @@ -371,7 +389,7 @@ void Unit::ChooseBestMultipleForValue(Expression * units, double * value, bool t } } -void Unit::chooseBestMultipleForValue(double * value, const double exponent, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext) { +void Unit::chooseBestMultipleForValue(double * value, const float exponent, bool tuneRepresentative, ExpressionNode::ReductionContext reductionContext) { assert(!std::isnan(*value) && exponent != 0.0); if (*value == 0 || *value == 1.0 || std::isinf(*value)) { return; @@ -393,7 +411,7 @@ void Unit::chooseBestMultipleForValue(double * value, const double exponent, boo double val = *value * std::pow(Division::Builder(clone(), Unit::Builder(dim, rep, &EmptyPrefix)).deepReduce(reductionContext).approximateToScalar(reductionContext.context(), reductionContext.complexFormat(), reductionContext.angleUnit()), exponent); // Get the best prefix and update val accordingly const Prefix * pre = rep->bestPrefixForValue(val, exponent); - if (std::fabs(std::log10(std::fabs(bestVal))) - std::fabs(std::log10(std::fabs(val))) > Epsilon()) { + if (compareMagnitudeOrders(std::log10(std::fabs(val)), std::log10(std::fabs(bestVal)))) { /* At this point, val is closer to one than bestVal is.*/ bestRep = rep; bestPre = pre; @@ -425,7 +443,7 @@ bool Unit::isMeter() const { bool Unit::isKilogram() const { // See comment on isSecond - return node()->dimension() == MassDimension && node()->representative() == KilogramRepresentative && node()->prefix() == &KiloPrefix; + return node()->dimension() == MassDimension && node()->representative() == KilogramRepresentative && node()->prefix() == &EmptyPrefix; } bool Unit::isSI() const { diff --git a/poincare/test/simplification.cpp b/poincare/test/simplification.cpp index 863dab156..b0124a85a 100644 --- a/poincare/test/simplification.cpp +++ b/poincare/test/simplification.cpp @@ -256,13 +256,21 @@ QUIZ_CASE(poincare_simplification_units) { assert_parsed_expression_simplify_to("_kg×_m^2×_s^(-2)×_A^(-2)", "1×_H"); assert_parsed_expression_simplify_to("_mol×_s^-1", "1×_kat"); + /* Displayed order of magnitude */ + assert_parsed_expression_simplify_to("1_t", "1×_t"); + assert_parsed_expression_simplify_to("100_kg", "100×_kg"); + assert_parsed_expression_simplify_to("1_min", "1×_min"); + assert_parsed_expression_simplify_to("0.1_m", "100×_mm"); + assert_parsed_expression_simplify_to("180_MΩ", "180×_MΩ"); + assert_parsed_expression_simplify_to("180_MH", "180×_MH"); + /* Test simplification of all possible (prefixed) unit symbols. * Some symbols are however excluded: * - At present, some units will not appear as simplification output: * t, Hz, S, ha, L. These exceptions are tested below. */ for (const Unit::Dimension * dim = Unit::DimensionTable; dim < Unit::DimensionTableUpperBound; dim++) { for (const Unit::Representative * rep = dim->stdRepresentative(); rep < dim->representativesUpperBound(); rep++) { - if (strcmp(rep->rootSymbol(), "t") == 0 || strcmp(rep->rootSymbol(), "Hz") == 0 || strcmp(rep->rootSymbol(), "S") == 0 || strcmp(rep->rootSymbol(), "ha") == 0 || strcmp(rep->rootSymbol(), "L") == 0) { + if (strcmp(rep->rootSymbol(), "Hz") == 0 || strcmp(rep->rootSymbol(), "S") == 0 || strcmp(rep->rootSymbol(), "ha") == 0 || strcmp(rep->rootSymbol(), "L") == 0) { continue; } static constexpr size_t bufferSize = 12; @@ -280,11 +288,10 @@ QUIZ_CASE(poincare_simplification_units) { } /* Units that do not appear as output yet */ - assert_parsed_expression_simplify_to("_t", "1×_Mg"); assert_parsed_expression_simplify_to("_Hz", "1×_s^\u0012-1\u0013"); assert_parsed_expression_simplify_to("_S", "1×_Ω^\u0012-1\u0013"); assert_parsed_expression_simplify_to("_L", "0.001×_m^3"); - assert_parsed_expression_simplify_to("_ha", "0.01×_km^2"); + assert_parsed_expression_simplify_to("_ha", "10000×_m^2"); /* Unit sum/subtract */ assert_parsed_expression_simplify_to("_m+_m", "2×_m");