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

Define exp(UnitQuaternion{T}(0, 0, 0, 0)) = one(UnitQuaternion{T}) #126

Closed
bzinberg opened this issue Jul 21, 2020 · 6 comments
Closed

Define exp(UnitQuaternion{T}(0, 0, 0, 0)) = one(UnitQuaternion{T}) #126

bzinberg opened this issue Jul 21, 2020 · 6 comments

Comments

@bzinberg
Copy link

As currently written, exp(UnitQuaternion{Float64}(0, 0, 0, 0)) evaluates to UnitQuaternion(NaN, NaN, NaN, NaN). Upon initial thought, I think it's mathematically "correct" to special-case this behavior (i.e. when the argument is exactly zero) and define the result to be the identity quaternion. We're applying the exponential map to zero, viewed as an element of the Lie algebra of unit quaternions, and the result should be the identity element of the Lie group. WDYT?

@tkoolen
Copy link
Contributor

tkoolen commented Jul 21, 2020

I'm mostly just a bystander at this point, butUnitQuaternion{Float64}(0, 0, 0, 0) already evaluates to all-NaNs and

We're applying the exponential map to zero, viewed as an element of the Lie algebra of unit quaternions

is not really true, since an all-zeros UnitQuaternion (which must have been constructed by explicitly skipping normalization in the constructor) is not mathematically a unit quaternion. It seems strange to introduce complexity and possibly reduce performance for this case.

@bzinberg
Copy link
Author

bzinberg commented Jul 22, 2020

I agree that it's weird that "logs of unit quaternions" are expressed using the same data type as the unit quaternions themselves, but AFAICS, that is how the code is currently written. The zero I was referring to is an element of the Lie algebra (not the Lie group). Exp of it gives you a quaternion but it's not one -- and this is true not just of the zero Lie algebra element but also of all other logs-of-quaternions. Concretely, the line

M = es*/θ

computes an expression containing sin(θ) / θ, which is NaN, but that is a removable point discontinuity, and AFAICS there is one clear choice for its value. I.m.o., the more complex version is the one that doesn't do the mathematically natural thing (so it all boils down to what the mathematically natural thing is 🙂).

This showed up when I was doing SLERP interpolation, with q^t = exp(t * log(q)), and saw weird behavior that I tracked down to be occurring when t was zero in a particular code path.

Good point on performance loss -- would that be due to a run-time check whether the input equals zero?

bzinberg added a commit to bzinberg/Rotations.jl that referenced this issue Jul 22, 2020
@bzinberg
Copy link
Author

The code change I'm proposing is something like #127.

@bzinberg
Copy link
Author

bzinberg commented Jul 22, 2020

UnitQuaternion{Float64}(0, 0, 0, 0) already evaluates to all-NaNs

Ah, I did not realize that. 0 * UnitQuaternion{Float64}(1, 0, 0, 0) produces the non-NaN value I would have expected to be created by that call to the constructor.

@tkoolen
Copy link
Contributor

tkoolen commented Jul 24, 2020

I agree that it's weird that "logs of unit quaternions" are expressed using the same data type as the unit quaternions themselves

Ah, I was somehow thinking of log instead of exp because of the quaternion argument. Yeah, that is weird. I agree with your statement in #30.

@hyrodium
Copy link
Collaborator

We now have RotationGenerator with #203.

julia> exp(RotationVecGenerator(0.,0.,0.)) == one(QuatRotation)
true

This issue seems resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants