JuliaSpace / ReferenceFrameRotations.jl

A toolbox to represent 3D rotations of coordinate frames for Julia language.
MIT License
58 stars 12 forks source link

Mutable quaternion? #27

Open mattsignorelli opened 3 days ago

mattsignorelli commented 3 days ago

My organization has a need for a mutable quaternion. We really like this package as a lightweight, fast quaternion implementation for simulations, and we were wondering if a PR would be accepted where we implement a MQuaternion which is mutable, and have both Quaternion and MQuaternion inherit from some AbstractQuaternion, which the current function type definitions would then use. This would have no impact on current users of the package but also expand its functionality

ronisbr commented 3 days ago

Hi @mattsignorelli !

I am glad that ReferenceFrameRotations.jl is being useful :)

One of the reasons for the speed of this package is because all the types are immutable. Can you please highlight why do you need a mutable quaternion? Furthermore, did you check Accessors.jl? Using this package, it is possible to do this:

julia> using ReferenceFrameRotations

julia> using Accessors

julia> function test()
       q = Quaternion(1, 2, 3, 4)
       @reset q.q0 = 10
       return q
       end
test (generic function with 1 method)

julia> test()
Quaternion{Int64}:
  + 10 + 2⋅i + 3⋅j + 4⋅k

It automatically creates a copy with the new field. In most of cases, it is significantly faster than using a mutable structure.

mattsignorelli commented 3 days ago

Hello!

The idea is that we'd have a separate type, MQuaternion which could be used as an alternative to the primary fast Quaternion.

We are currently planning for our accelerator physics simulation API to be mutating, because instead of just simulating particles going through the accelerator (which could basically just be SVector{Float64}), we also need to simulate "truncated power series" (implemented as TPS in GTPSA.jl; the result of propagating a TPS is a Taylor map representing the particle transport to some high order.

The TPS type however is mutable, and to perform simulations without any extra allocations from each arithmetic operation including a TPS, we can instead modify in-place the result TPS which has been pre-allocated. So instead of writing separate mutating functions for TPS types, we would prefer the entire API to be mutating and have the same universally-polymorphic functions work for both immutable number types and our mutable TPSs.

ronisbr commented 3 days ago

I see, sounds very reasonable! :)

Can you make a PR? We just need to make sure we did not decrease the performance of the previous version.

mattsignorelli commented 2 days ago

Sure! I'll get started on it now

ronisbr commented 2 days ago

Awesome!

ronisbr commented 2 days ago

By the way! When coding, please, if possible, follow the BlueStyle: https://github.com/JuliaDiff/BlueStyle