-
Notifications
You must be signed in to change notification settings - Fork 114
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 degree + radian is unitless #537
Comments
Just to clarify, which of the following is the issue here:
What operation is type-unstable because of this?
Yes, it is unfortunate that dispatching on |
Thanks for the follow-up, I'd like to say both are the same issue, which is that Radians are sometimes treated as an angle and sometimes number and this inconsistency leads to confusion. ulia> Angle{T} = Union{Quantity{T,NoDims,typeof(u"rad")}, Quantity{T,NoDims,typeof(u"°")}} where T
Union{Quantity{T, NoDims, Unitful.FreeUnits{(rad,), NoDims, nothing}}, Quantity{T, NoDims, Unitful.FreeUnits{(°,), NoDims, nothing}}} where T
julia> g(a::Angle) = @show a
g (generic function with 1 method)
julia> g(1°)
a = 1°
1°
julia> g(1u"rad")
a = 1 rad
1 rad
julia> g(1°+2°)
a = 3°
3°
julia> g(1u"rad" + 2u"rad")
a = 3 rad
3 rad
julia> g(1° + 2u"rad")
ERROR: MethodError: no method matching g(::Float64)
Closest candidates are:
g(::Union{Quantity{T, NoDims, Unitful.FreeUnits{(rad,), NoDims, nothing}}, Quantity{T, NoDims, Unitful.FreeUnits{(°,), NoDims, nothing}}} where T) at REPL[49]:1
Stacktrace:
[1] top-level scope
@ REPL[54]:1 Again I think the value of unit systems is that they help to distinguish variables and guide users in creating consistent, physically-intelligible models. While angles are, I guess, a mathematical convention rather than traceable to a physical quantity, it strains the conceptual model to define julia> AngleOrNumber = Union{Angle, Real}
Union{Real, Union{Quantity{T, NoDims, Unitful.FreeUnits{(rad,), NoDims, nothing}}, Quantity{T, NoDims, Unitful.FreeUnits{(°,), NoDims, nothing}}} where T}
julia> typeof(1) <: AngleOrNumber
true
julia> typeof(1°) <: AngleOrNumber
true That is I would prefer to treat angles as a fully-contrived unit that is not mapped to dimensionless/Real at all. Alternately, a fallback promotion from Real to Radian might enable |
@bc0n checkout DimensionfulAngles.jl, it treats angles as a dimension. |
Here's a two-line hack I used to solve this problem: AngularUnits = Union{typeof(unit(1.0°)), typeof(unit(1.0rad))};
# without this there's a bug when adding different angular units...
Unitful.promote_unit(lhs::T, rhs::S) where {T<:AngularUnits, S<:AngularUnits} = rad This always returns the result of addition/subtraction in Unitful.promote_unit(lhs::T, rhs::S) where {T<:AngularUnits, S<:AngularUnits} = lhs but that somehow yields a Stackoverflow error. |
Btw I think it would be really good if this issue would be dealt with properly, or at least throw an error of some kind. I had a pretty painful debugging session trying to find this, especially since |
I expect to have to explicitly ustrip() when I want a unitless quantity.
This behavior makes certain operations type-unstable, which the docs describe and dismiss.
Much of the point of having typed functions goes away if I also have to accept floats because their input units were automatically, silently canceled.
UnitfulAngles.jl does not fix this.
The text was updated successfully, but these errors were encountered: