Adjust QuantityType calculations for temperatures #3792

mherwege · 2023-08-31T16:28:25Z

Signed-off-by: Mark Herwege mark.herwege@telenet.be

EDIT 05/09/23: Summary updated with analysis, conclusions and proposed behaviour.

In tests performed for a Blockly issue, it was identified that calculations with temperatures in °F produce strange results: openhab/openhab-webui#2001 (comment)

Notably:

65 °F / Qty(1) produces 36.11 K: reason is 65° F is treated as relative temperature and the result is relative as well. Δ 1 K = Δ 0.55 °F. Converting back the result to °F therefore is not equal to 65 °F, which would be the typical expectation fo a user. Note that 65 °F / 1 does produce 65 °F, so there is a difference in behaviour between scalar and dimensionless quantity.
65 °F / 1 °F produces 65: seems intuitively correct, but does not make sense, again shows division is with relative values, ignoring the offset.
65 °F * 1 °F produces 20.0617 K²: this is the result of Δ 65 °F * Δ 1 °F = 65 * 0.55 K * 1 * 0.55 K.

The issue is that all quantities in OH are considered relative. This is fine for most dimensions and units that have the same zero value, but leads to issues with temperatures in general and with °F in particular (°C is easier to interpret as Δ 1° C = Δ 1 K).

This PR changes the behaviour by converting the quantities to absolute before doing the calculations, i.e.

for addition or subtraction, convert the first argument to absolute, second argument remains relative. This effectively makes the second argument an offset.
for multiplication or division, convert both arguments to absolute. Multiplying or dividing relative values does not make sense in most cases.

Note that this has been discussed before: #2386 and eclipse-archived/smarthome#5697. It has lead to creating special treatment for temperatures in the SystemOffsetProfile, but no adjustment for rule languages in general. The proposed fix eliminates the need for a specific fix in the SystemOffsetProfile and brings the behaviour in rules in line with the offset profile behaviour.

With the proposed changes:

No change to behaviour for units with same 0 base (as far as I know, the only practical exception in the smarthome context is with temperatures).
Adding/substracting temperatures will be commutative: A + B = B + A
Adding/substracting temperatures will be associative: (A + B) + C = A + (B + C)
Mixing compatible units in adding/substracting yields expected results and respects commutativity, associativity (was not the case before)
Converting units on the result of addition/substraction yieds the same as converting units on the argument(s) before addition/substraction (was not the case before)
Multiplication/division always works with absolute values from the absolute 0, i.e. uses Kelvin for temperatures (was not the case before)
Multiplying/dividing with scalar or dimensionless quantities gives the same result (was not the case before)
Multiplying/dividing with 1 or Qty(1) gives the same as the input (was not the case before for Qty(1))
Converting units on the result of multiplication/division yieds the same as converting units on the argument(s) before multiplication/division (was not the case before)
Distributivity of addition/substraction would not be respected: (A + B) * factor is not equal (A * factor) + (B * factor), e.g. (1 °C + 2 °C) * 2 = 279.15 °C while 1 °C * 2 + 2 °C * 2 = 552.3 °C. This would have been respected before when working with scalars, and yield 6°C (no conversion would take place, arguments and result are relative). It would not have been respected with dimensionless quantities or when mixing units anyway, so distributivity would only have been respected in very specific cases. I also think the difference is easy enough to understand, easier than the issues seen before. If one would want to force distributivity, both arguments in addition/substraction should always be converted to absolute, but in most cases adding temperatures would be expected to be relative in its second argument (or you would have to force users to work in Kelvin only for addition). I therefore think breaking distributivity is not a major concern. Note that distributivity is not broken for all common units except temperatures. Also for temperatures, it is respected when working with Kelvin.

I did tests with this in a scratchpad using DSL. I believe this gives much more predictable and expected results than before. These tests are now also part of the Java unit tests.

Here is the DSL script used for the test:

val temperature = 65|°F
val one = java.math.BigDecimal.valueOf(1)
val two = java.math.BigDecimal.valueOf(2)
val temperature1 = 1|°F
val temperature2 = 2|°F
val energykWh = 65|kWh
val energykJ = 65|kJ
val energykWh1 = 1|kWh
val energykJ1 = 1|kJ

var QuantityType result

result =  temperature.multiply(one)
logInfo("Quantity test", "Qty({}) * 1 = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature.divide(one)
logInfo("Quantity test", "Qty({}) / 1 = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)

result =  temperature.multiply(two)
logInfo("Quantity test", "Qty({}) * 2 = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature.divide(two)
logInfo("Quantity test", "Qty({}) / 2 = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)

result =  temperature.multiply(QuantityType.valueOf(1, ONE))
logInfo("Quantity test", "Qty({}) * Qty(1) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature.divide(QuantityType.valueOf(1, ONE))
logInfo("Quantity test", "Qty({}) / Qty(1) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)

result =  temperature.multiply(QuantityType.valueOf(2, ONE))
logInfo("Quantity test", "Qty({}) * Qty(2) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature.divide(QuantityType.valueOf(2, ONE))
logInfo("Quantity test", "Qty({}) / Qty(2) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)

result =  result =  QuantityType.valueOf(1, ONE).multiply(temperature)
logInfo("Quantity test", "Qty(1) * Qty({}) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  result =  QuantityType.valueOf(1, ONE).divide(temperature)
logInfo("Quantity test", "Qty(1) / Qty({}) = {}", temperature.toString, result.toString)
  
result =  result =  QuantityType.valueOf(2, ONE).multiply(temperature)
logInfo("Quantity test", "Qty(2) * Qty({}) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  result =  QuantityType.valueOf(2, ONE).divide(temperature)
logInfo("Quantity test", "Qty(2) / Qty({}) = {}", temperature.toString, result.toString)

result =  temperature.add(temperature1)
logInfo("Quantity test", "Qty({}) + Qty(1 °F) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature.subtract(temperature1)
logInfo("Quantity test", "Qty({}) - Qty(1 °F) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature.multiply(temperature1)
logInfo("Quantity test", "Qty({}) * Qty(1 °F) = {}", temperature.toString, result.toString)
result =  temperature.divide(temperature1)
logInfo("Quantity test", "Qty({}) / Qty(1 °F) = {}", temperature.toString, result.toString)

result =  temperature.add(temperature2)
logInfo("Quantity test", "Qty({}) + Qty(2 °F) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature.subtract(temperature2)
logInfo("Quantity test", "Qty({}) - Qty(2 °F) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature.multiply(temperature2)
logInfo("Quantity test", "Qty({}) * Qty(2 °F) = {}", temperature.toString, result.toString)
result =  temperature.divide(temperature2)
logInfo("Quantity test", "Qty({}) / Qty(2 °F) = {}", temperature.toString, result.toString)

result =  temperature1.add(temperature)
logInfo("Quantity test", "Qty(1 °F) + Qty({}) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature1.subtract(temperature)
logInfo("Quantity test", "Qty(1 °F) - Qty({}) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature1.multiply(temperature)
logInfo("Quantity test", "Qty(1 °F) * Qty({}) = {}", temperature.toString, result.toString)
result =  temperature1.divide(temperature)
logInfo("Quantity test", "Qty(1 °F) / Qty({}) = {}", temperature.toString, result.toString)

result =  temperature2.add(temperature)
logInfo("Quantity test", "Qty(2 °F) + Qty({}) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature2.subtract(temperature)
logInfo("Quantity test", "Qty(2 °F) - Qty({}) = {} = {}", temperature.toString, result.toString, result.toUnit("°F").toString)
result =  temperature2.multiply(temperature)
logInfo("Quantity test", "Qty(2 °F) * Qty({}) = {}", temperature.toString, result.toString)
result =  temperature2.divide(temperature)
logInfo("Quantity test", "Qty(2 °F) / Qty({}) = {}", temperature.toString, result.toString)

result = energykWh.multiply(QuantityType.valueOf(1, ONE))
logInfo("Quantity test", "Qty({}) * Qty(1) = {} = {}", energykWh.toString, result.toString, result.toUnit("kWh").toString)
result = energykWh.divide(QuantityType.valueOf(1, ONE))
logInfo("Quantity test", "Qty({}) / Qty(1) = {} = {}", energykWh.toString, result.toString, result.toUnit("kWh").toString)

result =  energykWh.add(energykWh1)
logInfo("Quantity test", "Qty({}) + Qty(1 kWh) = {} = {}", energykWh.toString, result.toString, result.toUnit("kWh").toString)
result =  energykWh.subtract(energykWh1)
logInfo("Quantity test", "Qty({}) - Qty(1 kWh) = {} = {}", energykWh.toString, result.toString, result.toUnit("kWh").toString)
result =  energykWh.multiply(energykWh1)
logInfo("Quantity test", "Qty({}) * Qty(1 kWh) = {}", energykWh.toString, result.toString)
result =  energykWh.divide(energykWh1)
logInfo("Quantity test", "Qty({}) / Qty(1 kWh) = {}", energykWh.toString, result.toString)

result =  energykJ.add(energykJ1)
logInfo("Quantity test", "Qty({}) + Qty(1 kJ) = {} = {}", energykJ.toString, result.toString, result.toUnit("kJ").toString)
result =  energykJ.subtract(energykJ1)
logInfo("Quantity test", "Qty({}) - Qty(1 kJ) = {} = {}", energykJ.toString, result.toString, result.toUnit("kJ").toString)
result =  energykJ.multiply(energykJ1)
logInfo("Quantity test", "Qty({}) * Qty(1 kJ) = {}", energykJ.toString, result.toString)
result =  energykJ.divide(energykJ1)
logInfo("Quantity test", "Qty({}) / Qty(1 kJ) = {}", energykJ.toString, result.toString)

result =  energykWh.add(energykJ1)
logInfo("Quantity test", "Qty({}) + Qty(1 kJ) = {} = {} = {}", energykWh.toString, result.toString, result.toUnit("kWh").toString, result.toUnit("kJ").toString)
result =  energykWh.subtract(energykJ1)
logInfo("Quantity test", "Qty({}) - Qty(1 kJ) = {} = {} = {}", energykWh.toString, result.toString, result.toUnit("kWh").toString, result.toUnit("kJ").toString)
result =  energykWh.multiply(energykJ1)
logInfo("Quantity test", "Qty({}) * Qty(1 kJ) = {}", energykWh.toString, result.toString)
result =  energykWh.divide(energykJ1)
logInfo("Quantity test", "Qty({}) / Qty(1 kJ) = {} = {}", energykWh.toString, result.toString, result.toUnit(ONE).toString)

And the result:

18:20:49.342 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) * 1 = 65.00000000000000000000000000000004 °F = 65.00000000000000000000000000000004 °F
18:20:49.344 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) / 1 = 65.00000000000000000000000000000004 °F = 65.00000000000000000000000000000004 °F
18:20:49.347 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) * 2 = 589.6700000000000000000000000000001 °F = 589.6700000000000000000000000000001 °F
18:20:49.358 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) / 2 = -197.3350000000000000000000000000000 °F = -197.3350000000000000000000000000000 °F
18:20:49.361 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) * Qty(1) = 291.483333333333333333333333333333356652 K = 65.00000000000000000000000000000004 °F
18:20:49.365 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) / Qty(1) = 291.483333333333333333333333333333356652 K = 65.00000000000000000000000000000004 °F
18:20:49.370 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) * Qty(2) = 582.966666666666666666666666666666713304 K = 589.6700000000000000000000000000001 °F
18:20:49.373 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) / Qty(2) = 145.7416666666666666666666666666666783260 K = -197.3350000000000000000000000000000 °F
18:20:49.378 [INFO ] [nhab.core.model.script.Quantity test] - Qty(1) * Qty(65 °F) = 291.483333333333333333333333333333356652 K = 65.00000000000000000000000000000004 °F
18:20:49.380 [INFO ] [nhab.core.model.script.Quantity test] - Qty(1) / Qty(65 °F) = 0.003430727886099834181485505174681228 1/K
18:20:49.385 [INFO ] [nhab.core.model.script.Quantity test] - Qty(2) * Qty(65 °F) = 582.966666666666666666666666666666713304 K = 589.6700000000000000000000000000001 °F
18:20:49.386 [INFO ] [nhab.core.model.script.Quantity test] - Qty(2) / Qty(65 °F) = 0.006861455772199668362971010349362456 1/K
18:20:49.390 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) + Qty(1 °F) = 66.00000000000000000000000000000004 °F = 66.00000000000000000000000000000004 °F
18:20:49.393 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) - Qty(1 °F) = 64.00000000000000000000000000000004 °F = 64.00000000000000000000000000000004 °F
18:20:49.397 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) * Qty(1 °F) = 74598.68175925925925925925925925927 K²
18:20:49.400 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) / Qty(1 °F) = 1.138928083009529598193934920876115122114246640762367855514793670089202480
18:20:49.403 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) + Qty(2 °F) = 67.00000000000000000000000000000004 °F = 67.00000000000000000000000000000004 °F
18:20:49.406 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) - Qty(2 °F) = 63.00000000000000000000000000000004 °F = 63.00000000000000000000000000000004 °F
18:20:49.409 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) * Qty(2 °F) = 74760.61694444444444444444444444446 K²
18:20:49.411 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 °F) / Qty(2 °F) = 1.136461108584053544739749171486126553533555353390950245846600385556783676
18:20:49.413 [INFO ] [nhab.core.model.script.Quantity test] - Qty(1 °F) + Qty(65 °F) = 66.00000000000000000000000000000003 °F = 66.00000000000000000000000000000003 °F
18:20:49.415 [INFO ] [nhab.core.model.script.Quantity test] - Qty(1 °F) - Qty(65 °F) = -63.99999999999999999999999999999996 °F = -63.99999999999999999999999999999996 °F
18:20:49.420 [INFO ] [nhab.core.model.script.Quantity test] - Qty(1 °F) * Qty(65 °F) = 74598.68175925925925925925925925927 K²
18:20:49.422 [INFO ] [nhab.core.model.script.Quantity test] - Qty(1 °F) / Qty(65 °F) = 0.878018564049783673547182038233556349552596235093804994885674169795613456
18:20:49.425 [INFO ] [nhab.core.model.script.Quantity test] - Qty(2 °F) + Qty(65 °F) = 67.00000000000000000000000000000003 °F = 67.00000000000000000000000000000003 °F
18:20:49.427 [INFO ] [nhab.core.model.script.Quantity test] - Qty(2 °F) - Qty(65 °F) = -62.99999999999999999999999999999996 °F = -62.99999999999999999999999999999996 °F
18:20:49.430 [INFO ] [nhab.core.model.script.Quantity test] - Qty(2 °F) * Qty(65 °F) = 74760.61694444444444444444444444446 K²
18:20:49.434 [INFO ] [nhab.core.model.script.Quantity test] - Qty(2 °F) / Qty(65 °F) = 0.879924523986505803648007318886157031927295252253797625173918844225890256
18:20:49.435 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) * Qty(1) = 65 kWh = 65 kWh
18:20:49.436 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) / Qty(1) = 65 kWh = 65 kWh
18:20:49.437 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) + Qty(1 kWh) = 66 kWh = 66 kWh
18:20:49.437 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) - Qty(1 kWh) = 64 kWh = 64 kWh
18:20:49.438 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) * Qty(1 kWh) = 65 kWh²
18:20:49.438 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) / Qty(1 kWh) = 65
18:20:49.438 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kJ) + Qty(1 kJ) = 66 kJ = 66 kJ
18:20:49.439 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kJ) - Qty(1 kJ) = 64 kJ = 64 kJ
18:20:49.439 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kJ) * Qty(1 kJ) = 65 kJ²
18:20:49.440 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kJ) / Qty(1 kJ) = 65
18:20:49.441 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) + Qty(1 kJ) = 65.00027777777777777777777777777778 kWh = 65.00027777777777777777777777777778 kWh = 234001 kJ
18:20:49.467 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) - Qty(1 kJ) = 64.99972222222222222222222222222222 kWh = 64.99972222222222222222222222222222 kWh = 233999 kJ
18:20:49.468 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) * Qty(1 kJ) = 65 kWh·kJ
18:20:49.472 [INFO ] [nhab.core.model.script.Quantity test] - Qty(65 kWh) / Qty(1 kJ) = 65 kWh/kJ = 234000

~~If this is an acceptable approach, I can convert the test DSL script to unit test.~~

~~Open question for me still: should the result of calculations be converted to relative again. I would be happy to hear views on this.~~

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

J-N-K · 2023-08-31T21:05:04Z

I remember we discussed that in the past, maybe even in ESH times and I think something was changed back then. It would be good to find that discussion and see why it is the way it is now (or we broke it again because of insufficient tests).

mherwege · 2023-09-01T06:42:33Z

@J-N-K I think the discussion about it is #2386 and eclipse-archived/smarthome#5697.

It still leaves me with an uncomfortable feeling about the decision. I also see the behaviour changed over time as the UOM library got updates. And I think there is a relatively easy solution to solve it to make it more intuitive for the typical smarthome calculations. If you look at the linked discusison from @rkoshak tests, it clearly show we leave the rules builder with a big issue. And it is because we do not properly define if the quantity should be interpreted absolute or relative. We can solve that.

An alternative approach from what I do could be to define quantities absolute by default in constructing them (now it is relative). Only when adding or substracting, make the second argument relative. It would avoid the problem of potentially having to convert back to relative at the end of calculation. Defining as absolute by default should not make a difference for all units (except temperature) we typically use in the smarthome. And it would make it easier to understand for the person building a rule.

Also, I see what got implemented as a workaround for this is only implemented in profiles, not calculations for rules.

mherwege · 2023-09-01T13:55:48Z

@J-N-K I actually think that with the change proposed here, the specific work around for temperatures in the SystemOffsetProfile

openhab-core/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/profiles/SystemOffsetProfile.java

Line 118 in e1d2b88

// take care of temperatures because they start at offset -273°C = 0K

can be completely removed.
This workaround calls the add method I propose to correct here.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege · 2023-09-01T15:33:29Z

Open question for me still: should the result of calculations be converted to relative again. I would be happy to hear views on this.

Not required, as new QuantityType objects are created for the end result, and these default to relative.

mherwege · 2023-09-01T15:34:56Z

I now removed the workaround in the offsetProfile and added all tests I had in this PR. All tests (both old and new in QuantityType and old in SystemOffsetProfile) run through fine.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege · 2023-09-04T10:32:33Z

The last commit improved associativity for adding temperatures. Now (A + B) + C = A + (B + C). Communautivity is also guaranteed.
Unit conversion of temperature results from addition/substraction or multiplication/division generate predictable results.

Distributivity will not work for temperatures (but will work for other units with only a scale factor): (A + B) * factor is not equal (A * factor) + (B * factor), e.g. (1 °C + 2°C) * 2 = 279.15 °C while 1 °C * 2 + 2 °C * 2 = 552.3 °C. The difference is because in the second example, the 2 °C are treated absolute in the multiplication.
Still, I believe this is a small price to pay for having normal addition/substraction and basic multiplication with temperatures generate a more easy to interpret and predict outcome.

mherwege · 2023-09-05T07:54:56Z

@J-N-K @rkoshak I updated the description in the PR to better explain what this change is about and what the consequences of it would be. All comments in previous post are included in the summary.

rkoshak · 2023-09-05T15:02:47Z

That all looks reasonable to me. The behaviors are much more intuitive now.

J-N-K · 2023-09-08T14:20:01Z

Do we have a test for 68 °F + 5 °C(or similar)? I would expect that the result is 77 °F.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

J-N-K

Thanks!

mherwege · 2023-09-08T14:42:48Z

Do we have a test for 68 °F + 5 °C(or similar)? I would expect that the result is 77 °F.

Just added a test like that, confirmed that works.

Special handling for temperature differences in °F and °F/%, DPT 9.002 and 9.003, need to be adapted due to change in core. Refs openhab/openhab-core#3792 Implementation is valid for 4.0 and 4.x snapshot. Fixes openhab#15567. Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>

Special handling for temperature differences in °F and °F/%, DPT 9.002 and 9.003, needs to be adapted due to change in core. Refs openhab/openhab-core#3792. Implementation is valid for 4.0 and 4.x snapshot. Fixes openhab#15567. Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>

Special handling for temperature differences in °F and °F/%, DPT 9.002 and 9.003, needs to be adapted due to change in core. Refs openhab/openhab-core#3792. Implementation is valid for 4.0 and 4.x snapshot. Fixes #15567. Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>

jimtng · 2023-09-16T10:40:56Z

I don't understand this change. Now I got this:

2 °C * 1 = 2 °C
2 °C * 2 = 277.15 °C

How is this making sense?

J-N-K · 2023-09-16T10:42:42Z

Interesting. So getting the addition right breaks multiplication?

jimtng · 2023-09-16T10:53:07Z

I have a rule that calculates an estimated change in temperature based on a constant rate of change per hour (e.g. 1.8°C / hour). So the change in temp is rate of change * number of hours. The change here broke that calculation. I'll have to convert to float, multiply by the hours, then convert back to °C before subtracting.

Here's the results of various calculations in rulesdsl

val temp = 20|°C
logInfo("Temp", "{}", temp.multiply(java.math.BigDecimal.valueOf(1))) // => 20 °C
logInfo("Temp", "{}", temp.multiply(java.math.BigDecimal.valueOf(2))) // => 313.15 °C
logInfo("Temp", "{}", temp.divide(java.math.BigDecimal.valueOf(1))) // => 20 °C
logInfo("Temp", "{}", temp.divide(java.math.BigDecimal.valueOf(2))) // => -126.575 °C

J-N-K · 2023-09-16T12:00:35Z

20 C = (273.15 + 20) K = 293.15 K
40 C = (273.15 + 2* 20) K = 313.15 K

293.15 K / 2 = 146.575 K = -126.575 C

It seems that units are mixed up.

mherwege · 2023-09-16T14:13:04Z

The change here broke that calculation. I'll have to convert to float, multiply by the hours, then convert back to °C before subtracting.

If you simply replace the change of 1.8 °C per hour by 1.8 K per hour it will work as expected. The issue with temperatures is that all of the calculations so far explicitly assumed we were always working with relative values. But internally, sometimes it was converted to absolute. This PR tries having it standardized. The impact is only with temperatures.
Addition: first argument is considered absolute, second considered relative, therefore 5 °C + 10 °C now works and gives 15 °C. But also 5 °C + 10 °F will now work. It didn’t before.
Multiplication: both arguments are considered absolute, which essentially for temperatures converts to K before doing multiplication. If you don’t do this multiplication is not commutative (you cannot switch the arguments around). I would think that’s bad and unexpected.

I don't understand this change. Now I got this:

2 °C * 1 = 2 °C

2 °C * 2 = 277.15 °C

How is this making sense?

I do think it does make sense. Multiplying temperatures is only done on the absolute scale, which is K. I know what you intend to do is multiplying temperature differences and adding them to a starting temperature, but for formulas it is a guess that’s your intention. And a choice needed to be made. It was not consistent across units before. The easy solution to keep your formulas is multiplying with Kelvin where absolute is equal to relative.

Also, before there was a difference between multiplying with a BigDecimal or a Dimensionless QuantityType. Results where different. Have a look at the original issue linked in this.

Assume you have an item that gets updated with °C from one source and °F from another source. QuantityTypes will take proper care off it. But you can not reliably calculate with the states of the item anymore if you do not first convert to a base unit. With this PR, you can.

