ICU-21862 icu4c unit conversions: support inverting 0 and Infinity (for vehicle-fuel)

See #1947
This commit is contained in:
Hugo van der Merwe 2021-12-03 14:36:59 +00:00 committed by Shane F. Carr
parent 08c3f99c08
commit 44c7137ae5
6 changed files with 96 additions and 71 deletions

View file

@ -9,6 +9,7 @@
#include "cmemory.h"
#include "double-conversion-string-to-double.h"
#include "measunit_impl.h"
#include "putilimp.h"
#include "uassert.h"
#include "unicode/errorcode.h"
#include "unicode/localpointer.h"
@ -588,10 +589,7 @@ double UnitsConverter::convert(double inputValue) const {
if (conversionRate_.reciprocal) {
if (result == 0) {
// TODO: demonstrate the resulting behaviour in tests... and figure
// out desired behaviour. (Theoretical result should be infinity,
// not 0.)
return 0.0;
return uprv_getInfinity();
}
result = 1.0 / result;
}
@ -603,10 +601,7 @@ double UnitsConverter::convertInverse(double inputValue) const {
double result = inputValue;
if (conversionRate_.reciprocal) {
if (result == 0) {
// TODO(ICU-21862): demonstrate the resulting behaviour in tests...
// and figure out desired behaviour. (Theoretical result should be
// infinity, not 0.)
return 0.0;
return uprv_getInfinity();
}
result = 1.0 / result;
}

View file

@ -2188,14 +2188,20 @@ UBool IntlTest::assertEqualsNear(const char* message,
double expected,
double actual,
double delta) {
bool bothNaN = std::isnan(expected) && std::isnan(actual);
bool bothPosInf = uprv_isPositiveInfinity(expected) && uprv_isPositiveInfinity(actual);
bool bothNegInf = uprv_isNegativeInfinity(expected) && uprv_isNegativeInfinity(actual);
if (bothPosInf || bothNegInf || bothNaN) {
// We don't care about delta in these cases
return TRUE;
}
if (std::isnan(delta) || std::isinf(delta)) {
errln((UnicodeString)("FAIL: ") + message + "; nonsensical delta " + delta +
" - delta may not be NaN or Inf");
" - delta may not be NaN or Inf. (Got " + actual + "; expected " + expected + ".)");
return FALSE;
}
bool bothNaN = std::isnan(expected) && std::isnan(actual);
double difference = std::abs(expected - actual);
if (expected != actual && (difference > delta || std::isnan(difference)) && !bothNaN) {
if (expected != actual && (difference > delta || std::isnan(difference))) {
errln((UnicodeString)("FAIL: ") + message + "; got " + actual + "; expected " + expected +
"; acceptable delta " + delta);
return FALSE;

View file

@ -1747,7 +1747,7 @@ void NumberFormatterApiTest::unitUsage() {
30500,
u"350 m");
assertFormatSingle(u"Fuel consumption: inverted units", //
assertFormatSingle(u"Fuel consumption: inverted units", //
u"unit/liter-per-100-kilometer usage/vehicle-fuel", //
u"unit/liter-per-100-kilometer usage/vehicle-fuel", //
NumberFormatter::with() //
@ -1757,17 +1757,35 @@ void NumberFormatterApiTest::unitUsage() {
6.6, //
"36 mpg");
// // TODO(ICU-21862): determine desired behaviour. Commented out for now to not enforce undesirable
// // behaviour
// assertFormatSingle(u"Fuel consumption: inverted units, divide-by-zero", //
// u"unit/liter-per-100-kilometer usage/vehicle-fuel", //
// u"unit/liter-per-100-kilometer usage/vehicle-fuel", //
// NumberFormatter::with() //
// .unit(MeasureUnit::forIdentifier("liter-per-100-kilometer", status)) //
// .usage("vehicle-fuel"), //
// Locale("en-US"), //
// 0, //
// "0 mpg"); // TODO(ICU-21862)
assertFormatSingle(u"Fuel consumption: inverted units, divide-by-zero, en-US", //
u"unit/liter-per-100-kilometer usage/vehicle-fuel", //
u"unit/liter-per-100-kilometer usage/vehicle-fuel", //
NumberFormatter::with() //
.unit(MeasureUnit::forIdentifier("liter-per-100-kilometer", status)) //
.usage("vehicle-fuel"), //
Locale("en-US"), //
0, //
u"∞ mpg");
assertFormatSingle(u"Fuel consumption: inverted units, divide-by-zero, en-ZA", //
u"unit/mile-per-gallon usage/vehicle-fuel", //
u"unit/mile-per-gallon usage/vehicle-fuel", //
NumberFormatter::with() //
.unit(MeasureUnit::forIdentifier("mile-per-gallon", status)) //
.usage("vehicle-fuel"), //
Locale("en-ZA"), //
0, //
u"∞ l/100 km");
assertFormatSingle(u"Fuel consumption: inverted units, divide-by-inf", //
u"unit/mile-per-gallon usage/vehicle-fuel", //
u"unit/mile-per-gallon usage/vehicle-fuel", //
NumberFormatter::with() //
.unit(MeasureUnit::forIdentifier("mile-per-gallon", status)) //
.usage("vehicle-fuel"), //
Locale("de-CH"), //
uprv_getInfinity(), //
u"0 L/100 km");
// Test calling `.usage("")` should unset the existing usage.
// First: without usage

View file

@ -326,8 +326,11 @@ void UnitsTest::testConverter() {
{"cubic-meter-per-meter", "mile-per-gallon", 2.1383143939394E-6, 1.1},
{"cubic-meter-per-meter", "mile-per-gallon", 2.6134953703704E-6, 0.9},
{"liter-per-100-kilometer", "mile-per-gallon", 6.6, 35.6386},
// // TODO(ICU-21862): we should probably return something other than "0":
// {"liter-per-100-kilometer", "mile-per-gallon", 0, 0},
{"liter-per-100-kilometer", "mile-per-gallon", 0, uprv_getInfinity()},
{"mile-per-gallon", "liter-per-100-kilometer", 0, uprv_getInfinity()},
{"mile-per-gallon", "liter-per-100-kilometer", uprv_getInfinity(), 0},
// We skip testing -Inf, because the inverse conversion loses the sign:
// {"mile-per-gallon", "liter-per-100-kilometer", -uprv_getInfinity(), 0},
// Test Aliases
// Alias is just another name to the same unit. Therefore, converting
@ -351,54 +354,42 @@ void UnitsTest::testConverter() {
continue;
}
double maxDelta = 1e-6 * uprv_fabs(testCase.expectedValue);
if (testCase.expectedValue == 0) {
maxDelta = 1e-12;
}
double inverseMaxDelta = 1e-6 * uprv_fabs(testCase.inputValue);
if (testCase.inputValue == 0) {
inverseMaxDelta = 1e-12;
}
ConversionRates conversionRates(status);
if (status.errIfFailureAndReset("conversionRates(status)")) {
continue;
}
UnitsConverter converter(source, target, conversionRates, status);
if (status.errIfFailureAndReset("UnitsConverter(<%s>, <%s>, ...)", testCase.source,
testCase.target)) {
continue;
}
double maxDelta = 1e-6 * uprv_fabs(testCase.expectedValue);
if (testCase.expectedValue == 0) {
maxDelta = 1e-12;
}
assertEqualsNear(UnicodeString("testConverter: ") + testCase.source + " to " + testCase.target,
testCase.expectedValue, converter.convert(testCase.inputValue), maxDelta);
maxDelta = 1e-6 * uprv_fabs(testCase.inputValue);
if (testCase.inputValue == 0) {
maxDelta = 1e-12;
}
assertEqualsNear(
UnicodeString("testConverter inverse: ") + testCase.target + " back to " + testCase.source,
testCase.inputValue, converter.convertInverse(testCase.expectedValue), maxDelta);
testCase.inputValue, converter.convertInverse(testCase.expectedValue), inverseMaxDelta);
// TODO: Test UnitsConverter created using CLDR separately.
// Test UnitsConverter created by CLDR unit identifiers
UnitsConverter converter2(testCase.source, testCase.target, status);
if (status.errIfFailureAndReset("UnitsConverter(<%s>, <%s>, ...)", testCase.source,
testCase.target)) {
continue;
}
maxDelta = 1e-6 * uprv_fabs(testCase.expectedValue);
if (testCase.expectedValue == 0) {
maxDelta = 1e-12;
}
assertEqualsNear(UnicodeString("testConverter2: ") + testCase.source + " to " + testCase.target,
testCase.expectedValue, converter2.convert(testCase.inputValue), maxDelta);
maxDelta = 1e-6 * uprv_fabs(testCase.inputValue);
if (testCase.inputValue == 0) {
maxDelta = 1e-12;
}
assertEqualsNear(
UnicodeString("testConverter2 inverse: ") + testCase.target + " back to " + testCase.source,
testCase.inputValue, converter2.convertInverse(testCase.expectedValue), maxDelta);
testCase.inputValue, converter2.convertInverse(testCase.expectedValue), inverseMaxDelta);
}
}

View file

@ -474,6 +474,11 @@ public class UnitsTest {
new TestData("liter-per-100-kilometer", "mile-per-gallon", 6.6, 35.6386),
// // TODO(ICU-21862): we should probably return something other than "0":
// new TestData("liter-per-100-kilometer", "mile-per-gallon", 0, 0),
// new TestData("mile-per-gallon", "liter-per-100-kilometer", 0, 0),
// // TODO(ICU-21862): deal with infinity input in Java?
// new TestData("mile-per-gallon", "liter-per-100-kilometer", INFINITY, 0),
// We skip testing -Inf, because the inverse conversion loses the sign:
// new TestData("mile-per-gallon", "liter-per-100-kilometer", -INFINITY, 0),
// Test Aliases
// Alias is just another name to the same unit. Therefore, converting
// between them should be the same.
@ -488,45 +493,32 @@ public class UnitsTest {
MeasureUnitImpl source = MeasureUnitImpl.forIdentifier(test.sourceIdentifier);
MeasureUnitImpl target = MeasureUnitImpl.forIdentifier(test.targetIdentifier);
UnitsConverter converter = new UnitsConverter(source, target, conversionRates);
double maxDelta = 1e-6 * Math.abs(test.expected.doubleValue());
if (test.expected.doubleValue() == 0) {
maxDelta = 1e-12;
}
double inverseMaxDelta = 1e-6 * Math.abs(test.input.doubleValue());
if (test.input.doubleValue() == 0) {
inverseMaxDelta = 1e-12;
}
UnitsConverter converter = new UnitsConverter(source, target, conversionRates);
assertEquals("testConverter: " + test.sourceIdentifier + " to " + test.targetIdentifier,
test.expected.doubleValue(), converter.convert(test.input).doubleValue(),
maxDelta);
maxDelta = 1e-6 * Math.abs(test.input.doubleValue());
if (test.input.doubleValue() == 0) {
maxDelta = 1e-12;
}
assertEquals(
"testConverter inverse: " + test.targetIdentifier + " back to " + test.sourceIdentifier,
test.input.doubleValue(), converter.convertInverse(test.expected).doubleValue(),
maxDelta);
inverseMaxDelta);
// TODO: Test UnitsConverter created using CLDR separately.
// Test UnitsConverter created by CLDR unit identifiers
UnitsConverter converter2 = new UnitsConverter(test.sourceIdentifier, test.targetIdentifier);
maxDelta = 1e-6 * Math.abs(test.expected.doubleValue());
if (test.expected.doubleValue() == 0) {
maxDelta = 1e-12;
}
assertEquals("testConverter2: " + test.sourceIdentifier + " to " + test.targetIdentifier,
test.expected.doubleValue(), converter2.convert(test.input).doubleValue(),
maxDelta);
maxDelta = 1e-6 * Math.abs(test.input.doubleValue());
if (test.input.doubleValue() == 0) {
maxDelta = 1e-12;
}
assertEquals("testConverter2 inverse: " + test.targetIdentifier + " back to " + test.sourceIdentifier,
test.input.doubleValue(), converter2.convertInverse(test.expected).doubleValue(),
maxDelta);
inverseMaxDelta);
}
}

View file

@ -1683,7 +1683,7 @@ public class NumberFormatterApiTest extends TestFmwk {
// // TODO(ICU-21862): determine desired behaviour. Commented out for now
// // to not enforce undesirable behaviour
// assertFormatSingle("Fuel consumption: inverted units, divide-by-zero",
// assertFormatSingle("Fuel consumption: inverted units, divide-by-zero, en-US",
// "unit/liter-per-100-kilometer usage/vehicle-fuel",
// "unit/liter-per-100-kilometer usage/vehicle-fuel",
// NumberFormatter.with()
@ -1693,6 +1693,29 @@ public class NumberFormatterApiTest extends TestFmwk {
// 0, //
// "0 mpg");
// // TODO(ICU-21862): determine desired behaviour. Commented out for now
// // to not enforce undesirable behaviour
// assertFormatSingle("Fuel consumption: inverted units, divide-by-zero, en-ZA",
// "unit/mile-per-gallon usage/vehicle-fuel",
// "unit/mile-per-gallon usage/vehicle-fuel",
// NumberFormatter.with()
// .unit(MeasureUnit.forIdentifier("mile-per-gallon"))
// .usage("vehicle-fuel"),
// new ULocale("en-ZA"), //
// 0, //
// "0 mpg");
// // TODO(ICU-21862): Once we support Inf as input:
// assertFormatSingle("Fuel consumption: inverted units, divide-by-inf",
// "unit/mile-per-gallon usage/vehicle-fuel",
// "unit/mile-per-gallon usage/vehicle-fuel",
// NumberFormatter.with()
// .unit(MeasureUnit.forIdentifier("mile-per-gallon"))
// .usage("vehicle-fuel"),
// new ULocale("de-CH"), //
// INFINITY_GOES_HERE, //
// "0 mpg");
// Test calling .usage("") or .usage(null) should unset the existing usage.
// First: without usage
assertFormatSingle("Rounding Mode propagates: rounding up",