unitsofmeasurement / indriya

JSR 385 - Reference Implementation
Other
115 stars 40 forks source link

Overflow or underflow happening for simple addition? #421

Closed pascalriquier closed 3 weeks ago

pascalriquier commented 3 weeks ago

We want to do a simple addition of 2 quantities, which are in unit (kg/(ha.year.year)), see code example below. This is not giving us the correct answer, we assume due to the fact that the two numbers are converted to the system-unit, which will convert the year to seconds and then to the second power of that. Could this be handled better or could we do anything else on our side?

package be.vlaanderen.omgeving.pasberekening.domain.trend;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static tech.units.indriya.quantity.Quantities.getQuantity;

import javax.measure.Quantity;
import javax.measure.Unit;
import org.junit.jupiter.api.Test;
import si.uom.NonSI;
import tech.units.indriya.unit.Units;

class QuantitiesTest {
  public static interface DepositionChange extends Quantity<DepositionChange> {
  }

  public static final Unit<DepositionChange> KILOGRAM_PER_HECTARE_PER_YEAR_PER_YEAR =
      Units.KILOGRAM.divide(NonSI.HECTARE.multiply(NonSI.YEAR_CALENDAR.pow(2)))
          .asType(DepositionChange.class);

  @Test
  void test() {
    assertEquals(
        getQuantity(4, KILOGRAM_PER_HECTARE_PER_YEAR_PER_YEAR)
            .add(getQuantity(2.25, KILOGRAM_PER_HECTARE_PER_YEAR_PER_YEAR)),
        getQuantity(6.25, KILOGRAM_PER_HECTARE_PER_YEAR_PER_YEAR));
  }

}
andi-huber commented 3 weeks ago

Be aware, that comparing floating point numbers with equals is a bad idea in general.

Better to use something like assertEquals(number_a, number_b, 1E-6) with margin of tolerance to account for rounding errors.

pascalriquier commented 3 weeks ago

Excuse me, I should have been more precise about that, the actual value is

-5.342751485506088923587863698926158782093908691025920000 kg/(ha·day*365²)

It is not a rounding issue.

andi-huber commented 3 weeks ago

Agreed that's definitely not what we'd expect.

pascalriquier commented 3 weeks ago

@andi-huber for further clarity, we are using version 2.1.3 of indriya, do you want me to verify if the issue is also there in 2.2?

andi-huber commented 3 weeks ago

@pascalriquier yes that would be great thanks!

pascalriquier commented 3 weeks ago

OK @andi-huber I see that it is fixed in 2.2, we know what to do ;-)

andi-huber commented 3 weeks ago

No longer an issue in 2.2.