Add Base.convert to convert between different rotation representations

[juliohm]

@ronisbr I am trying to refactor some code in the Meshes.jl + GeoStats.jl ecosystem and noticed that it would be much easier to work with rotations from ReferenceFrameRotations.jl if they implemented Base.convert instead of different functions angle_to_dcm, quaternion_to_dcm, ... for the pairs:

Base.convert(::Type{DCM}, a::EulerAngles) = angle_to_dcm(a)
Base.convert(::Type{DCM}, q::Quaternion) = quarternion_to_dcm(a)
...

That way users can always call the same function convert regardless of representation and Julia decides what to do. Can I submit a PR with this feature? It should be backward compatible.

Can you elaborate on why you have chosen specific names for the conversions? Are you aware that Julia can do pretty cool conversions in the presence of convert methods like converting a vector of rotation objects to direction cosine matrix:

# prefix Vector with DCM to perform implicit conversion
DCM[EulerAngles(...), Quaternion(...), ...]

[ronisbr]

Hi @juliohm !

That way users can always call the same function convert regardless of representation and Julia decides what to do. Can I submit a PR with this feature? It should be backward compatible.

Sure! This feature is something I wanted to implement a long time ago. Thanks!

Can you elaborate on why you have chosen specific names for the conversions? Are you aware that Julia can do pretty cool conversions in the presence of convert methods like converting a vector of rotation objects to direction cosine matrix:

Let's say backward compatibility :D When this package was created in 2013/2014, I followed an approach very, very close to MATLAB so that people at INPE could migrate with lower effort. I think we can now take a more Julian approach. We can have those functions (angle_to_dcm, etc.) and use them in convert. This will be awesome!

[juliohm]

Awesome! :) I'll work on the PR tomorrow. If you feel that future users will end up using the convert methods only we could even deprecate the specific function names and move the implementations.

On Thu, Nov 11, 2021, 23:05 Ronan Arraes Jardim Chagas < @.***> wrote:

Hi @juliohm https://github.com/juliohm !

That way users can always call the same function convert regardless of representation and Julia decides what to do. Can I submit a PR with this feature? It should be backward compatible.

Sure! This feature is something I wanted to implement a long time ago. Thanks!

Can you elaborate on why you have chosen specific names for the conversions? Are you aware that Julia can do pretty cool conversions in the presence of convert methods like converting a vector of rotation objects to direction cosine matrix:

Let's say backward compatibility :D When this package was created in 2013/2014, I followed an approach very, very close to MATLAB so that people at INPE could migrate with lower effort. I think we can now take a more Julian approach. We can have those functions (angle_to_dcm, etc.) and use them in convert. This will be awesome!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaSpace/ReferenceFrameRotations.jl/issues/18#issuecomment-966758928, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3L6Q2NW4A3F52GM52TULRY73ANCNFSM5H3XXX7A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[ronisbr]

Nice! Thanks!

If you feel that future users will end up using the convert methods only we could even deprecate the specific function names and move the implementations.

Yes, we can do this. However, this kind of approach (using the old functions) is very good for people migrating from MATLAB.

[ronisbr]

Done @juliohm !

Now you can use the convert API to convert between any representation:

julia> using ReferenceFrameRotations

julia> d = rand(DCM)
DCM{Float64}:
 -0.938722  -0.116848   0.324265
 -0.168246   0.976429  -0.135203
 -0.300823  -0.181474  -0.936254

julia> convert(EulerAngles(:XYZ), d)
EulerAngles{Float64}:
  R(X) :  2.95014  rad  ( 169.03°)
  R(Y) : -0.305556 rad  (-17.5071°)
  R(Z) :  2.96425  rad  ( 169.839°)

julia> convert(Quaternion, d)
Quaternion{Float64}:
  + 0.159258 + 0.0726354⋅i - 0.98125⋅j + 0.0806823⋅k

julia> EulerAngleAxis[d for i in 1:10]
10-element Vector{EulerAngleAxis}:
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]
 EulerAngleAxis{Float64}: θ = 2.82171 rad, v = [0.0735744, -0.993936, 0.0817253]

Can you please test against master before I tag a new version?

[juliohm]

That is awesome @ronisbr , well-done! The only case that is a bit hacky still is the EulerAngles with this private EulerAngleConversion type. I wonder if the EulerAngles definition could be changed to make the symbol a type parameter. So it would be more consistent with the way Base.convert works. Would it be too much to refactor the type to make the symbol a type parameter instead of a runtime field? I feel that without the symbol the rotation type is ill-specified.

[ronisbr]

That is awesome @ronisbr , well-done!

Thanks :)

The only case that is a bit hacky still is the EulerAngles with this private EulerAngleConversion type. I wonder if the EulerAngles definition could be changed to make the symbol a type parameter. So it would be more consistent with the way Base.convert works. Would it be too much to refactor the type to make the symbol a type parameter instead of a runtime field? I feel that without the symbol the rotation type is ill-specified.

