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

UnitQuaternion scalar division #140

Closed
wants to merge 2 commits into from

Conversation

janbruedigam
Copy link

This adds scalar division for a UnitQuaternion q and scalar s. So far, only scalar multiplication had been implemented.
q/s and s\q are just element-wise division, s/q and q\s use the inverse for regular quaternions since multiplication and division with scalars might result in non-unit quaternions.

@andyferris
Copy link
Contributor

So this should be defined by matrix - scalar division, as a UnitQuaternion is just a compressed representation of a 3x3 matrix.

julia> r = one(UnitQuaternion)
3×3 UnitQuaternion{Float64} with indices SOneTo(3)×SOneTo(3)(1.0, 0.0, 0.0, 0.0):
 1.0  0.0  0.0
 0.0  1.0  0.0
 0.0  0.0  1.0

julia> r / 2
3×3 StaticArrays.SMatrix{3, 3, Float64, 9} with indices SOneTo(3)×SOneTo(3):
 0.5  0.0  0.0
 0.0  0.5  0.0
 0.0  0.0  0.5

I think the UnitQuaternion invariants should be maintained by all operations, and never return a non-unit quaternion.

@janbruedigam
Copy link
Author

Thanks for the reply!
I see your point that UnitQuaternions should stay unit quaternions to some degree. For me this issue came up when working with discrete-time angular velocities where you sometimes need to multiply quaternions with a time step dt.

When you say UnitQuaternion invariants should be maintained by all operations, do you also mean the currently implemented scalar multiplication should be removed? Because the scalar multiplication would then give you a 3x3 matrix which you would have to convert to a non-unit quaternion for which there is no nice constructor as far as I see it, but maybe I missed it.
But if scalar multiplication is kept, it seems a bit inconsistent that I can do q*(1/2), but not simply q/2.

@hyrodium
Copy link
Collaborator

I think the current * operator for UnitQuaternion should be removed like conj method.
(See #171 (comment))

@hyrodium
Copy link
Collaborator

For the multiplication of quaternion, Quaternions.jl will be helpful. I think it's better to add dependency on the package and support type comvesion from Rotations.UnitQuaternion to Quaternions.Quaternion. (#168)

@andyferris
Copy link
Contributor

I think the current * operator for UnitQuaternion should be removed like conj method.

Gosh! Yes.

julia> one(UnitQuaternion)
3×3 UnitQuaternion{Float64} with indices SOneTo(3)×SOneTo(3)(1.0, 0.0, 0.0, 0.0):
 1.0  0.0  0.0
 0.0  1.0  0.0
 0.0  0.0  1.0

julia> 3 * one(UnitQuaternion)
3×3 UnitQuaternion{Float64} with indices SOneTo(3)×SOneTo(3)(3.0, 0.0, 0.0, 0.0):
 9.0  0.0  0.0
 0.0  9.0  0.0
 0.0  0.0  9.0

These are meant to follow matrix multiplication, etc.

For me this issue came up when working with discrete-time angular velocities where you sometimes need to multiply quaternions with a time step dt.

Right.

This is why scattered through the issues and PRs here you'll find discussion of log and exp and pow and the difference between Lie algebras / Lie groups and skew-symmetric matrices. (Here our rotations are the Lie group of orthogonal matrices). We all want to get there :)

With the new release I think you can do rotation ^ dt and create an integration over time that way?

For the multiplication of quaternion, Quaternions.jl will be helpful. I think it's better to add dependency on the package and support type comvesion from Rotations.UnitQuaternion to Quaternions.Quaternion. (#168)

Yeah I can see that the status quo is confusing people and even we've got it muddled now.

I propose

struct UnitQuaterion{T} <: Rotation{3, T}
    quat::Quaternions.Quaternion
end

so that it's a wrapper around the Quaternions.jl quaternion, and conversion back-and-forth is trivial. You can use the quaternion algebra or matrix algebra as you wish.

@hyrodium
Copy link
Collaborator

I will close this PR because we have removed *(::UnitQuaternion, ::Real) method. (#175)

@hyrodium hyrodium closed this Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants