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

Change the order of QuatRotation from (w,x,y,z) to (x,y,z,w) #210

Open
ufechner7 opened this issue Dec 26, 2021 · 12 comments
Open

Change the order of QuatRotation from (w,x,y,z) to (x,y,z,w) #210

ufechner7 opened this issue Dec 26, 2021 · 12 comments
Labels

Comments

@ufechner7
Copy link

ufechner7 commented Dec 26, 2021

To be able to set the orientation of an object in a 3D scene of Makie.jl I use the following code:

    q0        = Rotations.params(QuatRotation(state.orient))     # returns an SVector in the order w,x,y,z
    quat[]    = Quaternionf0(q0[2], q0[3], q0[4], q0[1])         # the constructor expects the order x,y,z,w

This looks overly complicated to me. Can't this be done easier?

@hyrodium
Copy link
Collaborator

In what package is Quaternionf0 defined?
I'm not familiar with Makie.jl, and I couldn't find the definition in the Makie.jl repository.

I think the package (which defines Quaternionsf0) should depend on this package, and add some methods for Quaternionf0(::QuatRotation).

@ufechner7
Copy link
Author

@hyrodium
Copy link
Collaborator

Okay, I found Quaternionf instead of Quaternionf0.
https://github.com/JuliaPlots/Makie.jl/blob/master/src/utilities/quaternions.jl#L18

I think we would not like to have an additional method for (x,y,z,w), because it is verbose.
However, I understand that many libraries use (x,y,z,w) order instead of (w,x,y,z).

We have the following choices to solve this problem:

In any case, I think this is not an issue of Rotations.jl.

@hyrodium
Copy link
Collaborator

hyrodium commented Jan 6, 2022

With StaticArrays@v1.3.0, we can access elements of SVector with .x etc. (The PR: JuliaArrays/StaticArrays.jl#980)

julia> using StaticArrays

julia> v = SVector(1,2,3,4)
4-element SVector{4, Int64} with indices SOneTo(4):
 1
 2
 3
 4

julia> v.x,v.w
(1, 4)

This is incompatible with Rotations.params.

julia> using Rotations

julia> q = QuatRotation(1.,0.,0.,0.)
3×3 QuatRotation{Float64} with indices SOneTo(3)×SOneTo(3)(Quaternion{Float64}(1.0, 0.0, 0.0, 0.0, true)):
 1.0  0.0  0.0
 0.0  1.0  0.0
 0.0  0.0  1.0

julia> Rotations.params(q)
4-element SVector{4, Float64} with indices SOneTo(4):
 1.0
 0.0
 0.0
 0.0

julia> Rotations.params(q).x
1.0

We should change the order of arguments of QuatRotation and return value of params(::QuatRotation), and release v2.0.0

@hyrodium hyrodium changed the title Interoperability with Makie.jl Change the order of QuatRotation from (w,x,y,z) to (x,y,z,w) Jan 6, 2022
@hyrodium hyrodium added the v2.0.0 label Jan 6, 2022
@c42f
Copy link
Member

c42f commented Jan 9, 2022

I hope JuliaArrays/StaticArrays.jl#980 isn't causing problems here?

I guess it has wider consequences than expected (it was meant to be an optional concrete API for doing "normal geometry stuff" with short static vectors...)

@andyferris
Copy link
Contributor

However it fell out, it’s going to be better to make a breaking change here than in StaticArrays…

@c42f
Copy link
Member

c42f commented Jan 9, 2022

Hah, you're right about that, I don't think we can (or really should) take that PR back as I think it's a major usability win for a very common use case.

But it's still always a little unnerving adding features to StaticArrays (or Base for that matter). It tends to have unexpected consequences, both good and bad.

@bjack205
Copy link
Collaborator

bjack205 commented Jan 9, 2022

At least in my fields of dynamics, aerospace, robotics, and mechanical engineering, it's more standard to have the quaternion ordered as [w,x,y,z]. So I'm in favor of keeping it as-is.

Instead of depending on the change in StaticArrays, could we just define getproperty(::QuatRotation, ::Symbol) and special case x, y,z, and w? Especially since accessing these is now a layer deeper than it used to be (e.g. q.q.s), which is rather inconvenient and has already broken quite a bit of existing code that relied on the previous fields.

@andyferris
Copy link
Contributor

Yeah IMO it would probably be good to have direct property access on the rotation rather than going via params anyway.

@c42f
Copy link
Member

c42f commented Jan 10, 2022

define getproperty(::QuatRotation, ::Symbol) and special case x, y,z, and w?

This makes sense to me. Code which knows it's acting on a particular concrete type can just depend on the field names of that type. There's no need to have uniformity/consistency between different concrete types for this use case.

@hyrodium
Copy link
Collaborator

define getproperty(::QuatRotation, ::Symbol) and special case x, y,z, and w?

This will be implemented in #209 and is waiting for some reviews.

@hyrodium
Copy link
Collaborator

hyrodium commented Nov 24, 2023

At least in my fields of dynamics, aerospace, robotics, and mechanical engineering, it's more standard to have the quaternion ordered as [w,x,y,z]. So I'm in favor of keeping it as-is.

@bjack205
I'm not much familiar with these fields, but OpenGL, SciPy, Unity, PyBullet, and ROS uses [x,y,z,w]-order. (there are more!)
The software using [w,x,y,z]-order that I found was only Blender and OGRE.
Could you provide some references for [w,x,y,z]-order?

The respective advantages of [w,x,y,z]-order and [x,y,z,w]-order can be summarized as follows:

Advantages of [w,x,y,z]-order

  • Quaternions.jl uses $(w + xi + yj + zk)$-order to be consistent with Base.Complex's $(x+yi)$-order.
  • The original issue was from Makie.jl, but Makie should now use Quaternions.Quaternion which uses [w,x,y,z]-order.
  • Rotations.jl should be consistent with the order in Quaternions.jl

Advantages of [x,y,z,w]-order

  • Most of software adopt [x,y,z,w] order
    • This is for keeping consistency with the "x-first" rule: [x,y], [x,y,z], and [x,y,z,w].
  • StaticArrays.jl have getproperty(::SVector, ::Symbol) which is not consistent with [w,x,y,z] order
    • This can be avoided by deprecating Rotations.params(::QuatRotation) and add a new function Rotations.parameters(::QuatRoations) which returns Tuple (not SVector).

I personally believe all of the software should be consistent with [w,x,y,z]-order, and consistency with other software is not our No.1 priority.

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

No branches or pull requests

5 participants