JacobiSVD/BDCSVD cannot be used with scalars that does not support implicit type conversion.

Submitted by Shuntaro Yamazaki

Assigned to Nobody

Link to original bugzilla bug (#1403)
Version: 3.3 (current stable)

Description

Created attachment 781
patch to JacobiSVD

Build fails if JacobiSVD is instantiated with scalar types that does not support implicit conversion from int/float/double/etc. A typical example is the combination with ceres::Jet in ceres-solver (http://ceres-solver.org/).

$ cat a.cpp
#include <Eigen/Dense>
#include <ceres/jet.h>
int main() {
typedef Eigen::Matrix<ceres::Jet<double, 1>, Eigen::Dynamic, Eigen::Dynamic> Mat;
Mat m(1, 1);
Eigen::JacobiSVD<Mat> e(m);
return 0;
}

$ clang++ -Ieigen -Iceres a.cpp
./Eigen/src/QR/ColPivHouseholderQR.h:556:41: error: invalid operands to binary expression ('Scalar' (aka 'ceres::Jet<double, 1>') and 'int')
if (m_colNormsUpdated.coeffRef(j) != 0) { [<=======(1)]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
./Eigen/src/QR/ColPivHouseholderQR.h:559:25: error: incompatible operand types ('int' and 'RealScalar' (aka 'Jet<double, 1>'))
temp = temp < 0 ? 0 : temp; [<=======(2)]
^ ~ ~~~~
(more errors)

---------- * ---------- * ----------

Among the errors, the error like (1) can be solved by defining operator!=() as follows:

namespace ceres {
template <typename T, typename S, int N, std::enable_if_t<std::is_arithmetic<S>::value, std::nullptr_t> = nullptr>
bool operator!=(const Jet<T,N>& jet, const S& scalar) {
return jet.a != scalar;
}
} [<=======(3)]

The error like (2), however, may not be solved without adding an implicit constructor to 'RealScalar' class; i.e. We need to modify the code in either eigen or ceres-solver.

I woule propose to fix this issue in Eigen. The rationale is as follows: Implicit type conversion is strictly prohibited in many projects including ceres-solver due to unexpected data loss and overlooked runtime cost. On the other hand, Eigen::AutoDiffScalar does not fully support dynamically-sized object (http://eigen.tuxfamily.org/bz/show_bug.cgi?id=1281) whereas ceres::Jet does. So replacing ceres::Jet with Eigen::AutoDiffScalar is not an option for now. Finally, nothing is bad about avoiding implicit type conversion.

Attached are the patch to JacobiSVD. Note that even after applying these patches, the compile of 'a.cpp' above will fail. To pass the build, we need to define many operators like (3), or apply more patches to the code in Eigen to completely eliminate implicit type conversions from scalar to RealScalar. The same issue exists in BDCSVD.

Patch 781, "patch to JacobiSVD":
patch1.diff