This PR addressed the following problems: 1. Operates under the rhs unit ``` a = QuantityType.new("20 °C") b = QuantityType.new("9 °F") logger.warn a + b # => 45 °F - this should be 25 °C logger.warn b + a # => 25 °C - this should be 45 °F ``` self gets converted to other's unit + other converted to self.unit then added to self. This gave the wrong result in the wrong unit. The lhs just needs to be converted to the unit defined by the block, and rhs needs to be converted using `to_unit_relative` to the unit defined by the block. 2. Inside a unit block ``` a = QuantityType.new("20 °C") b = QuantityType.new("9 °F") unit("°C") do logger.warn a + b # => 7.2222 °C => This should be 25 °C logger.warn b + a # => 7.2222 °C => correct. 9 °F + 20 °C -> -12.78 °C + 20 °C logger.warn 2 + b # -> - 20.999 °F => Should be 2°C + 5°C = 7 °C logger.warn 2 - b # => 24.999 °F => Should be 2 - 5 = -3 °C end ``` 3. Multiplications inside a unit block didn't take up the block's unit ``` kw = 5 | "kW" unit("W") do logger.warn (kw * 2).format("%.0f %unit%") # => 10 kW => Should turn into 10000 W logger.warn (2 * kw).format("%.0f %unit%") # => 10 kW => Should turn into 10000 W end ``` 4. +/- against non-quantity type is still allowed. It takes up the other operand's unit. This should raise an exception instead. ``` kw = 5 | "kW" logger.warn kw + 5 # => 10kW logger.warn 5 + kw # => java.lang.NullPointerException: Cannot read field "quantity" because "state" is null ``` 5. Fix failing specs due to core changes in openhab/openhab-core#3792 --------- Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Special handling for temperature differences in °F and °F/%, DPT 9.002 and 9.003, needs to be adapted due to change in core. Refs openhab/openhab-core#3792. Implementation is valid for 4.0 and 4.x snapshot. Fixes openhab#15567. Signed-off-by: Holger Friedrich <mail@holger-friedrich.de> Signed-off-by: AndreasV <andreas.viborg@gmail.com>

Special handling for temperature differences in °F and °F/%, DPT 9.002 and 9.003, needs to be adapted due to change in core. Refs openhab/openhab-core#3792. Implementation is valid for 4.0 and 4.x snapshot. Fixes openhab#15567. Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>

Special handling for temperature differences in °F and °F/%, DPT 9.002 and 9.003, needs to be adapted due to change in core. Refs openhab/openhab-core#3792. Implementation is valid for 4.0 and 4.x snapshot. Fixes openhab#15567. Signed-off-by: Holger Friedrich <mail@holger-friedrich.de> Signed-off-by: querdenker2k <querdenker2k@gmx.de>

openhab-bot · 2023-12-29T16:07:33Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-1-release-discussion/152252/87

openhab-bot · 2024-01-15T11:47:07Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/working-with-number-temperature-items-in-rules/116197/29

openhab-bot · 2024-02-13T14:23:11Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/issue-with-quantity-of-temeprature-since-upgrade-to-oh4-1-1-from-4-0-4/153865/2

Special handling for temperature differences in °F and °F/%, DPT 9.002 and 9.003, needs to be adapted due to change in core. Refs openhab/openhab-core#3792. Implementation is valid for 4.0 and 4.x snapshot. Fixes openhab#15567. Signed-off-by: Holger Friedrich <mail@holger-friedrich.de> Signed-off-by: Jørgen Austvik <jaustvik@acm.org>

openhab-bot · 2024-08-19T07:08:31Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/blockly-script-unable-to-divide-or-multiply-temperatures/157871/5

fix quantity calculations

4501e6f

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege requested a review from a team as a code owner August 31, 2023 16:28

mherwege added 2 commits September 1, 2023 16:01

remove temperature offsetProfile workaround

d9c2dd8

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

include tests

61267ac

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege added 2 commits September 2, 2023 08:26

force relative scale for calculation result

0e63b80

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

improve associativity

e43520f

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege changed the title ~~Fix QuantityType calculations~~ Adjust QuantityType calculations for temperatures Sep 5, 2023

add mixed unit temperature add test

3c10514

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