I tried it first, the problem is that you can get a lot of type instabilities. Let's say we change it to EulerAngles{R, T}, where R is a symbol. Then, a function like:

function angle_to_angle(e::EulerAngle{R, T}, new_rot_seq::Symbol) where {R, T}
    convert(EulerAngle{new_rot_seq, t}, a)
end

is type unstable. I am not sure if this is a good thing. In my simulation, this new EulerAngle decreased the performance by a lot. In the current implementation, if you do not specify any rotation sequence, it defaults to ZYX.

Do you have any idea to improve this? The new version will be breaking anyway so we have flexibility here :D

[ronisbr]

I think the only way was to call the rotation sequence by Val(:XYZ) instead of :XYZ, but I think it will confuse the users a lot. It will also to break everything with angle_to_*.

[juliohm]

The example you gave above with angle_to_angle written in terms of Base.convert is an example we should be worried about? I thought that the master had the function implemented in the opposite direction, i.e., Base.convert implemented in terms of angle_to_angle?

I think a good approach would be to always assume that computations are done with DCM? Is there a good reason to have tight loops using a different rotation representation? My current understanding is that the different representations exist because they facilitate the specification by end users in different kinds of scenarios, but at the end of the day we always get a static matrix and apply the transformation with efficient matrix-vector multiplication.

If this mental model is valid, then we could document it properly and ask that end users worried with performance should convert to DCM before tight loops?

[ronisbr]

The example you gave above with angle_to_angle written in terms of Base.convert is an example we should be worried about?

No, any function in which that symbol is not explicit defined will have problems.

I thought that the master had the function implemented in the opposite direction, i.e., Base.convert implemented in terms of angle_to_angle?

Yes, this is precisely what is implemented. The API convert just call angle_to_angle. The problem is when you are converting a DCM to Euler angle inside a function and the angle sequence is not hard coded. Actually, all conversion functions to Euler Angles will have such problems.

I think a good approach would be to always assume that computations are done with DCM?

No, this is very bad! There are some times you are better using DCM (like when you need to convert vectors) and another times when Quaternions are way better (propagating attitude and using at Kalman filters). Euler angle and axis and Euler angles are for analysis, mostly.

If this mental model is valid, then we could document it properly and ask that end users worried with performance should convert to DCM before tight loops?

We can say that some computations using Euler Angles will be slow. However, I am not seeing the gain here to justify that change. Right now, everything is type stable and very fast. The only downside is that you will not able to do things like EulerAngles(:XYZ)[d for i = 1:10], which can be replaced by convert.(EulerAngles(:XYZ), [d for i = 1:10]).

[ronisbr]

Btw, now with have the operator \circ to compose rotations in the same order of DCMs. It automatically converts the representations:

julia> D = angle_to_dcm(0.5, 0, 0, :ZYX)
DCM{Float64}:
  0.877583  0.479426  -0.0
 -0.479426  0.877583   0.0
  0.0       0.0        1.0

julia> q = angle_to_quat(0.3, 0, 0, :ZXY)
Quaternion{Float64}:
  + 0.988771 + 0.0⋅i + 0.0⋅j + 0.149438⋅k

julia> D ∘ q
DCM{Float64}:
  0.696707  0.717356  0.0
 -0.717356  0.696707  0.0
  0.0       0.0       1.0

[juliohm]

Very nice! I think the EulerAngles corner case is fine. I am more concerned with users trying to code functions in terms of types of rotations and then getting unexpected errors programmatically. That is, a function that takes a rotation type as input and calls Base.convert will work in all cases except EulerAngles.

If you can trigger a minor release for this awesome feature that would be great. I will bump the version in downstream projects and remove the workaround methods I added there.

[juliohm]

BTW, do you recommend any book on the subject? I feel that we could benefit from this more general usage of rotations in geo applications. We are mostly using DCM on vectors but I feel that optimizations could be performed with other types of rotation as you explained.

[ronisbr]

Very nice! I think the EulerAngles corner case is fine. I am more concerned with users trying to code functions in terms of types of rotations and then getting unexpected errors programmatically. That is, a function that takes a rotation type as input and calls Base.convert will work in all cases except EulerAngles.

If you can trigger a minor release for this awesome feature that would be great. I will bump the version in downstream projects and remove the workaround methods I added there.

Perfect! I will just export a Union called ReferenceFrameRotation with all supported rotations.

After that, I will tag 3.0.0 (it is breaking). I probably will do that today.

BTW, do you recommend any book on the subject? I feel that we could benefit from this more general usage of rotations in geo applications. We are mostly using DCM on vectors but I feel that optimizations could be performed with other types of rotation as you explained.

All the reference I have related to reference frame rotations / transformation are within the scope of control systems and attitude control. The two following books are very good:

Chi-Tsong Chen - Linear System Theory and Design: The eds. 1 and 2 have a very good introductory chapter on linear algebra that explains this kind of transformation.

Peter C. Hughes - Spacecraft attitude dynamics: This is the book I used in the discipline Spacecraft attitude kinematics and dynamics.