JuliaGeometry / Quaternions.jl

A Julia implementation of quaternions
https://juliageometry.github.io/Quaternions.jl
MIT License
116 stars 37 forks source link

Bug in `log()` for negative reals? #114

Closed trahflow closed 1 year ago

trahflow commented 1 year ago

I think there is a bug in Base.log(q::Quaternion) if q is a negative real.

For example:

julia> q = quat(-1.0)
QuaternionF64(-1.0, 0.0, 0.0, 0.0, true)

julia> exp(log(q)) == q
false

note that this is not due to floating point (im-)precision:

julia> exp(log(q))
QuaternionF64(1.0, 0.0, 0.0, 0.0, true)

julia> log(q)
QuaternionF64(0.0, 0.0, 0.0, 0.0, false)

Related answer on math.stackexchange: https://math.stackexchange.com/a/3750503

Note in particular:

For negative reals, the imaginary component of the branches has the form (2n+1)πu^. Thus (log(k),0,0,0) is not a valid value of log(k,0,0,0).

I think a fix would be to choose the branch of the logarithm that corresponds to the logarithm on complex numbers, again quoting above math.stackexchange answer:

But, for example, if you choose u^=(1,0,0) then you get log(−2,0,0,0)=(log(|−2|),(2n+1)π,0,0). This is exactly the logarithm on C.

sethaxen commented 1 year ago

Indeed this is a bug that it appears was introduced in #56, where we incorrectly removed this check: https://github.com/JuliaGeometry/Quaternions.jl/blob/2ea186f3806862383175ea87c675221e9fc61432/src/Quaternion.jl#L134-L139

We kept that check in extend_analytic but use a separate implementation for log. I'll push a fix.