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

Mutable quaternion? #27

Open
mattsignorelli opened this issue Sep 17, 2024 · 7 comments
Open

Mutable quaternion? #27

mattsignorelli opened this issue Sep 17, 2024 · 7 comments

Comments

@mattsignorelli
Copy link

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
Copy link
Member

ronisbr commented Sep 17, 2024

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 + 2i + 3j + 4k

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

@mattsignorelli
Copy link
Author

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
Copy link
Member

ronisbr commented Sep 17, 2024

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
Copy link
Author

Sure! I'll get started on it now

@ronisbr
Copy link
Member

ronisbr commented Sep 18, 2024

Awesome!

@ronisbr
Copy link
Member

ronisbr commented Sep 18, 2024

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

@mattsignorelli
Copy link
Author

Sounds good! It might take a couple weeks to finish since I'm a bit swamped right now but it's on my to-do list

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

No branches or pull requests

2 participants