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

Change auto diff jacobian storage order when vertex dimension is 1 #725

Merged
merged 2 commits into from
Oct 21, 2023

Conversation

otamachan
Copy link
Contributor

Hi,
When I use audo diff with a one dimension vertex, Eigen generates a static assert and compilation fails.
It seems that Eigen does now allow Eigen::RowMajor if the column size is 1.
This patch changes the jacobian storage order if the vertex dimension is 1 to avoid this error.

Here is a reproducable example

#include <g2o/core/auto_differentiation.h>
#include <g2o/core/base_unary_edge.h>
#include <g2o/types/slam2d/vertex_se2.h>

#include <Eigen/Core>
#include <vector>

class VertexTest : public g2o::BaseVertex<1, double> {};

class EdgeTest
    : public g2o::BaseUnaryEdge<2, Eigen::Vector2d, VertexTest> {
 public:
  EIGEN_MAKE_ALIGNED_OPERATOR_NEW
  EdgeTest() {}
  template <typename T>
  bool operator()(const T *v, T *error) const {
    return true;
  }

  G2O_MAKE_AUTO_AD_FUNCTIONS_BY_GET
  bool read(std::istream &) override { return true; }
  bool write(std::ostream &os) const override { return os.good(); }
};

int main() {
}
/usr/include/eigen3/Eigen/src/Core/PlainObjectBase.h: In instantiation of 'static void Eigen::PlainObjectBase<Derived>::_check_template_params() [with Derived = Eigen::Matrix<double, 2, 1, 1, 2, 1>]':
/usr/include/eigen3/Eigen/src/Core/Matrix.h:261:35:   required from 'Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::Matrix() [with _Scalar = double; int _Rows = 2; int _Cols = 1; int _Options = 1; int _MaxRows = 2; int _MaxCols = 1]'
/usr/include/c++/9/tuple:123:22:   required from 'constexpr std::_Head_base<_Idx, _Head, false>::_Head_base() [with long unsigned int _Idx = 0; _Head = Eigen::Matrix<double, 2, 1, 1, 2, 1>]'
/usr/include/c++/9/tuple:340:15:   required from 'static void g2o::AutoDifferentiation<Edge, EstimateAccess>::linearizeOplusNs(Edge*, std::index_sequence<Ints ...>) [with long unsigned int ...Ints = {0}; Edge = EdgeTest; EstimateAccess = g2o::EstimateAccessorGet<EdgeTest>; std::index_sequence<Ints ...> = std::integer_sequence<long unsigned int, 0>]'
third_party/g2o/g2o/core/auto_differentiation.h:180:21:   required from 'static void g2o::AutoDifferentiation<Edge, EstimateAccess>::linearize(Edge*) [with Edge = EdgeTest; EstimateAccess = g2o::EstimateAccessorGet<EdgeTest>]'
test.cpp:20:3:   required from here
/usr/include/eigen3/Eigen/src/Core/PlainObjectBase.h:903:7: error: static assertion failed: INVALID_MATRIX_TEMPLATE_PARAMETERS
  903 |       EIGEN_STATIC_ASSERT((EIGEN_IMPLIES(MaxRowsAtCompileTime==1 && MaxColsAtCompileTime!=1, (Options&RowMajor)==RowMajor)
      |       ^~~~~~~~~~~~~~~~~~~
make[2]: *** [CMakeFiles/test.dir/build.make:63: CMakeFiles/test.dir/src/test.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:1222: CMakeFiles/test.dir/all] Error 2
make: *** [Makefile:152: all] Error 2

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #725 (8b94ea9) into master (babd2c7) will not change coverage.
Report is 3 commits behind head on master.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #725   +/-   ##
=======================================
  Coverage   50.96%   50.96%           
=======================================
  Files         271      271           
  Lines       10986    10986           
=======================================
  Hits         5599     5599           
  Misses       5387     5387           
Files Coverage Δ
g2o/core/auto_differentiation.h 78.94% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@RainerKuemmerle RainerKuemmerle merged commit 5196040 into RainerKuemmerle:master Oct 21, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants