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

Addition of temperatures revisited #200

Open
Balinus opened this issue Dec 11, 2018 · 18 comments
Open

Addition of temperatures revisited #200

Balinus opened this issue Dec 11, 2018 · 18 comments
Labels
affine affine quantities like °C, °F, …

Comments

@Balinus
Copy link

Balinus commented Dec 11, 2018

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!

@cstjean
Copy link
Contributor

cstjean commented Dec 11, 2018

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.

@Balinus
Copy link
Author

Balinus commented Dec 11, 2018

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.

@cstjean
Copy link
Contributor

cstjean commented Dec 11, 2018

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.

@Balinus
Copy link
Author

Balinus commented Dec 11, 2018

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
Copy link
Author

Balinus commented Dec 12, 2018

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).

@cstjean
Copy link
Contributor

cstjean commented Dec 13, 2018

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".

@Balinus
Copy link
Author

Balinus commented Dec 13, 2018

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!

@giordano
Copy link
Collaborator

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.

@cstjean
Copy link
Contributor

cstjean commented Dec 27, 2018

See #137

@giordano
Copy link
Collaborator

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.

@ajkeller34 ajkeller34 changed the title Addition of NaNs throws an error Addition of temperatures revisited Jun 12, 2019
@c42f
Copy link
Contributor

c42f commented Jun 19, 2019

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).

@jtrakk
Copy link

jtrakk commented Jan 17, 2021

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"

@sostock
Copy link
Collaborator

sostock commented Jan 17, 2021

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.

@goretkin
Copy link

goretkin commented Jan 17, 2021

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 JuliaLang/julia#27677

@Keno
Copy link
Contributor

Keno commented Jan 21, 2021

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".

@goretkin
Copy link

goretkin commented Jan 21, 2021 via email

@Keno
Copy link
Contributor

Keno commented Jan 21, 2021

Strawman proposal for difference units: #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.

@sostock
Copy link
Collaborator

sostock commented Dec 5, 2022

Issue regarding statistics functions like mean in affine spaces: JuliaStats/Statistics.jl#47

@sostock sostock added the affine affine quantities like °C, °F, … label Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affine affine quantities like °C, °F, …
Projects
None yet
Development

No branches or pull requests

8 participants