Conversions for temp (degress C<->F<->K) wrong

[GregRR]

The code:

"<kelvin>" : [["degK","kelvin"], 1.0, "temperature", ["<kelvin>"]],
 "<celsius>" : [["degC","celsius","celsius","centigrade"], 1.0, "temperature", ["<kelvin>"]],
 "<fahrenheit>" : [["degF","fahrenheit"], 1/1.8, "temperature", ["<kelvin>"]],

TempC = TempK - 273.15

E.g., 20 degC = 293.15 degK. Currently js-quantities converts 20degC to 20degK.

Same issue with degC to degF.... js-quantities converts 20degC to 36degF ..... it should be 68degF.

From firebug console:

qty1 = new Qty("20degC"); 20 degC { scalar=20, base_scalar=20, unit_name="degC", more...} qty1.to("degK"); 20 degK { scalar=20, base_scalar=20, unit_name="degK", more...} qty2 = new Qty("20degC"); 20 degC { scalar=20, base_scalar=20, unit_name="degC", more...} qty2.to("degF"); 36 degF { scalar=36, base_scalar=20, unit_name="degF", more...}

[rage-shadowman]

I fixed this in my fork and sent a pull request.

[rage-shadowman]

I have to wonder why the code originally included degK and tempK as separate units with separate bases...

Since they were identical, I made them interchangeable, but was I wrong to do so?

Is one of them supposed to be measured as a kelvin-relative vector (1K == 1C == 5/9F), and the other as a kelvin-relative temperature (32F == 0C == 273.15K)?

My fix makes a check to see if the units are to be used in a ratio ("1mm/degC" or "1degC/min") to be calculated as a vector without base offset, or as a number of units ("1degC") to be calculated as a temperature with base offset. Should I have made it simpler by always calculating one with and the other without?

[gentooboontoo]

The code is based on ruby-units unit definitions. Temperatures (tempX) are a very special case because there are relative and not absolute as other units (including degX).

Currently, js-quantities does not handle relative temperatures because they had no use for me.

If implemented, the code should handle them as below:

1degK == 1degC == 5/9degF // Differences 32tempF == 0tempC == 273.15tempK

Moreover,

• operations between quantities should handle automatic conversions tempX - tempX => degX tempX + degX => tempX
• operation other than difference between 2 tempX are forbidden: tempX (+|*|/) tempX => error

[rage-shadowman]

This is more complicated than I expected. My code works for simple cases, but not complicated math. I'd like to treat them both as vector and scalar, where rhs is always vector and if the lhs is ever multiplied or divided by a non-unitless qty it is first turned into vector.

But that does not allow things like: 2tempC - 2tempF == 18.667tempK

To me the above seems like wrong behavior anyway, but maybe that is why they separated them, so that you can get what seems correct to me by 2tempC - 2degF, and people who expect the other behavior can still get it by doing the above.

[rage-shadowman]

I added some unit tests for more complicated math and made changes to the code so that they pass.

They do not follow exactly what you put up above (ie: "degX" is still synonymous with "tempX"). All math operations are allowed, but I think they are done in a sane way with unit tests to prove that they do what is expected. I start by not converting temperatures to base units in math operations.

To start out, I'll mention the possible issue in case it is a deal breaker (this probably deserves a short blurb in the readme also): ...since: "2degC/s".to("degF/s") == "(18/5) degF/s" (converted as vector, as in this case of exaggerated global warming) ...then: "2degC/s".to("degF/s").mul("s") == "(18/5) degF" (which could be converted to degC as a temperature that is not 2)

...Now, the general behavior as I intended it (and believe I achieved)...

There are some rounding issues, but there always were and these values are very close (hence the unit tests using toBeCloseTo rather than toBe):

"0degC".to("degF") == "32degF" ("to" function converts temperature values, not degrees) "0degC".add("2degC") == "2 degC" "0degC".add("2degF") == "2_5/9 degC" "0degC".sub("2degF") == "-2_5/9 degC"

"2degC".mul("2degF") == "4 degC*degF" (you must then divide by degF or degC to get back to a sane unit) "2degC/degF".mul("2degF") == "4 degC"

"4degC*degF".div("2degF") == "2 degC" "4degC".div("degF") == "4degC/degF" "4degC".div("2degC") == "2" (unitless quantity)

If this looks sane to you, or close to sane, feel free to add any unit tests (broken or passing) that make sense and I will see if I can make any broken ones work.

If this does not look sane to you, and you can think of unit tests that do, then please still feel free to push them (as long as they cover deg/temp conversion and math, throwing errors or returning results) and I'll see what I can do about them.

