Conversion bugs - Githubissues

unitsofmeasurement / uom-systems

Units of Measurement Systems

http://www.uom.systems

Other

36 stars 17 forks source link

Conversion bugs #103

Closed frank-montyne closed 7 years ago

frank-montyne commented 7 years ago

When I use UCUM and do a conversion from KELVIN to FAHRENHEIT I get a wrong result: For example 273.15 KELVIN converts into -307.920 FAHRENHEIT.

Looking into the getConverterTo function I saw the following:

On the source unit we go from KELVIN to KELVIN so no converter
On the target unit the FAHRENHEIT unit has a converter from KELVIN to FAHRENHEIT. That converter is returned but then inverted!
That converter then gets concatenated with the source unit converter (that does nothing)
the result is the wrong conversion.

Can you have a look please? Thanks, Frank Montyne

keilw commented 7 years ago

According to UCUM these are "special units", which can have an impact on cnversion to other temperature units.

frank-montyne commented 7 years ago

I would assume a simple kelvin to fahrenheit conversion should always be correct. What's the point of the library otherwise? Perhaps I don't really understand what you are saying, can you elaborate a bit please? Thanks, Frank

On Thu, May 11, 2017 at 12:43 PM, Werner Keil notifications@github.com wrote:

According to UCUM these are "special units", which can have an impact on cnversion to other temperature units.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/unitsofmeasurement/uom-systems/issues/103#issuecomment-300752641, or mute the thread https://github.com/notifications/unsubscribe-auth/AJKoL7YU26wk_UHntqbwY34sTUs82pfTks5r4uZogaJpZM4NW1e2 .

keilw commented 7 years ago

See http://unitsofmeasure.org/ucum.html#section-Special-Units-on-non-ratio-Scales in the UCUM specification. Full UCUM implementations that pass its tests like UOMo UCUM cover these under https://github.com/eclipse/uomo/tree/master/bundles/org.eclipse.uomo.ucum/src/main/java/org/eclipse/uomo/ucum/special. The operation seems to combine two or more steps in the UCUM class, so could you please try to reproduce the problem in https://github.com/unitsofmeasurement/uom-systems/blob/master/common/src/main/java/systems/uom/common/USCustomary.java?

Thanks, Werner

keilw commented 7 years ago

It works with USCustomary, see https://github.com/unitsofmeasurement/uom-systems/blob/master/common-java8/src/test/java/systems/uom/common/ConverterTest.java

keilw commented 7 years ago

Fixed, REAUMUR had a similar issue, but both are addressed, see https://github.com/unitsofmeasurement/uom-systems/blob/master/ucum-java8/src/test/java/systems/uom/ucum/ConverterTest.java Thanks for raising the issue.

frank-montyne commented 7 years ago

Thanks!

On Sat, May 13, 2017 at 8:28 PM, Werner Keil notifications@github.com wrote:

Should be fixed, there was a similar issue with REAUMUR, but both are addressed, see https://github.com/unitsofmeasurement/uom- systems/blob/master/ucum-java8/src/test/java/systems/ uom/ucum/ConverterTest.java Thanks for raising the issue.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/unitsofmeasurement/uom-systems/issues/103#issuecomment-301266050, or mute the thread https://github.com/notifications/unsubscribe-auth/AJKoL_3NGmsQRahT2m3J0-zDFrEBl2VKks5r5fZpgaJpZM4NW1e2 .