Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[automation] Arithmetic with temperature Quantities (add, subtract) #2174

Closed
Rossko57 opened this issue Feb 1, 2021 · 16 comments
Closed

[automation] Arithmetic with temperature Quantities (add, subtract) #2174

Rossko57 opened this issue Feb 1, 2021 · 16 comments

Comments

@Rossko57
Copy link

Rossko57 commented Feb 1, 2021

2°C + 1K = 3°C
That's not as simple as it looks; we would generally read that to mean "take an absolute temperature on this scale, and then add an increment using intervals of that scale".
In openHAB, we have Quantity of type temperature to represent absolute temp, but no Quantity for intervals. That leaves this kind of arithmetic ambiguous. Nevertheless, most users expect the sum to work as shown.

Demo in DSL using constants.

logInfo("sums", "constants {}", 23|°C + 2|K)   // result -248.15 °C
logInfo("sums", "constants {}", 2|K + 23|°C)   // result 298.15 K
logInfo("sums", "constants {}", 23|°C - 2|K)   // result 294.15 °C

clearly there is confusion in the framework between absolute, increment, and which units apply to results.

Typically, a real user will be trying maths like
someItem.state + 2°C
but good/bad results depend on the scale some sensor supplies, °C /°F.

While there are ways to circumvent this by conversion of everything to a given unit within each rule to process, it's misleading to allow the obvious (to users) arithmetic to appear to work yet produce wrong answers.
Request a fix for add/subtract of temperature Quantities.
Failing that, generate an error for dissimilar units.

More complex maths like
2°C * 2°C
are meaningless, and should probably error, but that's another story.

@Rossko57 Rossko57 changed the title [automation] [automation] Arithmetic with temperature Quantities (add, subtract) Feb 1, 2021
@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/formating-logfile-entries-units-of-measurement/115363/6

@krocans
Copy link

krocans commented Feb 1, 2021

Those logInfo expressions are correct.

  1. 23|°C (you are creating number temperature in °C) + 2K (which is being converted -271.15°C) = 23°C - 271.15°C = -248.15 °C
  2. 2|K (you are creating number temperature in Kelvins) + 23|°C (which is being converted 296.15K) = 2 + 296.15 = 298.15 K

2°C + 1K = 3°C this is mathematically and physically wrong although seems to be correct (you are adding apples to pears).

So mathematically and physically operators + and - does exactly what they are meant to do and does that correctly.

What you probably need are new functions, something like increaseBy(2|C, 1|K) or 23|C + convertToInterval(1|K, "°C"). I aggreee there may be use of those but there is nothing to do with +/- operators - they works correctly.

@Rossko57
Copy link
Author

Rossko57 commented Feb 1, 2021

The trouble with that viewpoint is rather philosophical, but I think it is wrong. Absolute temperature and relative temperature are clearly distinct. That approach interchanges the usage in different parts of the process.

23|°C + 2|K
Alright, 23°C is unambiguous, our starting absolute temperature.
Next, conversion of 2K to -271.15°C ... that's treating it as an absolute temperature, not an increment.
Next add two absolute temperatures ... that's a nonsense, it cannot be done. (You could average them instead, for example.)
It's like adding two map grid references, what could that represent, that's not how it works.
The problem is treating 2K as an absolute for conversion but then immediately treating as an increment for addition. Schroedinger's temperature?

I would say the mathematically correct result of 23°C + 2K is 'undefined', if we treat both elements as absolute.

User's expectation will be 25°C of course, and maybe that is incorrect. Yes, apples and pears indeed -but no way define pears in OH. (real life cases will be mostly C/F work of course)

But I don't think current function is justifiable.
Nor do I think we should be introducing a new Quantity for "temperature-interval", it seems overkill. I do not know the best way to proceed here. The simplest way is toforce an error if units don't match.

@krocans
Copy link

krocans commented Feb 1, 2021

School physics basics - calculations make sense only if values are converted to same units. And adding absolute values makes lot of sense - for example average average is sum of absolutes divided by count - average temperature between 23C and 2K is (2 + 296.15)/2 = 149,075K

You can't redefine math, + and - operates with absolute values and let's leave it so :).
Imagine you "break math" and make +/- behave "intuitive as user expected" as if second argument is "interval" - what happens. My thermometer outside shows 32F, my thermometer inside shows 20C - for how many C room is warmer than outside. By your intuitive math (size of 1F interval is 5/9 of 1C interval).
20C - 32F = 20C - 17,77... = 2.2...C .... !WRONG! it's 20C degrees warmer because 32F is exactly 0C not 17.7 not 18,8 or whatever else

That's why I proposed function approach (lintruduce functions addInterval or converttointerval) for interval adding - it's seems to be correct way.

@rkoshak
Copy link

rkoshak commented Feb 1, 2021

There are two use cases here though.

  1. When the user really did mean 23°C + (-273.15 K) (i.e. an absolute value)
  2. When the user really mean 23°C + 2°C (i.e. as an increment)

Right now the mathematical operators +/- implement the first use case. It seems clear to me what's happening. First the units are normalized using the units of the first operand. So in the above the first operand is °C so it converts 2K to °C and then does the operation. This is a valid use case and it must be supported.

To support the second use case an entirely different set of logic is required. In that case the second argument needs to be something like 2*(formula to convert between the difference between 1 unit of k and one unit of °C). I don't know if I'm conveying what I mean very well. It's more informative to show with °F. It would be 2*(5/9) which is how many °F that are in 1 °C. For °C to K it would be 2(1). For other non-temperature units, particularly units that don't follow a linear scale (e.g. decibel) it's much more complicated. And in a lot of cases the operation is meaningless.

So I agree with krocans, if use case two is to be supported at all, it needs to be a separate function, not a modification to the +/- operations.

As for the expectation, there are lots and lots of cases like these where what the user expects is simply not correct. For example, I'm at a store and the sign says "50% off with an additional 25% of it you use the store card." If the Item is $100 how much is it going to cost? Intuitively people jump to the incorrect conclusion that it will cost $25. That's wrong. It's $37.50 (i.e. $50 - ($50 * .25)). I think this is another similar case where the user's expectations stems from a misunderstanding what that calculation actually means and how it works. I'm not sure how to handle that.

Perhaps it can be handled with some additions to the docs instead of adding new methods (and a huge lookup table that defines the formulas to convert one unit of x to one unit of y). Something like "If you want to increment a UoM value, you must use the same units. Incrementing only makes sense when both operands are the same unit." Although it's more nuanced than that because if 0 in unit X == 0 in unit Y then the increment would work.

@Rossko57
Copy link
Author

Rossko57 commented Feb 2, 2021

I maintain that it is a nonsense to add two absolute temperatures. I am eager to see any use case for that!
"something that is 10 above zero" + "something that is ten above zero" = "two somethings that are ten above zero"
They're all tethered to the same origin point. I repeat my example of adding two map grid refs, arithmetically possible but meaningless.

Calculating an average is a good example - we're really just adding the numerics, and the units are outside the calculation as though in big brackets. Otherwise the units would be half-Ks or whatever when divided by two.

I appreciate this is headache inducing. I fully agree we do not want new Quantity types (which would be technically correct solution) nor new operators.

At one and the same time,it's an oddball corner case but one that our users will frequently encounter. I feel it needs more attention than a cryptic note.

@ssalonen
Copy link
Contributor

ssalonen commented Feb 2, 2021

I find this headache inducing as well.

If openHAB +- arithmetic would work in absolute temperatures, would we not expert temp1 + temp2 to equal temp2 + temp1?

        QuantityType kelvinPlusCelsius = QuantityType.valueOf("2 K").add((QuantityType) QuantityType.valueOf("23 °C"));
        QuantityType celsiusPlusKelvin = QuantityType.valueOf("23 °C").add((QuantityType) QuantityType.valueOf("2 K"));
        System.err.println("K+C in K " + kelvinPlusCelsius.toUnit("K") + ", C+K in K " + celsiusPlusKelvin.toUnit("K"));
        System.err.println(
                "K+C in °C " + kelvinPlusCelsius.toUnit("°C") + ", C+K in °C " + celsiusPlusKelvin.toUnit("°C"));

prints

K+C in K 298.15 K, C+K in K 25.00 K
K+C in °C 25.00 °C, C+K in °C -248.15 °C

Funnily enough, example of average results in "consistent" results

        QuantityType kelvinPlusCelsiusDiv2 = QuantityType.valueOf("2 K")
                .add((QuantityType) QuantityType.valueOf("23 °C")).divide(BigDecimal.valueOf(2));
        QuantityType celsiusPlusKelvinDiv2 = QuantityType.valueOf("23 °C")
                .add((QuantityType) QuantityType.valueOf("2 K")).divide(BigDecimal.valueOf(2));
        System.err.println("(K+C)/2 in K " + kelvinPlusCelsiusDiv2.toUnit("K") + ", (C+K)/2 in K "
                + celsiusPlusKelvinDiv2.toUnit("K"));
        System.err.println("(K+C)/2 in °C " + kelvinPlusCelsiusDiv2.toUnit("°C") + ", (C+K)/2 in °C "
                + celsiusPlusKelvinDiv2.toUnit("°C"));
(K+C)/2 in K 149.075 K, (C+K)/2 in K 149.075 K
(K+C)/2 in °C -124.075 °C, (C+K)/2 in °C -124.075 °C

I'm not sure if I use the correct terms myself but I think only Kelvin is in absolute scale, Celsius is 'relative'. There's good section on this in wikipedia. Indeed, wikipedia calls Celsius as "interval scale" (no absolute zero defined, only interval endpoints 0, 100 and splitting that even intervals), parallel is drawn to location in Cartertesian coordinates or compass direction.

There is definately a log, openHAB always treats °C as referring to a specific temperature, not a temperature increment/interval. I do agree with @Rossko57 that there is no clear meaning when summing quantities referring to specific temperatures together -- especially when we talk about relative scale like Celsius. I think implicitly when we say 23 °C + 2 °C we mean adding temperature increment of 2 °C to specific temperature of 23 °C.

With kelvins the addition makes sense as there is a clear absolute zero, we could interpret this adding some increment of particle motion, just like adding 5 kilojoules to 2 kilojoules.

Take the above more as philosophical musings, not sure what is the right approach forward.

@krocans
Copy link

krocans commented Feb 2, 2021

As a workaround do 23|°C + 2|K - 0|K that does the trick for any quantity types for which 0 points are not the same. Eg. for temperature where 0C ≠ 0K ≠ 0F.

@rkoshak
Copy link

rkoshak commented Feb 2, 2021

All we are talking about is temperature. What about all the other units? Does this same problem apply to the other units?

No, it doesn't. I can add 2 km + 2 mi and get the expected result. The problem only arises when 0 from one unit does not equal 0 from the other unit. 0 km == 0 mi but 0 °C != 0 K != 0 °F

But if we change how +/- works, it will change it for all the QuantityTypes. The actual operation takes place in QuantityType and it's done with the same code regardless of units. https://github.com/openhab/openhab-core/blob/127724c0e31ad8abf0855d87fbf96204267a39be/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java and in fact the actual addition is performed by jaxax.measure.Quantity: https://unitsofmeasurement.github.io/unit-api/site/apidocs/javax/measure/Quantity.html#add(javax.measure.Quantity) which we as the openHAB project have no control over anyway.

So if the behavior has any changes of changing, it has to happen over on the unitsofmeasurement project, not here.

Looking through the issues over on the unitsofmeasurement site though shows that this is a known about concern. For example unitsofmeasurement/unit-api#140 has a lengthy discussion but I cannot tell if anything they talk about there has actually been implemented. If so, then there might need to be some changes to how OH uses them. If not, we have to wait for them to make changes first and then adjust OH to use those changes.

@krocans
Copy link

krocans commented Feb 2, 2021

