ICU-21349 Fix ICU4J reciprocal unit conversions

See #1565
This commit is contained in:
Hugo van der Merwe 2021-02-06 00:06:33 +00:00 committed by Peter Edberg
parent a138318922
commit 377dc22280
5 changed files with 76 additions and 41 deletions

View file

@ -198,7 +198,7 @@ void UnitsTest::testConverter() {
{"mebibyte", "byte", 1, 1048576},
{"gibibyte", "kibibyte", 1, 1048576},
{"pebibyte", "tebibyte", 4, 4096},
{"zebibyte", "pebibyte", 1.0/16, 65536.0},
{"zebibyte", "pebibyte", 1.0 / 16, 65536.0},
{"yobibyte", "exbibyte", 1, 1048576},
// Mass
{"gram", "kilogram", 1.0, 0.001},
@ -232,6 +232,9 @@ void UnitsTest::testConverter() {
{"square-yard", "square-foot", 0, 0},
{"square-yard", "square-foot", 0.000001, 0.000009},
{"square-mile", "square-foot", 0.0, 0.0},
// Fuel Consumption
{"cubic-meter-per-meter", "mile-per-gallon", 2.1383143939394E-6, 1.1},
{"cubic-meter-per-meter", "mile-per-gallon", 2.6134953703704E-6, 0.9},
};
for (const auto &testCase : testCases) {
@ -262,6 +265,14 @@ void UnitsTest::testConverter() {
}
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);
}
}

View file

@ -83,29 +83,13 @@ consumption; vehicle-fuel; BR; 11 / 10000000; 1.1E-6; cubic-meter-per-meter; 11
consumption; vehicle-fuel; BR; 1 / 1000000; 1.0E-6; cubic-meter-per-meter; 1; 1.0; liter-per-kilometer
consumption; vehicle-fuel; BR; 9 / 10000000; 9.0E-7; cubic-meter-per-meter; 9 / 10; 0.9; liter-per-kilometer
# TODO: these are local ICU-specific edits until the CLDR test file is updated:
# consumption-inverse; default; 001; 110000000; 1.1E8; meter-per-cubic-meter; 11 / 10; 1.1; kilometer-per-centiliter
consumption; default; 001; 110000000; 1.1E8; meter-per-cubic-meter; 10 / 11; 0.90909090909090906; liter-per-100-kilometer
# consumption-inverse; default; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; kilometer-per-centiliter
consumption; default; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; liter-per-100-kilometer
# consumption-inverse; default; 001; 90000000; 9.0E7; meter-per-cubic-meter; 9 / 10; 0.9; kilometer-per-centiliter
consumption; default; 001; 90000000; 9.0E7; meter-per-cubic-meter; 10 / 9; 1.1111111111111112; liter-per-100-kilometer
consumption; vehicle-fuel; US; 112903 / 52800000000; 2.1383143939394E-6; cubic-meter-per-meter; 11 / 10; 1.1; mile-per-gallon
consumption; vehicle-fuel; US; 112903 / 48000000000; 2.3521458333333E-6; cubic-meter-per-meter; 1; 1.0; mile-per-gallon
consumption; vehicle-fuel; US; 112903 / 43200000000; 2.6134953703704E-6; cubic-meter-per-meter; 9 / 10; 0.9; mile-per-gallon
# consumption-inverse; vehicle-fuel; 001; 110000000; 1.1E8; meter-per-cubic-meter; 11 / 10; 1.1; kilometer-per-centiliter
consumption; vehicle-fuel; 001; 110000000; 1.1E8; meter-per-cubic-meter; 10 / 11; 0.90909090909090906; liter-per-100-kilometer
# consumption-inverse; vehicle-fuel; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; kilometer-per-centiliter
consumption; vehicle-fuel; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; liter-per-100-kilometer
# consumption-inverse; vehicle-fuel; 001; 90000000; 9.0E7; meter-per-cubic-meter; 9 / 10; 0.9; kilometer-per-centiliter
consumption; vehicle-fuel; 001; 90000000; 9.0E7; meter-per-cubic-meter; 10 / 9; 1.1111111111111112; liter-per-100-kilometer
# TODO: these still pass, as expected/desired. Leaving them categorised as consumption-inverse, this is ignored by tests anyway
consumption-inverse; vehicle-fuel; US; 52800000000 / 112903; 467658.0781732992; meter-per-cubic-meter; 11 / 10; 1.1; mile-per-gallon
consumption-inverse; vehicle-fuel; US; 48000000000 / 112903; 425143.707430272; meter-per-cubic-meter; 1; 1.0; mile-per-gallon
consumption-inverse; vehicle-fuel; US; 43200000000 / 112903; 382629.3366872448; meter-per-cubic-meter; 9 / 10; 0.9; mile-per-gallon
consumption-inverse; vehicle-fuel; CA; 177027840000 / 454609; 389406.8089281118; meter-per-cubic-meter; 11 / 10; 1.1; mile-per-gallon-imperial
consumption-inverse; vehicle-fuel; CA; 160934400000 / 454609; 354006.1899346471; meter-per-cubic-meter; 1; 1.0; mile-per-gallon-imperial
consumption-inverse; vehicle-fuel; CA; 144840960000 / 454609; 318605.5709411824; meter-per-cubic-meter; 9 / 10; 0.9; mile-per-gallon-imperial
consumption; vehicle-fuel; CA; 454609 / 177027840000; 2.5680085121075E-6; cubic-meter-per-meter; 11 / 10; 1.1; mile-per-gallon-imperial
consumption; vehicle-fuel; CA; 454609 / 160934400000; 2.8248093633182E-6; cubic-meter-per-meter; 1; 1.0; mile-per-gallon-imperial
consumption; vehicle-fuel; CA; 454609 / 144840960000; 3.1386770703536E-6; cubic-meter-per-meter; 9 / 10; 0.9; mile-per-gallon-imperial
duration; default; 001; 95040; 95040.0; second; 11 / 10; 1.1; day
duration; default; 001; 86400; 86400.0; second; 1; 1.0; day

View file

@ -13,6 +13,7 @@ import com.ibm.icu.util.MeasureUnit;
public class UnitsConverter {
private BigDecimal conversionRate;
private boolean reciprocal;
private BigDecimal offset;
/**
@ -27,6 +28,7 @@ public class UnitsConverter {
*/
public UnitsConverter(MeasureUnitImpl source, MeasureUnitImpl target, ConversionRates conversionRates) {
Convertibility convertibility = extractConvertibility(source, target, conversionRates);
// TODO(icu-units#82): throw exception if conversion between incompatible types was requested?
assert (convertibility == Convertibility.CONVERTIBLE || convertibility == Convertibility.RECIPROCAL);
Factor sourceToBase = conversionRates.getFactorToBase(source);
@ -35,11 +37,15 @@ public class UnitsConverter {
if (convertibility == Convertibility.CONVERTIBLE) {
this.conversionRate = sourceToBase.divide(targetToBase).getConversionRate();
} else {
assert convertibility == Convertibility.RECIPROCAL;
this.conversionRate = sourceToBase.multiply(targetToBase).getConversionRate();
}
this.reciprocal = convertibility == Convertibility.RECIPROCAL;
// calculate the offset
this.offset = conversionRates.getOffset(source, target, sourceToBase, targetToBase, convertibility);
// We should see no offsets for reciprocal conversions - they don't make sense:
assert convertibility != Convertibility.RECIPROCAL || this.offset == BigDecimal.ZERO;
}
static public Convertibility extractConvertibility(MeasureUnitImpl source, MeasureUnitImpl target, ConversionRates conversionRates) {
@ -83,11 +89,36 @@ public class UnitsConverter {
}
public BigDecimal convert(BigDecimal inputValue) {
return inputValue.multiply(this.conversionRate).add(offset);
BigDecimal result = inputValue.multiply(this.conversionRate).add(offset);
if (this.reciprocal) {
// We should see no offsets for reciprocal conversions - they don't make sense:
assert offset == BigDecimal.ZERO;
if (result == BigDecimal.ZERO) {
// TODO: demonstrate the resulting behaviour in tests... and
// figure out desired behaviour. (Theoretical result should be
// infinity, not 0, but BigDecimal does not support infinity.)
return BigDecimal.ZERO;
}
result = BigDecimal.ONE.divide(result, DECIMAL128);
}
return result;
}
public BigDecimal convertInverse(BigDecimal inputValue) {
return inputValue.subtract(offset).divide(this.conversionRate, DECIMAL128);
BigDecimal result = inputValue;
if (this.reciprocal) {
// We should see no offsets for reciprocal conversions - they don't make sense:
assert offset == BigDecimal.ZERO;
if (result == BigDecimal.ZERO) {
// TODO: demonstrate the resulting behaviour in tests... and
// figure out desired behaviour. (Theoretical result should be
// infinity, not 0, but BigDecimal does not support infinity.)
return BigDecimal.ZERO;
}
result = BigDecimal.ONE.divide(result, DECIMAL128);
}
result = result.subtract(offset).divide(this.conversionRate, DECIMAL128);
return result;
}
public enum Convertibility {

View file

@ -83,21 +83,13 @@ consumption; vehicle-fuel; BR; 11 / 10000000; 1.1E-6; cubic-meter-per-meter; 11
consumption; vehicle-fuel; BR; 1 / 1000000; 1.0E-6; cubic-meter-per-meter; 1; 1.0; liter-per-kilometer
consumption; vehicle-fuel; BR; 9 / 10000000; 9.0E-7; cubic-meter-per-meter; 9 / 10; 0.9; liter-per-kilometer
consumption-inverse; default; 001; 110000000; 1.1E8; meter-per-cubic-meter; 11 / 10; 1.1; kilometer-per-centiliter
consumption-inverse; default; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; kilometer-per-centiliter
consumption-inverse; default; 001; 90000000; 9.0E7; meter-per-cubic-meter; 9 / 10; 0.9; kilometer-per-centiliter
consumption; vehicle-fuel; US; 112903 / 52800000000; 2.1383143939394E-6; cubic-meter-per-meter; 11 / 10; 1.1; mile-per-gallon
consumption; vehicle-fuel; US; 112903 / 48000000000; 2.3521458333333E-6; cubic-meter-per-meter; 1; 1.0; mile-per-gallon
consumption; vehicle-fuel; US; 112903 / 43200000000; 2.6134953703704E-6; cubic-meter-per-meter; 9 / 10; 0.9; mile-per-gallon
consumption-inverse; vehicle-fuel; 001; 110000000; 1.1E8; meter-per-cubic-meter; 11 / 10; 1.1; kilometer-per-centiliter
consumption-inverse; vehicle-fuel; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; kilometer-per-centiliter
consumption-inverse; vehicle-fuel; 001; 90000000; 9.0E7; meter-per-cubic-meter; 9 / 10; 0.9; kilometer-per-centiliter
consumption-inverse; vehicle-fuel; US; 52800000000 / 112903; 467658.0781732992; meter-per-cubic-meter; 11 / 10; 1.1; mile-per-gallon
consumption-inverse; vehicle-fuel; US; 48000000000 / 112903; 425143.707430272; meter-per-cubic-meter; 1; 1.0; mile-per-gallon
consumption-inverse; vehicle-fuel; US; 43200000000 / 112903; 382629.3366872448; meter-per-cubic-meter; 9 / 10; 0.9; mile-per-gallon
consumption-inverse; vehicle-fuel; CA; 177027840000 / 454609; 389406.8089281118; meter-per-cubic-meter; 11 / 10; 1.1; mile-per-gallon-imperial
consumption-inverse; vehicle-fuel; CA; 160934400000 / 454609; 354006.1899346471; meter-per-cubic-meter; 1; 1.0; mile-per-gallon-imperial
consumption-inverse; vehicle-fuel; CA; 144840960000 / 454609; 318605.5709411824; meter-per-cubic-meter; 9 / 10; 0.9; mile-per-gallon-imperial
consumption; vehicle-fuel; CA; 454609 / 177027840000; 2.5680085121075E-6; cubic-meter-per-meter; 11 / 10; 1.1; mile-per-gallon-imperial
consumption; vehicle-fuel; CA; 454609 / 160934400000; 2.8248093633182E-6; cubic-meter-per-meter; 1; 1.0; mile-per-gallon-imperial
consumption; vehicle-fuel; CA; 454609 / 144840960000; 3.1386770703536E-6; cubic-meter-per-meter; 9 / 10; 0.9; mile-per-gallon-imperial
duration; default; 001; 95040; 95040.0; second; 11 / 10; 1.1; day
duration; default; 001; 86400; 86400.0; second; 1; 1.0; day

View file

@ -336,11 +336,15 @@ public class UnitsTest {
new TestData("square-yard", "square-foot", 0, 0),
new TestData("square-yard", "square-foot", 0.000001, 0.000009),
new TestData("square-mile", "square-foot", 0.0, 0.0),
// Fuel Consumption
new TestData("cubic-meter-per-meter", "mile-per-gallon", 2.1383143939394E-6, 1.1),
new TestData("cubic-meter-per-meter", "mile-per-gallon", 2.6134953703704E-6, 0.9),
};
ConversionRates conversionRates = new ConversionRates();
for (TestData test : tests) {
UnitsConverter converter = new UnitsConverter(test.source, test.target, conversionRates);
double maxDelta = 1e-6 * Math.abs(test.expected.doubleValue());
if (test.expected.doubleValue() == 0) {
maxDelta = 1e-12;
@ -348,6 +352,14 @@ public class UnitsTest {
assertEquals("testConverter: " + test.source + " to " + test.target,
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.target + " back to " + test.source,
test.input.doubleValue(), converter.convertInverse(test.expected).doubleValue(),
maxDelta);
}
}
@ -489,8 +501,13 @@ public class UnitsTest {
}
public String toString() {
return "TestCase: " + category + ", " + usage + ", " + region +
"; Input: " + input + " " + inputUnit.first + "; Expected Values: " + expectedInOrder;
ArrayList<MeasureUnitImpl> outputUnits = new ArrayList<>();
for (Pair<String, MeasureUnitImpl> unit : outputUnitInOrder) {
outputUnits.add(unit.second);
}
return "TestCase: " + category + ", " + usage + ", " + region + "; Input: " + input +
" " + inputUnit.first + "; Expected Values: " + expectedInOrder +
", Expected Units: " + outputUnits;
}
}