search

JuliaGeometry / Quaternions.jl

A Julia implementation of quaternions
https://juliageometry.github.io/Quaternions.jl
MIT License
115 stars 37 forks source link

fix piracy and type instability #53

Closed mikmoore closed 2 years ago

mikmoore commented 2 years ago

This PR is intended to remove the piracy of LinearAlgebra.normalize(::Vector) that was previously being committed by Quaternions. This piracy would make normalize(zeros(3)) return a vector of 0.0 when it would normally return a vector of NaN.

The guilty method was only used in qrotation(::Vector,::Any) so it was easy enough to remove it and provide the matching calculation in its place. I also noticed that qrotation([0,0,0],1.0) == Quaternion(0.8775825618903728, 0.0, 0.0, 0.0, true), which is wrong in multiple capacities. I have made it an error to use this method with a zero vector in the first argument. A potential alternative would be to force the rotation angle to zero in this case, but I opted for the error since a zero axis seemed more likely to be used in error than deliberately (given that qrotation(::Vector) is available for the latter case). EDIT: An all-zero rotation axis will force the rotation to zero, yielding Quaternion(1.0).

Along the way, I noticed that the related method qrotation(::Vector) was type-unstable for zero inputs and that input vectors with NaN entries failed to propagate their NaNs. Both of these issues are resolved by making it mirror the changes already made to the 2-arg method.

codecov[bot] commented 2 years ago

Codecov Report

Merging #53 (d361d81) into master (88d7057) will increase coverage by 1.95%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   43.62%   45.58%   +1.95%     
==========================================
  Files           3        3              
  Lines         353      351       -2     
==========================================
+ Hits          154      160       +6     
+ Misses        199      191       -8
Impacted Files Coverage Δ
src/Quaternion.jl 85.05% <100.00%> (+4.37%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 88d7057...d361d81. Read the comment docs.