Unitful.jl icon indicating copy to clipboard operation
Unitful.jl copied to clipboard

Addition of temperatures revisited

Open Balinus opened this issue 5 years ago • 18 comments

Hello,

not sure if it's by design, but basic mathematical operators with NaNs throws an error (it does work with minus though! But returns the wrong unit).

edit - It seems that the problem only occurs with Celsius, as far as I can tell and to e related to Affine units.

julia> NaN*°C + NaN*°C
ERROR: AffineError: an invalid operation was attempted with affine quantities: NaN °C + NaN °C
Stacktrace:
 [1] +(::Unitful.Quantity{Float64,�,Unitful.FreeUnits{(K,),�,Unitful.Affine{-5463//20}}}, ::Unitful.
Quantity{Float64,�,Unitful.FreeUnits{(K,),�,Unitful.Affine{-5463//20}}}) at C:\Users\Marie\.julia\pa
ckages\Unitful\b6IPw\src\quantities.jl:108
 [2] top-level scope at none:0

julia> NaN + NaN
NaN

julia> NaN*°C - NaN*°C # returns a Kelvin
NaN K

I think it would be best to just return NaNs. I have some big 3D Array and there's always some NaNs in it and it sadly throws an error.

Thanks for your effort!

Balinus avatar Dec 11 '18 01:12 Balinus

It's not related to NaNs; 3°C + 3°C is also an error since the latest release, because it's ill-defined. Consider whether 1°C ought to be equal to 1K or to 274K. The decision was made that it should be equal to 274K, and as such, addition of quantities in °C should not be defined to prevent errors. Use K for temperature differences. You can add celsius and kelvins though, and there's a way to get the result in celsius.

cstjean avatar Dec 11 '18 14:12 cstjean

ok, thanks for the explanation. I was under the impression that it was indeed related to such a concept. I guess I'll stick to Kelvin in my package and remove the option to play with Celsius.

Balinus avatar Dec 11 '18 16:12 Balinus

It depends what you do. We do industrial monitoring, and we use celsius for "room temperature", or "gas temperature". We just have to remember that temperature deltas are in kelvin.

cstjean avatar Dec 11 '18 20:12 cstjean

Yeah, it's about climate data manipulation. I was giving the option to extract the data and convert it to Celsius and then do some thresholds calculations and some basic operations on the climate grids (3D/4D grids). I can't have some function failing because of that. I'll think about it. I don't want to add lots of if and etc.

Balinus avatar Dec 11 '18 23:12 Balinus

I should have mentioned that the intended use was to calculate the mean daily temperature, from maximum and minimum daily temperature which are in Celsius. So, in the end, this can be seen as a temporal interpolation.

edit - Another intended use would be to do some bias correction. That is, correct the bias between a reference and a simulation and then apply the transfer function (which basically do "add" and "subtract" temperatures).

Balinus avatar Dec 12 '18 01:12 Balinus

The fact that celsius temperature means doesn't work is unfortunate, and should be fixed. Which other function is failing? I don't understand why you would have to use ifs. If the user remembers to use Kelvins for temperature deltas, everything should "just work".

cstjean avatar Dec 13 '18 00:12 cstjean

I'm loading netCDF files into an in-memory structure where the user has the option to specify the units (Kelvin or Celsius, defaults to Kelvin). I guess I should just throw a warning/error when the use specify Celsius as the output unit.

Thanks for the info! I'll see if other function is failing. Cheers!

Balinus avatar Dec 13 '18 01:12 Balinus

We just have to remember that temperature deltas are in kelvin.

I'm not sure I agree on this. Summing two absolute temperatures doesn't often make sense even when using absolute temperature, one usually sums a base temperature and a delta. Deltas in degree Celsius are the same as deltas in Kelvin.

In the case of 1u"°C" + 2u"°C" one of the two is the base temperature and the other one is the delta, in any case the result of the sum seems well-defined to me.

giordano avatar Dec 27 '18 19:12 giordano

See #137

cstjean avatar Dec 27 '18 19:12 cstjean

See #137

All examples I can find there (like your comments here and here) are related to the fact that combining Kelvin and degree Celsius is ill-defined, and I agree on that (which one is the base temperature and which one is the offset?), but in my opinion if both temperature are expressed in Celsius there is no such ambiguity. Well, the ambiguity is there, but the result is just the same in either case.

giordano avatar Dec 27 '18 22:12 giordano

In the case of 1u"°C" + 2u"°C" one of the two is the base temperature and the other one is the delta, in any case the result of the sum seems well-defined to me.

But how did you get that "2u"°C"" (assuming that's the delta) in the first place? Presumably it was by taking a temperature difference at some point. If this is made to work, it will definitely lead to inconsistencies such as 1u"°C" + 2u"°C" not being equal to uconvert(u"°C", uconvert(u"K", 1u"°C") + uconvert(u"K", 2u"°C")) which seems pretty unfortunate.

On the other hand, allowing this can be seen as a pragmatic concession that having affine combinations like mean "just work" is more important than having this kind of consistency. This taking into account that affine combinations are usually (always?) implemented in terms of linear combinations because interpolation libraries implicitly assume that the inputs form a vector space rather than an affine space.

The alternative is to consider converting the various interpolation libraries to express their internal computation via affine rather than linear combinations. This seems like a whole heap of work though.

Side note - I've just noticed

julia> 1u"°C" + (1u"°C" - 1u"°C")
5483//20 K

which seems kind of "wrong" (I'd expect it to produce an output in °C).

c42f avatar Jun 19 '19 07:06 c42f

While this issue is under discussion, is there something I can do to get the following behavior?

Statistics.mean([1u"°C", 2u"°C"]) == 1.5u"°C"

jtrakk avatar Jan 17 '21 07:01 jtrakk

While this issue is under discussion, is there something I can do to get the following behavior?

Statistics.mean([1u"°C", 2u"°C"]) == 1.5u"°C"

Right now, the only thing you can do is convert to kelvin before calculating the mean and then convert the result back to celsius. But I agree that it would be good for this to just work, we should probably add a specialized method for Statistics.mean.

sostock avatar Jan 17 '21 11:01 sostock

we should probably add a specialized method for Statistics.mean.

The alternative is to consider converting the various interpolation libraries to express their internal computation via affine rather than linear combinations. This seems like a whole heap of work though.

I would be interested in having a primitive operation, which should be specialized for these affine quantities, and then define Statistics.mean in terms of that primitive.

https://ro-che.info/articles/2015-12-16-torsors-midpoints-homogeneous-coordinates is a bit relevant (not the focus on static error checking).

As a sloppy sketch:

# define a fallback that can be specialized for the affine case.
weighted_sum(x, y) = sum(map(*, x, y))

# If it were possible to express the operation differently,
# then e.g. `dot` could be specialized for the affine case.
# But none of these have the right meaning, as far as I can tell.
# where it succeeds for
# weights = fill(1, (2, 3))
# summand = fill([1, 2], (2, 3))
# result = sum(weights) * [1, 2]
# and errors for
# weights = fill(1, (2, 1))
# summand = fill([1, 2], (2, 3))

# using LinearAlgebra
# weighted_sum2(x, y) = sum(x .* y)
# weighted_sum3(x, y) = dot(x, y)
# weighted_sum4(x, y) = dot(Adjoint(x), y)

# weighted_sum fallback errors on Affine units, so specialize it for the case of an affine combination
weighted_sum(x::AffineCombination, y<:AbstractArray{<:AffineUnit}) = rewrap_affine(sum(map(*, x._, unwrap_affine(y))))

# specialize a common case for performance
weighted_sum(x::AffineCombination{<:ConstantArray}, y<:AbstractArray{<:AffineUnit}) = rewrap_affine(x._._ * sum(unwrap_affine(y)))

# maybe only specialized for `AbstractArray{<:AffineUnit}`, maybe not.
mean(x) = weighted_sum(AffineCombination(ConstantArray(1/length(x))), x)

weighted_sum(x, y) might not be needed if mapreduce(*, +, x, y) works, and then we would just have to add methods to mapreduce to handle the affine case. See https://github.com/JuliaLang/julia/pull/27677

goretkin avatar Jan 17 '21 22:01 goretkin

I've also stumbled upon this since I've been wanting to do some arithmetic with temperatures. My apologies if I'm missing some relevant context here, but why not just have an "AffineDifference" unit kind that wraps an Affine, but is its own unit (but maybe gets defined alongside the regular unit), so you could do 1u"°C" + 2u"Δ°C", but still not 1u"°C" + 2u"°C".

Keno avatar Jan 21 '21 01:01 Keno

I agree with making the distinction systematically, but I don't think that addresses how to define mean on affine quantities.

Related to making this systematic distinction is "torsos", which I mentioned here: https://github.com/JuliaDiff/ChainRulesCore.jl/pull/138#issuecomment-599928868

On Wed, Jan 20, 2021, 8:50 PM Keno Fischer [email protected] wrote:

I've also stumbled upon this since I've been wanting to do some arithmetic with temperatures. My apologies if I'm missing some relevant context here, but why not just have an "AffineDifference" unit kind that wraps an Affine, but is its own unit (but maybe gets defined alongside the regular unit), so you could do 1u"°C" + 2u"Δ°C", but still not 1u"°C" + 2u"°C".

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/PainterQubits/Unitful.jl/issues/200#issuecomment-764173833, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEN3MZ5PLV4RIXLVY6LQRTS26B7ZANCNFSM4GJSBK2A .

goretkin avatar Jan 21 '21 02:01 goretkin

Strawman proposal for difference units: https://github.com/PainterQubits/Unitful.jl/pull/416. I agree that it doesn't solve the mean case, except that mean could be implemented by converting everything to its difference unit, and then converting back in the end which will have the right semantics and will do the correct promotions, etc.

Keno avatar Jan 21 '21 02:01 Keno

Issue regarding statistics functions like mean in affine spaces: https://github.com/JuliaStats/Statistics.jl/issues/47

sostock avatar Dec 05 '22 17:12 sostock