unitsofmeasurement / uom-systems

Units of Measurement Systems
http://www.uom.systems
Other
36 stars 17 forks source link

Liters per 100 km to mpg conversion issue #177

Open eulogi opened 4 years ago

eulogi commented 4 years ago

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 https://github.com/unitsofmeasurement/uom-demos/issues/97

keilw commented 3 years ago

@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.

andi-huber commented 3 years ago

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 3 years ago

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

andi-huber commented 3 years ago

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 3 years ago

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.

andi-huber commented 3 years ago

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 3 years ago

Thanks, do you think we could get to a PR for that? And what about the discussion in https://github.com/unitsofmeasurement/uom-demos/issues/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 3 years ago

@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 3 years ago

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