borglab / gtsam

GTSAM is a library of C++ classes that implement smoothing and mapping (SAM) in robotics and vision, using factor graphs and Bayes networks as the underlying computing paradigm rather than sparse matrices.
http://gtsam.org
Other
2.54k stars 752 forks source link

ShonanAveraging compile error? #689

Closed ToniRV closed 3 years ago

ToniRV commented 3 years ago

Sorry for the badly formatted issue. I think I got an error on the ShonanAveraging.cpp side

error: no match for ‘operator-’ (operand types are ‘const ScalarMultipleReturnType {aka const Eigen::CwiseUnaryOp<Eigen::internal::scalar_multiple_op<double>, const Eigen::CwiseNullaryOp<Eigen::internal::scalar_identity_op<double>, Eigen::Matrix<double, -1, -1> > >}’ and ‘const Sparse {aka const Eigen::SparseMatrix<double>}’)
   const Sparse C = pmEigenValue * Matrix::Identity(A.rows(), A.cols()) - A;

That's probably from Frank's latest paper?

/home/tonirv/Code/gtsam/gtsam/sfm/ShonanAveraging.cpp: In function ‘bool gtsam::PowerMinimumEigenValue(const Sparse&, const Matrix&, double&, gtsam::Vector*, std::size_t*, std::size_t, double)’:
/home/tonirv/Code/gtsam/gtsam/sfm/ShonanAveraging.cpp:556:72: error: no match for ‘operator-’ (operand types are ‘const ScalarMultipleReturnType {aka const Eigen::CwiseUnaryOp<Eigen::internal::scalar_multiple_op<double>, const Eigen::CwiseNullaryOp<Eigen::internal::scalar_identity_op<double>, Eigen::Matrix<double, -1, -1> > >}’ and ‘const Sparse {aka const Eigen::SparseMatrix<double>}’)
   const Sparse C = pmEigenValue * Matrix::Identity(A.rows(), A.cols()) - A;
                                                                        ^
In file included from /usr/include/eigen3/Eigen/src/Core/MatrixBase.h:126:0,
                 from /usr/include/eigen3/Eigen/Core:345,
                 from /home/tonirv/Code/gtsam/gtsam/3rdparty/Spectra/SymEigsSolver.h:10,
                 from /home/tonirv/Code/gtsam/gtsam/sfm/ShonanAveraging.cpp:19:
/usr/include/eigen3/Eigen/src/plugins/CommonCwiseUnaryOps.h:50:1: note: candidate: const NegativeReturnType Eigen::MatrixBase<Derived>::operator-() const [with Derived = Eigen::CwiseUnaryOp<Eigen::internal::scalar_multiple_op<double>, const Eigen::CwiseNullaryOp<Eigen::internal::scalar_identity_op<double>, Eigen::Matrix<double, -1, -1> > >; Eigen::MatrixBase<Derived>::NegativeReturnType = Eigen::CwiseUnaryOp<Eigen::internal::scalar_opposite_op<double>, const Eigen::CwiseUnaryOp<Eigen::internal::scalar_multiple_op<double>, const Eigen::CwiseNullaryOp<Eigen::internal::scalar_identity_op<double>, Eigen::Matrix<double, -1, -1> > > >; typename Eigen::internal::traits<T>::Scalar = double]
 operator-() const { return NegativeReturnType(derived()); }

Removing all Shonan* files makes gtsam compile just fine. @dellaert Using Ubuntu 16.04 :man_shrugging: if that helps

dellaert commented 3 years ago

Hmmm, this should be possible, and this compiles fine on my Mac. It also survived 23 CI runs.

BTW, @jingwuOUO, I hope we're sure here that we do not create a very large dense matrix, right? I think Eigen does the right thing with expression templates, but we better be sure :-)

@ToniRV, the expression templates might also be the issue for compilation. No good idea what to do here except try a different expression template: try

const Sparse C = Matrix::Identity(A.rows(), A.cols()) * pmEigenValue- A;

?

ToniRV commented 3 years ago

Ah, cleaning and re-building solves the issue... Sorry for the noise!

ToniRV commented 3 years ago

Cleaning and re-building resets the Eigen used to the one bundled in GTSAM (3.3.7)... but when using my system-wide Eigen (3.2.92) it breaks with:

[ 18%] Building CXX object gtsam/CMakeFiles/gtsam.dir/sfm/ShonanAveraging.cpp.o
/home/tonirv/Code/gtsam/gtsam/sfm/ShonanAveraging.cpp: In function ‘bool gtsam::PowerMinimumEigenValue(const Sparse&, const Matrix&, double&, gtsam::Vector*, std::size_t*, std::size_t, double)’:
/home/tonirv/Code/gtsam/gtsam/sfm/ShonanAveraging.cpp:557:71: error: no match for ‘operator-’ (operand types are ‘const ScalarMultipleReturnType {aka const Eigen::CwiseUnaryOp<Eigen::internal::scalar_multiple_op<double>, const Eigen::CwiseNullaryOp<Eigen::internal::scalar_identity_op<double>, Eigen::Matrix<double, -1, -1> > >}’ and ‘const Sparse {aka const Eigen::SparseMatrix<double>}’)
   const Sparse C = Matrix::Identity(A.rows(), A.cols()) * pmEigenValue- A;

gtsam/CMakeFiles/gtsam.dir/build.make:2726: recipe for target 'gtsam/CMakeFiles/gtsam.dir/sfm/ShonanAveraging.cpp.o' failed
make[2]: *** [gtsam/CMakeFiles/gtsam.dir/sfm/ShonanAveraging.cpp.o] Error 1
 Use System Eigen                                 : ON (Using version: 3.2.92) <-- This doesn't work!
 Use System Eigen                                 : OFF (Using version: 3.3.7) <-- This works.

I think this could be avoided without the user having to update Eigen? Removing all Shonan* files makes gtsam compile just fine.

dellaert commented 3 years ago

Ah. How could it be avoided? Did you try the other order I suggested?

ToniRV commented 3 years ago

The change of order didn't work. It seems more like an incompatibility between Dense and Sparse matrices...

ToniRV commented 3 years ago

Ok, as trivial as it may sound, I had to tell Eigen that the identity matrix is sparse :smile: const Sparse C = pmEigenValue * Matrix::Identity(A.rows(), A.cols()).sparseView() - A; Let's see if CI doesn't complain.

dellaert commented 3 years ago

Nice!

ToniRV commented 3 years ago

690 it works!

shmily53 commented 5 months ago

@ToniRV This kind of fix doesn't seem to cover all scenarios. When the eigen library globally defines EIGEN_DEFAULT_TO_ROW_MAJOR will compile error "error: ambiguous partial specializations of 'cwie_promote_storage_order<Eigen::Sparse, Eigen::Sparse, 1 , 0>'"