Skip to content

Commit

Permalink
Remove unnecessary type conversion inside qrotation (#38)
Browse files Browse the repository at this point in the history
* Add failing regression test for #8 (comment)

* Remove unnecessary type conversion.

Addresses #8.

* minor: simplify the "test this doesn't crash" logic

Co-authored-by: Seth Axen <seth.axen@gmail.com>

Co-authored-by: Seth Axen <seth.axen@gmail.com>
  • Loading branch information
bzinberg and sethaxen authored Feb 19, 2022
1 parent 5a2bb75 commit 23a8212
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
5 changes: 2 additions & 3 deletions src/Quaternion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,8 @@ function qrotation(axis::Vector{T}, theta) where {T <: Real}
error("Must be a 3-vector")
end
u = normalize(axis)
thetaT = convert(eltype(u), theta)
s = sin(thetaT / 2)
Quaternion(cos(thetaT / 2), s * u[1], s * u[2], s * u[3], true)
s = sin(theta / 2)
Quaternion(cos(theta / 2), s * u[1], s * u[2], s * u[3], true)
end

# Variant of the above where norm(rotvec) encodes theta
Expand Down
9 changes: 9 additions & 0 deletions test/test_Quaternion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ let # test rotations
@test angle(qrotation([0, 1, 0], pi / 4)) pi / 4
@test angle(qrotation([0, 0, 1], pi / 2)) pi / 2

# Regression test for
# https://github.com/JuliaGeometry/Quaternions.jl/issues/8#issuecomment-610640094
struct MyReal <: Real
val::Real
end
Base.:(/)(a::MyReal, b::Real) = a.val / b
# this used to throw an error
qrotation([1, 0, 0], MyReal(1.5))

let # test numerical stability of angle
ax = randn(3)
for θ in [1e-9, π - 1e-9]
Expand Down

0 comments on commit 23a8212

Please sign in to comment.