Drop normalize for quaternions

[sethaxen]

LinearAlgebra only implements 2 normalize methods:

julia> using LinearAlgebra

julia> methods(normalize)
# 2 methods for generic function "normalize":
[1] normalize(a::AbstractArray) in LinearAlgebra at /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/LinearAlgebra/src/generic.jl:1788
[2] normalize(a::AbstractArray, p::Real) in LinearAlgebra at /Applications/Julia-1.7.app/Contents/Resources/julia/share/julia/stdlib/v1.7/LinearAlgebra/src/generic.jl:1788

Notably, there is no normalize(::Real) or normalize(::Complex). For good reason, as for numbers, this is the role of the sign function, which is defined generically for all numbers:
https://github.com/JuliaLang/julia/blob/bb2d8630f3aeb99c38c659a35ee9aa57bb71012a/base/number.jl#L161
We should deprecate normalize then and refer users to the sign function.

A difference between our normalize and sign functions, however, is how they treat zero quaternions:

julia> normalize(q)
QuaternionF64(0.1015814137657451, 0.8417214120379644, 0.5262271589168154, 0.0653548629598791, true)

julia> sign(q)
QuaternionF64(0.1015814137657451, 0.8417214120379644, 0.5262271589168154, 0.0653548629598791, false)

julia> normalize(zero(q))
QuaternionF64(NaN, NaN, NaN, NaN, true)

julia> sign(zero(q))
QuaternionF64(0.0, 0.0, 0.0, 0.0, false)

There are at least 4 different ways one could handle normalize for zero quaternions:

• return a 0 quaternion (sign)
• return NaNs (current behavior)
• return a random unit quaternion
• return a deterministic unit quaternion (e.g. Quaternion(1))

I don't like the last 2, as they are arbitrary. The question is whether the current behavior is a bug or the desired behavior. If a bug, then we are free to define Base.@deprecate normalize(x) sign(x). If desired, then we should define a different function for normalizing instead of overriding LinearAlgebra.normalize and deprecate normalize.

[sethaxen]

I lean towards "bug". Here's why:

1. normalize([q.s, q.v1, q.v2, q.v3]) for a 0 quaternion returns a zero vector. (note that before fix piracy and type instability #53, we pirated normalize(::Vector) so that it also returned NaNs, but this was a bug). Since our behavior is inconsistent with the behavior of normalize, this seems like a bug. Edit: this was incorrect, see Drop normalize for quaternions #69 (comment) for correction.
2. A NaN return value is not useful. If a downstream user is currently relying on this behavior, then they must be calling isnan to special-case around it and give something actually normalized (but not isnan(::Quaternion), because that always returns false right now.
3. More likely, a user is already checking iszero before calling normalize to special-case if they really need a unit quaternion and zero quaternions are possible

[mikmoore]

I think you mis-typed. To be clear, the standard behavior is LinearAlgebra.normalize([0.0]) == [NaN] and it was only piracy in Quaternions.jl (which has recently been fixed) that resulted in [0.0].

I agree that deprecating normalize(::Quaternion) in favor of sign is the correct action. As you pointed out, normalize is strictly applicable to AbstractArray and not to Number, so we never should have overloaded LinearAlgebra.normalize. We should also specialize sign to set the .norm field on finite nonzero inputs.

As you point out, there really is no right way to produce a unit quaternion from a zero quaternion. It is up to the specific use site to dictate whether there is a reasonable value one can use (eg, one) or whether NaNs should be introduced or an error should be thrown.

For user-written functions, they will (they must) handle this on their own -- it's not the package's job to hand them an arbitrary value. For the package function axis, for example, my personal vote would be to return a zero or NaN vector, but the current behavior of returning [1,0,0] isn't totally crazy except that it can hide the source of them problem. Meanwhile, functions like linpol don't have any benign value and I would suggest a DomainError or a NaN return. For functions like rotationmatrix I would also suggest an error or NaN. There is no valid notion of what it means to rotate by a zero quaternion. In fact, I would personally argue that rotationmatrix should error on any non-unit quaternion except that it would be breaking. A random value should absolutely never be returned from any function that isn't specifically asking for one and NaN is much better than an arbitrary value (like 1) in most places.

[sethaxen]

I think you mis-typed. To be clear, the standard behavior is LinearAlgebra.normalize([0.0]) == [NaN] and it was only piracy in Quaternions.jl (which has recently been fixed) that resulted in [0.0].

Yes, thanks for correcting me! I agree with all of your points.