Numerically stable implementation of axis, angle and scaled_axis for Rotation3 and UnitQuaternion#1595
Open
nmayorov wants to merge 7 commits intodimforge:mainfrom
Open
Numerically stable implementation of axis, angle and scaled_axis for Rotation3 and UnitQuaternion#1595nmayorov wants to merge 7 commits intodimforge:mainfrom
nmayorov wants to merge 7 commits intodimforge:mainfrom
Conversation
* Add known-failing unit test case for SymmetricEigen * Fix eigenvector calculation for subdim=2 case in SymmetricEigen
…aternion::scaled_axis
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi and thank you for a great library!
In this PR I wan to resolve couple of issues regarding axis-angle representation of rotations.
Conceptual change
The conceptual idea is that we want to use "scaled_axis" aka "rotation vector" as the core representation, without splitting it into axis and angle in internal computations. This is the numerical stable way to work with such representation and it gracefully handles the case of zero or close to zero rotation angles without introducing special cases, singularities and epsilon tresholds.
What issues does it fix
It fixes the following practical issues and bugs:
scaled_axis, i.e creating rotation fromVector3::new(5.0e-20, 1e-16, -1.0e-17)and then queryingscaled_axisrepresentation will return this vector, not vector of zerosanglereturned asnanfromRotation3due toacosargument going out[-1, 1]bounds after some minor round-off error accumulation (the issue I encountered myself)Rotation3for angles close to pi (example in issue Numerically vulnerable axis calculation in Rotation3 #1382 )Basically all the operations regarding
scaled_axisandangleare 100% robust, theaxisextraction can be controlled by the user (i.e. for how small angles to extract rotation axis).How does it achieves that
The core change is in
Quaternion::expandUnitQuaternion::scaled_axiswhere scaling of the formsin(theta) / thetaand its inverse is implemented viasincwithout any epsilon threshold checking. After thatUnitQuaternionfunctionality is solid.And
Rotationmethods are delegated toUnitQuaternionasrotation matrix -> unit quaternion -> scaled axis. There are not many reasons or savings doing it in any other way, because scaled axis / rotation vector is directly connected to quaternion representation.Code concerns
I had to change trait bound of
TtoRealFieldinRotation3::angle. I'm not an expert in Rust and don't know exact implications of that, but it seems reasonable to do, because all other functions related to axis and angle have this specification.I plan to add more tests on the functionality, meanwhile, hopefully, someone review and consider these changes.
Closes #1382.