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

Cayley Rot3 fixes #597

Merged
merged 10 commits into from
Nov 13, 2020
2 changes: 2 additions & 0 deletions .github/scripts/unix.sh
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ function configure()
-DGTSAM_BUILD_EXAMPLES_ALWAYS=${GTSAM_BUILD_EXAMPLES_ALWAYS:-ON} \
-DGTSAM_ALLOW_DEPRECATED_SINCE_V41=${GTSAM_ALLOW_DEPRECATED_SINCE_V41:-OFF} \
-DGTSAM_USE_QUATERNIONS=${GTSAM_USE_QUATERNIONS:-OFF} \
-DGTSAM_ROT3_EXPMAP=${GTSAM_ROT3_EXPMAP:-ON} \
-DGTSAM_POSE3_EXPMAP=${GTSAM_POSE3_EXPMAP:-ON} \
-DGTSAM_BUILD_WITH_MARCH_NATIVE=OFF \
-DBOOST_ROOT=$BOOST_ROOT \
-DBoost_NO_SYSTEM_PATHS=ON \
Expand Down
14 changes: 14 additions & 0 deletions .github/workflows/build-special.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ jobs:
ubuntu-gcc-deprecated,
ubuntu-gcc-quaternions,
ubuntu-gcc-tbb,
ubuntu-cayleymap,
]

build_type: [Debug, Release]
Expand All @@ -47,6 +48,12 @@ jobs:
version: "9"
flag: tbb

- name: ubuntu-cayleymap
os: ubuntu-18.04
compiler: gcc
version: "9"
flag: cayley

steps:
- name: Checkout
uses: actions/checkout@master
Expand Down Expand Up @@ -109,6 +116,13 @@ jobs:
echo "GTSAM_WITH_TBB=ON" >> $GITHUB_ENV
echo "GTSAM Uses TBB"

- name: Use Cayley Transform for Rot3
if: matrix.flag == 'cayley'
run: |
echo "GTSAM_POSE3_EXPMAP=OFF" >> $GITHUB_ENV
echo "GTSAM_ROT3_EXPMAP=OFF" >> $GITHUB_ENV
echo "GTSAM Uses Cayley map for Rot3"

- name: Build & Test
run: |
bash .github/scripts/unix.sh -t
9 changes: 9 additions & 0 deletions cmake/HandleGeneralOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ if(NOT MSVC AND NOT XCODE_VERSION)
option(GTSAM_BUILD_WITH_CCACHE "Use ccache compiler cache" ON)
endif()

# Enable GTSAM_ROT3_EXPMAP if GTSAM_POSE3_EXPMAP is enabled, and vice versa.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit problematic as it is done "silently". Add a message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally it should be one flag 🙁 but I need to consult you before making that design change.

if(GTSAM_POSE3_EXPMAP)
message(STATUS "GTSAM_POSE3_EXPMAP=ON, enabling GTSAM_ROT3_EXPMAP as well")
set(GTSAM_ROT3_EXPMAP 1 CACHE BOOL "" FORCE)
elseif(GTSAM_ROT3_EXPMAP)
message(STATUS "GTSAM_ROT3_EXPMAP=ON, enabling GTSAM_POSE3_EXPMAP as well")
set(GTSAM_POSE3_EXPMAP 1 CACHE BOOL "" FORCE)
endif()

# Options relating to MATLAB wrapper
# TODO: Check for matlab mex binary before handling building of binaries
option(GTSAM_INSTALL_MATLAB_TOOLBOX "Enable/Disable installation of matlab toolbox" OFF)
Expand Down
12 changes: 11 additions & 1 deletion gtsam/geometry/Rot3M.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,17 @@ Vector3 Rot3::CayleyChart::Local(const Rot3& R, OptionalJacobian<3,3> H) {
if (H) throw std::runtime_error("Rot3::CayleyChart::Local Derivative");
// Create a fixed-size matrix
Matrix3 A = R.matrix();
// Mathematica closed form optimization (procrastination?) gone wild:

// Check if (A+I) is invertible. Same as checking for -1 eigenvalue.
if ((A + I_3x3).determinant() == 0.0) {
throw std::runtime_error("Rot3::CayleyChart::Local Invalid Rotation");
}

// Mathematica closed form optimization.
// The following are the essential computations for the following algorithm
// 1. Compute the inverse of P = (A+I), using a closed-form formula since P is 3x3
// 2. Compute the Cayley transform C = 2 * P^{-1} * (A-I)
// 3. C is skew-symmetric, so we pick out the computations corresponding only to x, y, and z.
const double a = A(0, 0), b = A(0, 1), c = A(0, 2);
const double d = A(1, 0), e = A(1, 1), f = A(1, 2);
const double g = A(2, 0), h = A(2, 1), i = A(2, 2);
Expand Down
6 changes: 3 additions & 3 deletions gtsam/geometry/tests/testPose3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -906,9 +906,9 @@ TEST(Pose3 , ChartDerivatives) {
Pose3 id;
if (ROT3_DEFAULT_COORDINATES_MODE == Rot3::EXPMAP) {
CHECK_CHART_DERIVATIVES(id,id);
// CHECK_CHART_DERIVATIVES(id,T2);
// CHECK_CHART_DERIVATIVES(T2,id);
// CHECK_CHART_DERIVATIVES(T2,T3);
CHECK_CHART_DERIVATIVES(id,T2);
CHECK_CHART_DERIVATIVES(T2,id);
CHECK_CHART_DERIVATIVES(T2,T3);
}
}

Expand Down
4 changes: 4 additions & 0 deletions gtsam_unstable/geometry/tests/testSimilarity3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,12 @@ TEST(Similarity3, AlignPose3) {

vector<Pose3Pair> correspondences{bTa1, bTa2};

// Cayley transform cannot accommodate 180 degree rotations,
// hence we only test for Expmap
#ifdef GTSAM_ROT3_EXPMAP
Similarity3 actual_aSb = Similarity3::Align(correspondences);
EXPECT(assert_equal(expected_aSb, actual_aSb));
#endif
}

//******************************************************************************
Expand Down