@rkoshak math is always the same
To perform operation to VALUE (given in units A) add INTERVAL (given in units B) we always in fact do following math:
VALUE|A + INTERVAL|B_CONVERTDED_TO_INTERVAL_IN_UNITS_A
INTERVAL|B_CONVERTDED_TO_INTERVAL_IN_UNITS_A is calculated as INTERVAL|B(as value)_CONVERTED_TO_A - 0|B_CONVERTED_TO_A
This meta math can be written in OH as follows
= VALUE + INTERVAL.toUnit(A) - (0|B).toUnit(A)
And I propose to make it as universal built-in function addInterval(value, interval)

This is universal for all metrics with common 0 point and without (of course if not adding meters to pascals).
If those units have "common zero point" we got what mathematicians call "special case" of this universal formula
In this case (0|B).toUnit(A) is also 0 so in fact we are getting
VALUE + INTERVAL.toUnit(A) - (0|B).toUnit(A) = VALUE + INTERVAL.toUnit(A) - 0|A = VALUE + INTERVAL.toUnit(A); that's why for distance (meters/miles) we can write simple VALUE + INTERVAL.toUnit(A)

in rules +/- expressions OH automatically converts all operands to units of first operand. So we skip typing those toUnit an write simply: 1|C + 2|K - 0|K ang get correct result

If yo want to be fully mathematically correct detailed your mile/km example should be written as 2|km + 2|mi - 0|mi ... we simply KNOW that 0|mi is also 0|km so we are skipping it.

@ssalonen
Copy link
Contributor

ssalonen commented Feb 2, 2021

Nice find @rkoshak , summarizes quite well the issues with the current implementation, as also exemplified above as well. The discussion goes pretty deep.

It's quite amusing to find that Kai has been pointing out the same issue some 2-3 years ago unitsofmeasurement/unit-api#95 (comment)

It's not clear to me either if this has been implemented in the library used by openHAB. I have understood there are multiple implementations satisfying the uom api/spec.

@rkoshak
Copy link

rkoshak commented Feb 2, 2021

in rules +/- expressions OH automatically converts all operands to units of first operand

One of the main points of my above reply is that it is not openHAB doing that. It's the third party units of measurement library that OH uses.

@Rossko57
Copy link
Author

Rossko57 commented Feb 3, 2021

Well, on reflection I think it boils down to -
We don't currently have a means to distinguish two basic different tasks; summing absolute temperatures OR incrementing/decrementing one absolute temperature.

We don't really want to introduce a new Quantity Type just for temperature interval.
We don't really want to introduce some new weird operator or method just for temperatures.

However, the rule author knows which task he wants to do. And it is possible to do either task using existing tools. It isn't intuitive, but works. (Really it's just always get everything into the same units first, then + operator functions for either task accurately. It's not obvious as it runs counter to the whole idea of freely mixing Quantity units.)

So that is just education. I have written a forum post on all things Rules&Temperature, hope will guide some, welcome improvement.
https://community.openhab.org/t/working-with-number-temperature-items-in-rules/116197

Of course people don't read until they're in trouble, and there are some "obvious" ways to approach all this that are in fact traps for the unwary. So we can expect "help! where did 289.7 come from?" traffic forever, but I cannot see a reasonable way to change OH to reduce confusion.

I'm not at all sure what should be added briefly to what official docs to somehow highlight this oddity, if anyone wants to think about that?

@Rossko57
Copy link
Author

Rossko57 commented Mar 3, 2021

I'm happy to add some commentary to openHAB docs about this pitfall, but have no idea which pages to do amongst other OH3 changes.

@rkoshak
Copy link

rkoshak commented Mar 4, 2021

I can see two candidates for where to add this to the docs. Either in the Units of Measurement page itself: https://www.openhab.org/docs/concepts/units-of-measurement.html or as a paragraph in the Rules page where working with Quantity types is discussed: https://www.openhab.org/docs/configuration/rules-dsl.html#number-item

Personally I'd choose the latter since the whole concept is doing calculations in the rules.

I also plan to bring the topic up briefly in the Getting Started tutorial when I get to the rules section. I just submitted the Pages part so it might be a bit before I get to it though.

@Rossko57
Copy link
Author

Rossko57 commented Mar 5, 2021

Okay, thanks.
Added small amount to working in rules.

Happy to close now, even though it's still headache inducing.

@Rossko57 Rossko57 closed this as completed Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants