Closed timholy closed 10 months ago
Merging #272 (6811364) into master (149edac) will increase coverage by
0.08%
. Report is 2 commits behind head on master. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #272 +/- ##
==========================================
+ Coverage 90.42% 90.50% +0.08%
==========================================
Files 17 17
Lines 1660 1664 +4
==========================================
+ Hits 1501 1506 +5
+ Misses 159 158 -1
Files | Coverage Δ | |
---|---|---|
src/angleaxis_types.jl | 99.00% <100.00%> (+1.07%) |
:arrow_up: |
:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!
A type instability was introduced in this PR.
Before this PR
julia> using Rotations
julia> QuatRotation(RotationVec(0f0, 0f0, 0f0))
3×3 QuatRotation{Float32} with indices SOneTo(3)×SOneTo(3)(QuaternionF32(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
julia> QuatRotation(RotationVec(0f0, 0f0, 2f0))
3×3 QuatRotation{Float32} with indices SOneTo(3)×SOneTo(3)(QuaternionF32(0.540302, 0.0, 0.0, 0.841471)):
-0.416147 -0.909297 0.0
0.909297 -0.416147 0.0
0.0 0.0 1.0
After this PR
julia> using Rotations
julia> QuatRotation(RotationVec(0f0, 0f0, 0f0))
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
julia> QuatRotation(RotationVec(0f0, 0f0, 2f0))
3×3 QuatRotation{Float32} with indices SOneTo(3)×SOneTo(3)(QuaternionF32(0.540302, 0.0, 0.0, 0.841471)):
-0.416147 -0.909297 0.0
0.909297 -0.416147 0.0
0.0 0.0 1.0
By converting straight from RotationVec->QuatRotation, and using an implementation that exploits
sinc
to handle θ ≈ 0, we get continuous & differentiable behavior.Requires https://github.com/JuliaDiff/ForwardDiff.jl/pull/669 before tests will pass.
What this fixes: currently, near the origin the Hessian is zero (which is wrong), because
*(::RotationVec, ::SVector{3})
expands only to linear order. But the fix is aimed at being broader than this specific case (otherwise, we could have just implemented a second-order approximation).Note: please check whether the relaxation of the
adjoint
test is reasonable, perhaps exact equality is necessary? If so I'll need to come up with a fix.