unitsofmeasurement/uom-systems

Liters per 100 km to mpg conversion issue

eulogi opened this issue · 9 comments

The conversion from liters per 100 km to miles per gallon is not working as expected. You can execute this JUnit test to observe this issue:

package com.test;

import javax.measure.Unit;
import javax.measure.quantity.Volume;

import org.junit.Assert;
import org.junit.Test;

import systems.uom.quantity.Consumption;
import systems.uom.unicode.CLDR;

/**
 * Test unit conversions.
 */
public class UnitConversionTest {

    @Test
    @SuppressWarnings("unchecked")
    public void testLiterPer100KilometersToMilesPerGallon() {
        // Just comment that the only value that seems to be working fine is 1 liter per 100 km.
        double literPer100Kilometers = 10;
        Unit<Consumption<Volume>> MILE_PER_GALLON = CLDR.MILE_PER_GALLON.asType(Consumption.class);
        double milesPerGallonActual = CLDR.LITER_PER_100KILOMETERS.getConverterTo(MILE_PER_GALLON).convert(literPer100Kilometers);
        double milesPerGallonExpected = (100 * 3.785411784) / (1.609344 * literPer100Kilometers);
        Assert.assertEquals(milesPerGallonExpected, milesPerGallonActual, 0.001);
    }

    @Test
    @SuppressWarnings("unchecked")
    public void testLiterPer100KilometersToMilesPerGallonImperial() {
        double literPer100Kilometers = 10;
        Unit<Consumption<Volume>> MILE_PER_GALLON_IMPERIAL = CLDR.MILE.divide(CLDR.GALLON_IMPERIAL).asType(Consumption.class);
        double milesPerGallonImperialActual = CLDR.LITER_PER_100KILOMETERS.getConverterTo(MILE_PER_GALLON_IMPERIAL).convert(literPer100Kilometers);
        double milesPerGallonImperialExpected = 282.480936332 / literPer100Kilometers;
        Assert.assertEquals(milesPerGallonImperialExpected, milesPerGallonImperialActual, 0.001);
    }
}

Related to unitsofmeasurement/uom-demos#97

keilw commented

@andi-huber This test is there but disabled as FuelConsumptionTest I think this is related to the CO2CarDemo. Doing this with double may not be optimal but it should nevertheless work, and right now here it shows the same 100 times too big symtom we saw elsewhere.

Was checking out uom-systems (master), but cannot compile ...

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /~/uom-systems/unicode/src/test/java/systems/uom/unicode/FuelConsumptionTest.java:[62,113] incompatible types: inferred type does not conform to equality constraint(s)
    inferred: systems.uom.quantity.Consumption<javax.measure.quantity.Volume>
    equality constraints(s): systems.uom.quantity.Consumption<javax.measure.quantity.Volume>,systems.uom.quantity.Consumption
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Units of Measurement Systems Parent 2.1-SNAPSHOT:
[INFO] 
[INFO] Units of Measurement Systems Parent ................ SUCCESS [  2.087 s]
[INFO] Units of Measurement Systems Quantities ............ SUCCESS [  3.302 s]
[INFO] Units of Measurement Common Unit Systems ........... SUCCESS [  9.379 s]
[INFO] Units of Measurement Unicode CLDR System ........... FAILURE [  0.223 s]
[INFO] Units of Measurement UCUM System ................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
keilw commented

Sounds weird, never saw that, which JDK are you using?

a bit outdated ...

mvn -version
Apache Maven 3.6.3
Java version: 1.8.0_192, vendor: Oracle Corporation, runtime: D:\opt\jdk8\jre
Default locale: en_US, platform encoding: UTF-8
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"

Which JDK major version do I need?

keilw commented

Try at least 9. The library should run with Java 8 based on the demos, but building requires Java 9 for most repositories. The Travis matrix uses 9, and 12-16, they all build correctly.

In CLDR those 2 definitions are wrong!

    /**Constant for unit of consumption: liter-per-100kilometers*/
    public static final Unit<Consumption<Volume>> LITER_PER_100KILOMETERS = addUnit(
            (KILOMETER.multiply(100)).divide(LITER).asType(Consumption.class));;

    /**Constant for unit of consumption: liter-per-kilometer*/
    public static final Unit<Consumption<Volume>> LITER_PER_KILOMETER = addUnit(
           KILOMETER.divide(LITER).asType(Consumption.class));

The divisions need to be flipped! And then also note: LITER_PER_XXX and MILE_PER_GALLON are not commensurable, so these cannot share the same generic unit type Unit<Consumption<Volume>>. However you like to define Consumption, but one unit has to be the reciprocal of the other. Meaning MILE_PER_GALLON is likely to be considered a 'reciprocal consumption'.

    public static final Unit<Consumption<Volume>> MILE_PER_GALLON = addUnit(
            MILE.divide(GALLON).asType(Consumption.class));

Hope that helps.

keilw commented

Thanks, do you think we could get to a PR for that? And what about the discussion in unitsofmeasurement/uom-demos#97? If <Consumption<Volume>> is fine here, then the CO2CarDemo and energy domain library should also take that into consideration because it makes no sense to handle this calculation differrently in the CLDR module and elsewhere.

keilw commented

@andi-huber Any luck building this? I changed the division in two cases and found a good Wikipedia reference: https://en.wikipedia.org/wiki/Fuel_efficiency

So according to that:
LITER_PER_100KILOMETERS is Fuel consumption, I would leave that with the current quantity type Consumption, whereas
LITER_PER_KILOMETER or MILE_PER_GALLON are not defined precisely enough by Unicode CLDR/ICU4J, they are called Fuel economy in above Wikipedia page.

There are terms like Energy efficiency and Fuel Efficiency which brings us back to the original page, so maybe call the Quantity type FuelEfficiency or define a Meta-Quantity like Consumption called Efficiency allowing to define LITER_PER_KILOMETER or MILE_PER_GALLON as. <Efficiency<Volume>>

keilw commented

There does not seem to be much activity here, so moving it to a later milestone.