Created attachment 781 [details] 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.
Thank you for the report, fixed: https://bitbucket.org/eigen/eigen/commits/dadabdfa31e3/ (devel) https://bitbucket.org/eigen/eigen/commits/2ce63814ae99/ (3.3)
More fixes for BDVSVD: https://bitbucket.org/eigen/eigen/commits/3e1cd2f29ed0 (devel) https://bitbucket.org/eigen/eigen/commits/e95fa43a6986 (3.3) BTW, note that it actually not always a good idea to convert constants likes 0, 1, 2, ... to the target scalar type prior to arithmetic operations or comparison. For instance, for it is much faster to multiply an arbitrary precision floating point number by an integer than by a float, double or even worse another arbitrary precision floating point number. For auto-diff (say AD<double>), it is also faster to multiply or add one AD<double> by a double rather than operating on two AD<double>. This is why in 3.3 we introduced a NumTraits<>::Literal type to cast such literals as the main scalar type prefer.
-- GitLab Migration Automatic Message -- This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/1403.