JuliaGeometry / Rotations.jl

Julia implementations for different rotation parameterizations
https://juliageometry.github.io/Rotations.jl
MIT License
176 stars 44 forks source link

`rotation_between` for general dimensions #257

Closed hyrodium closed 1 year ago

hyrodium commented 1 year ago

This PR fixes https://github.com/JuliaGeometry/Rotations.jl/issues/256.

julia> using Rotations

julia> using StaticArrays

julia> rotation_between(SVector(1,2,3,5),SVector(1,3,2,5))  # Can be `Rotation{4}`
4×4 RotMatrix{4, Float64, 16} with indices SOneTo(4)×SOneTo(4):
  0.999334  -0.027306  0.023976  -0.00333
  0.023976   0.983017  0.136863   0.11988
 -0.027306  -0.119547  0.983017  -0.13653
 -0.00333   -0.13653   0.11988    0.98335

julia> rotation_between(SVector(3,1),SVector(1,3))  # Can be `Rotation{2}`
2×2 Angle2d{Float64} with indices SOneTo(2)×SOneTo(2)(0.927295):
 0.6  -0.8
 0.8   0.6
codecov[bot] commented 1 year ago

Codecov Report

Merging #257 (8700683) into master (d56bc26) will increase coverage by 0.32%. The diff coverage is 100.00%.

:exclamation: Current head 8700683 differs from pull request most recent head 486de18. Consider uploading reports for the commit 486de18 to get more accurate results

@@            Coverage Diff             @@
##           master     #257      +/-   ##
==========================================
+ Coverage   89.51%   89.84%   +0.32%     
==========================================
  Files          15       17       +2     
  Lines        1593     1615      +22     
==========================================
+ Hits         1426     1451      +25     
+ Misses        167      164       -3     
Impacted Files Coverage Δ
src/Rotations.jl 100.00% <ø> (ø)
src/unitquaternion.jl 97.46% <ø> (-0.10%) :arrow_down:
src/rotation_between.jl 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

juliohm commented 1 year ago

What about making this helper function a new rotation type instead? Maybe a simple Between(u, v)? That would avoid using the naming convention with underscore.

hyrodium commented 1 year ago

What about making this helper function a new rotation type instead? Maybe a simple Between(u, v)? That would avoid using the naming convention with underscore.

What's the benefit of the new type Between? The role of rotation_between is similar to range. These functions return various types which depend on their inputs.

julia> range(1,2,3)
1.0:0.5:2.0

julia> range(1,3)
1:3

julia> range(1,2,3) |> typeof
StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}

julia> range(1,3) |> typeof
UnitRange{Int64}
julia> rotation_between(SVector(1,2), SVector(2,1))
2×2 Angle2d{Float64} with indices SOneTo(2)×SOneTo(2)(-0.643501):
  0.8  0.6
 -0.6  0.8

julia> rotation_between(SVector(1,2,3), SVector(2,1,3))
3×3 QuatRotation{Float64} with indices SOneTo(3)×SOneTo(3)(QuaternionF64(0.981981, 0.109109, 0.109109, -0.109109)):
  0.952381  0.238095   0.190476
 -0.190476  0.952381  -0.238095
 -0.238095  0.190476   0.952381

julia> rotation_between(SVector(1,2), SVector(2,1)) |> typeof
Angle2d{Float64}

julia> rotation_between(SVector(1,2,3), SVector(2,1,3)) |> typeof
QuatRotation{Float64}

That would avoid using the naming convention with underscore.

Julia style guide, blue style, and sciml style recommend using snake_case, so there's no problem with underscores.

juliohm commented 1 year ago

Makes sense. Feel free to ignore the suggestion. Looking forward to seeing it merged.

Em dom., 14 de mai. de 2023 12:30, Yuto Horikawa @.***> escreveu:

What about making this helper function a new rotation type instead? Maybe a simple Between(u, v)? That would avoid using the naming convention with underscore.

What's the benefit of the new type Between? The role of rotation_between is similar to range. These functions return various types which depend on their inputs.

julia> range(1,2,3)1.0:0.5:2.0

julia> range(1,3)1:3

julia> range(1,2,3) |> typeof StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}

julia> range(1,3) |> typeof UnitRange{Int64}

julia> rotation_between(SVector(1,2), SVector(2,1))2×2 Angle2d{Float64} with indices SOneTo(2)×SOneTo(2)(-0.643501): 0.8 0.6 -0.6 0.8

julia> rotation_between(SVector(1,2,3), SVector(2,1,3))3×3 QuatRotation{Float64} with indices SOneTo(3)×SOneTo(3)(QuaternionF64(0.981981, 0.109109, 0.109109, -0.109109)): 0.952381 0.238095 0.190476 -0.190476 0.952381 -0.238095 -0.238095 0.190476 0.952381

julia> rotation_between(SVector(1,2), SVector(2,1)) |> typeof Angle2d{Float64}

julia> rotation_between(SVector(1,2,3), SVector(2,1,3)) |> typeof QuatRotation{Float64}

That would avoid using the naming convention with underscore.

Julia style guide https://docs.julialang.org/en/v1/manual/style-guide/, blue style https://github.com/invenia/BlueStyle, and sciml style https://github.com/SciML/SciMLStyle recommend using snake_case, so there's no problem with underscores.

— Reply to this email directly, view it on GitHub https://github.com/JuliaGeometry/Rotations.jl/pull/257#issuecomment-1546925430, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3IMDBSIHRZ6M5IT6HDXGD3CBANCNFSM6AAAAAAXXEETEA . You are receiving this because you commented.Message ID: @.***>