Rotations.jl icon indicating copy to clipboard operation
Rotations.jl copied to clipboard

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

Open ufechner7 opened this issue 3 years ago • 12 comments

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?

ufechner7 avatar Dec 26 '21 23:12 ufechner7

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).

hyrodium avatar Dec 27 '21 00:12 hyrodium

Its defined in Makie.jl: https://github.com/JuliaPlots/Makie.jl/blob/master/src/utilities/quaternions.jl

ufechner7 avatar Dec 27 '21 08:12 ufechner7

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 avatar Dec 31 '21 09:12 hyrodium

With [email protected], we can access elements of SVector with .x etc. (The PR: https://github.com/JuliaArrays/StaticArrays.jl/pull/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 avatar Jan 06 '22 13:01 hyrodium

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...)

c42f avatar Jan 09 '22 01:01 c42f

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

andyferris avatar Jan 09 '22 04:01 andyferris

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.

c42f avatar Jan 09 '22 05:01 c42f

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.

bjack205 avatar Jan 09 '22 21:01 bjack205

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

andyferris avatar Jan 10 '22 00:01 andyferris

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.

c42f avatar Jan 10 '22 06:01 c42f

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 avatar Jan 10 '22 06:01 hyrodium

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.

hyrodium avatar Nov 24 '23 15:11 hyrodium