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

Use internal inverse function instead of Base.inv #274

Closed
juliohm opened this issue Nov 6, 2023 · 2 comments
Closed

Use internal inverse function instead of Base.inv #274

juliohm opened this issue Nov 6, 2023 · 2 comments

Comments

@juliohm
Copy link
Member

juliohm commented Nov 6, 2023

Apparently Base.inv is only intended for multiplicative inverses:

 inv(x)


  Return the multiplicative inverse of x, such that x*inv(x) or inv(x)*x yields
  one(x) (the multiplicative identity) up to roundoff errors.

  If x is a number, this is essentially the same as one(x)/x, but for some types
  inv(x) may be slightly more efficient.

We recently learned about InverseFunctions.jl, which introduces a inverse function for inverse functions. Rotations.jl could use the same function or add an internal name inverse that is not exported by default.

We are fixing all our packages to avoid the misuse of Base.inv.

@hyrodium
Copy link
Collaborator

hyrodium commented Nov 7, 2023

Base.inv is an inverse operation about the binary operator *, and InverseFunctions.inverse is an inverse operation about the binary operator (\circle).

The current usage in this package inv(::Rotation) is related to *, so we don't have to switch to InverseFunctions.inverse.

julia> using Rotations

julia> r = QuatRotation(2,1,0,0)
3×3 QuatRotation{Float64} with indices SOneTo(3)×SOneTo(3)(QuaternionF64(0.894427, 0.447214, 0.0, 0.0)):
 1.0  0.0   0.0
 0.0  0.6  -0.8
 0.0  0.8   0.6

julia> r * inv(r)
3×3 QuatRotation{Float64} with indices SOneTo(3)×SOneTo(3)(QuaternionF64(1.0, 0.0, 0.0, 0.0)):
 1.0  0.0  0.0
 0.0  1.0  0.0
 0.0  0.0  1.0

@juliohm
Copy link
Member Author

juliohm commented Nov 7, 2023 via email

@hyrodium hyrodium closed this as not planned Won't fix, can't repro, duplicate, stale Nov 7, 2023
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