J-N-K approved these changes Sep 8, 2023

View reviewed changes

J-N-K added the bug An unexpected problem or unintended behavior of the Core label Sep 8, 2023

J-N-K added this to the 4.1 milestone Sep 8, 2023

J-N-K merged commit 2b84752 into openhab:main Sep 8, 2023

This was referenced Sep 9, 2023

[knx] Handling temperature differences broken due to core change openhab/openhab-addons#15567

Closed

[knx] Disable tests failing due to QuantityType change in core openhab/openhab-addons#15568

Merged

holgerfriedrich mentioned this pull request Sep 10, 2023

[knx] Adapt to core, temperature differences openhab/openhab-addons#15573

Merged

mherwege deleted the quantity_calc branch September 11, 2023 06:36

jimtng mentioned this pull request Sep 13, 2023

Adjust specs for OH4.1 QuantityType changes openhab/openhab-jruby#145

Closed

jimtng mentioned this pull request Sep 17, 2023

Fix QuantityType arithmetic of mixed units openhab/openhab-jruby#147

Merged

mherwege mentioned this pull request Feb 4, 2024

[jsscripting] divide using quantity provides wrong results openhab/openhab-addons#16372

Closed

mherwege mentioned this pull request Mar 5, 2024

ECMAScript: Quantity can't be multiplied with negative value #4132

Closed

mherwege mentioned this pull request Aug 16, 2024

QuantityType<Temperature> misbehaviour #4350

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust QuantityType calculations for temperatures #3792

Adjust QuantityType calculations for temperatures #3792

mherwege commented Aug 31, 2023 •

edited

Loading

J-N-K commented Aug 31, 2023

mherwege commented Sep 1, 2023 •

edited

Loading

mherwege commented Sep 1, 2023 •

edited

Loading

mherwege commented Sep 1, 2023

mherwege commented Sep 1, 2023

mherwege commented Sep 4, 2023

mherwege commented Sep 5, 2023 •

edited

Loading

rkoshak commented Sep 5, 2023

J-N-K commented Sep 8, 2023

J-N-K left a comment

mherwege commented Sep 8, 2023

jimtng commented Sep 16, 2023

J-N-K commented Sep 16, 2023

jimtng commented Sep 16, 2023 •

edited

Loading

J-N-K commented Sep 16, 2023 •

edited

Loading

mherwege commented Sep 16, 2023

openhab-bot commented Dec 29, 2023

openhab-bot commented Jan 15, 2024

openhab-bot commented Feb 13, 2024

openhab-bot commented Aug 19, 2024

Adjust QuantityType calculations for temperatures #3792

Adjust QuantityType calculations for temperatures #3792

Conversation

mherwege commented Aug 31, 2023 • edited Loading

J-N-K commented Aug 31, 2023

mherwege commented Sep 1, 2023 • edited Loading

mherwege commented Sep 1, 2023 • edited Loading

mherwege commented Sep 1, 2023

mherwege commented Sep 1, 2023

mherwege commented Sep 4, 2023

mherwege commented Sep 5, 2023 • edited Loading

rkoshak commented Sep 5, 2023

J-N-K commented Sep 8, 2023

J-N-K left a comment

Choose a reason for hiding this comment

mherwege commented Sep 8, 2023

jimtng commented Sep 16, 2023

J-N-K commented Sep 16, 2023

jimtng commented Sep 16, 2023 • edited Loading

J-N-K commented Sep 16, 2023 • edited Loading

mherwege commented Sep 16, 2023

openhab-bot commented Dec 29, 2023

openhab-bot commented Jan 15, 2024

openhab-bot commented Feb 13, 2024

openhab-bot commented Aug 19, 2024

mherwege commented Aug 31, 2023 •

edited

Loading

mherwege commented Sep 1, 2023 •

edited

Loading

mherwege commented Sep 1, 2023 •

edited

Loading

mherwege commented Sep 5, 2023 •

edited

Loading

jimtng commented Sep 16, 2023 •

edited

Loading

J-N-K commented Sep 16, 2023 •

edited

Loading