MadsKirkFoged / EngineeringUnits

Working with units made easy with automatic unit-check and converting between units
MIT License
41 stars 10 forks source link

Unexpected / inconsistent behavior of BaseUnit.GetValueAs() for mass units (potential bug) #33

Closed ChristopherRotter closed 8 months ago

ChristopherRotter commented 10 months ago

I'm working on a project that is using EngineeringUnits for internal representation of physical units. So far it has worked as expected, but I've come across an issue where the method

BaseUnit.GetValueAs(UnitSystem to)

returns an incorrect value when the base unit is using the same UnitSystem as the method argument. I managed to pin it down to some particular sequence of instructions - here is a minimal XUnit test to reporduce the issue:

[Fact]
public void Test()
{
    var unit = EngineeringUnits.Units.MassUnit.Gram.Unit;
    var valueWithUnit = new BaseUnit(1, unit);

    //var before = valueWithUnit.GetValueAs(unit);

    for (int i = 0; i < 2; i++)
    {
        var siUnit = unit.GetSIUnitsystem();
        var convertedValue = valueWithUnit.GetValueAs(siUnit);
        var value = new UnknownUnit(convertedValue, unit);
    }

    var after = valueWithUnit.GetValueAs(unit);
    Assert.Equal(1, after);
}

Expectated behavior: The result of GetValueAs has to be 1, because converting 1 gram to grams should yield 1 gram.

Actual behavior: The result of GetValueAs is 0.001

This test seems to work for any dimension other than mass, but I haven't verified it for every single one. Interestingly, if you execute the commented line before the for-loop, it works as expected again...

My suspicion is that it has to do with the fact that for mass units, the SI base unit is kilogram - that might explain why the returned value is off by a factor of 1000 for mass units, but not for other units. But the details are beyond my understanding, like for example I don't understand why this issue only materializes when the loop is executed more than once...

Some help on this issue and a bugfix (if this is indeed unexpected behavior) would be much appreciated.

MadsKirkFoged commented 10 months ago

Hi ChristopherRotter,

This is indeed unexpected behavior! Thanks for reporting it! It should be fixed in the newest build.

We use a lot of cache to speed up performance and it was inhere the bug was. Therefore it sometimes worked and sometimes didnt.