[gentooboontoo]

That looks fine. I can't check right now because I'm on vacation without any computer (I'm answering with my smartphone) but I'll try as soon as possible. Le 3 juil. 2013 01:37, "Shadow Man" notifications@github.com a écrit :

I added some unit tests for more complicated math and made changes to the code so that they pass.

They do not follow exactly what you put up above (ie: "degX" is still synonymous with "tempX"). All math operations are allowed, but I think they are done in a sane way with unit tests to prove that they do what is expected.

To start out, I'll mention the possible issue in case it is a deal breaker (this probably deserves a short blurb in the readme also): ...since: "2degC/s".to("degF/s") == "(18/5) degF/s" (converted as vector, as in this case of exaggerated global warming) ...then: "2degC/s".to("degF/s").mul("s") == "(18/5) degF" (which could be converted to degC as a temperature that is not 2)

...Now, the general behavior as I intended it (and believe I achieved)...

There are some rounding issues, but there always were and these values are very close (hence the unit tests using toBeCloseTo rather than toBe):

"0degC".to("degF") == "32degF" ("to" function converts temperature values, not degrees) "0degC".add("2degC") == "2 degC" "0degC".add("2degF") == "2_5/9 degC" "0degC".sub("2degF") == "-2_5/9 degC"

"2degC".mul("2degF") == "4 degC*degF" (you must then divide by degF or degC to get back to a sane unit) "2degC/degF".mul("2degF") == "1 degC"

"4degC*degF".div("2degF") == "2 degC" "4degC".div("degF") == "4degC/degF" "4degC".div("2degC") == "2" (unitless quantity)

If this looks sane to you, or close to sane, feel free to add any unit tests (broken or passing) that make sense and I will see if I can make any broken ones work.

If this does not look sane to you, and you can think of unit tests that do, then please still feel free to push them (as long as they cover deg/temp conversion and math, throwing errors or returning results) and I'll see what I can do about them.

— Reply to this email directly or view it on GitHubhttps://github.com/gentooboontoo/js-quantities/issues/1#issuecomment-20386572 .

[rage-shadowman]

...in case you don't like my changes...

FYI - I've looked at the ruby code at https://github.com/olbrich/ruby-units and apparently they treat temp and deg the same as far as being without base offset, but they do specific checks in the add/sub/div/mul/to methods for temp in the signature. The math methods throw exceptions if a temp is on the wrong side of the equation and the to method handles the base offset when converting to other temps.

That could be implemented in your javascript code in those (and related) methods without adding the base offset field to UNIT definitions. Although there will still need to be cleanup to keep temp/deg objects from ever being converted toBase (for instance inside the mul/div functions, which are legal for deg units).

If you care to look for yourself, see lib/ruby_units/unit.rb definitions for usage of 'is_temperature?'.

[gentooboontoo]

js-quantities was based on a (very) older version of ruby-units whose temperatures implementation was incomplete as far I remember. But I had no use of them, that's why I didn't fix it in the past.

I will look the current implementation in ruby-units. Le 11 juil. 2013 21:19, "Shadow Man" notifications@github.com a écrit :

FYI - I've looked at the ruby code at https://github.com/olbrich/ruby-units and apparently they treat temp and deg the same as far as being without base offset, but they do specific checks in the add/sub/div/mul/to methods for temp in the signature. The math methods throw exceptions if a temp is on the wrong side of the equation and the to method handles the base offset when converting to other temps.

That could be implemented in your javascript code in those (and related) methods without adding the base offset field to UNIT definitions. Although there will still need to be cleanup to keep temp/deg objects from ever being converted toBase (for instance inside the mul/div functions, which are legal for deg units).

If you care to look for yourself, see lib/ruby_units/unit.rb definitions for usage of 'is_temperature?'.

— Reply to this email directly or view it on GitHubhttps://github.com/gentooboontoo/js-quantities/issues/1#issuecomment-20835069 .

[rage-shadowman]

Just to see what it would look like (and because I don't like the look of my toBaseUnits and fromBaseScalar functions), I made a branch on my repo for a ruby-units variant.

The unit tests were also modified to correspond with what I believe is the ruby-units behavior (I've never run anything ruby, but I think I understand what the code is saying).

It should be finished, although it is not merged into master as I'm not certain which one is better.

[rage-shadowman]

Ok, I've merged the ruby-units variant into master. Since this is supposed to be a port of ruby-units, that behavior is what should be used.

[gentooboontoo]

Closing now since @rage-shadowman's pull request #2 is merged.