ipab-slmc / exotica

Extensible Optimization Framework
https://ipab-slmc.github.io/exotica
BSD 3-Clause "New" or "Revised" License
149 stars 70 forks source link

Build failure #737

Closed Tobias-Fischer closed 3 years ago

Tobias-Fischer commented 3 years ago

Hi there,

We are trying to build exotica in the RoboStack project: https://github.com/RoboStack/ros-noetic

We are getting the following build error:

$SRC_DIR/ros-noetic-exotica-core/src/work/src/problems/bounded_time_indexed_problem.cpp: In member function 'virtual void exotica::BoundedTimeIndexedProblem::Update(Eigen::VectorXdRefConst, int)':
$SRC_DIR/ros-noetic-exotica-core/src/work/src/problems/bounded_time_indexed_problem.cpp:122:176: error: call of overloaded 'Update(__gnu_cxx::__alloc_traits<std::allocator<Eigen::Matrix<double, -1, 1> >, Eigen::Matrix<double, -1, 1> >::value_type&, Eigen::DenseBase<Eigen::Matrix<double, -1, 1> >::SegmentReturnType, Eigen::DenseBase<Eigen::Matrix<double, -1, -1> >::RowsBlockXpr)' is ambiguous
  122 |                 tasks_[i]->Update(x[t], Phi[t].data.segment(tasks_[i]->start, tasks_[i]->length), jacobian[t].middleRows(tasks_[i]->start_jacobian, tasks_[i]->length_jacobian));
      |                                                                                                                                                                                ^
In file included from $SRC_DIR/ros-noetic-exotica-core/src/work/include/exotica_core/planning_problem.h:40,
                 from $SRC_DIR/ros-noetic-exotica-core/src/work/include/exotica_core/problems/abstract_time_indexed_problem.h:35,
                 from $SRC_DIR/ros-noetic-exotica-core/src/work/include/exotica_core/problems/bounded_time_indexed_problem.h:34,
                 from $SRC_DIR/ros-noetic-exotica-core/src/work/src/problems/bounded_time_indexed_problem.cpp:30:
$SRC_DIR/ros-noetic-exotica-core/src/work/include/exotica_core/task_map.h:62:18: note: candidate: 'virtual void exotica::TaskMap::Update(Eigen::VectorXdRefConst, Eigen::VectorXdRef, Eigen::MatrixXdRef)'
   62 |     virtual void Update(Eigen::VectorXdRefConst q, Eigen::VectorXdRef phi, Eigen::MatrixXdRef jacobian);
      |                  ^~~~~~
$SRC_DIR/ros-noetic-exotica-core/src/work/include/exotica_core/task_map.h:68:18: note: candidate: 'virtual void exotica::TaskMap::Update(Eigen::VectorXdRefConst, Eigen::VectorXdRefConst, Eigen::VectorXdRef)'
   68 |     virtual void Update(Eigen::VectorXdRefConst x, Eigen::VectorXdRefConst u, Eigen::VectorXdRef phi);
      |                  ^~~~~~

Do you have any idea how to resolve this? It seems to come from this commit: https://github.com/ipab-slmc/exotica/commit/6e68adadb0a7a24337eb278a1858cfb559467011

We also face this error, but I think this is due to a newer version of tinyxml2:

$SRC_DIR/ros-noetic-exotica-core/src/work/src/loaders/xml_loader.cpp:192:70: error: conversion from 'tinyxml2::XMLNode*' to non-scalar type 'tinyxml2::XMLHandle' requested
  192 |     tinyxml2::XMLHandle root_tag = xml_file.RootElement()->FirstChild();
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
wxmerkt commented 3 years ago

Hi @Tobias-Fischer, Thank you for including Exotica in RoboStack and sorry to see you're experiencing a build failure. Since we aren't seeing this on CI, could you please specify:

Thank you!

Tobias-Fischer commented 3 years ago

Hi @wxmerkt, many thanks for your fast reply! Sorry, I should have included this information in the first place.

Please let me know if there is something else that I can help to debug.

wxmerkt commented 3 years ago

Thanks for the additional info! Are there Docker test images or scripts to replicate that is commonly used to debug conda-forge? I've only used conda as a user so far not for packaging and would appreciate pointers.

This is the setting we know working (among others currently tested on CI incl. 16.04 and 18.04):

If there is a GitHub Actions kind of CI you could suggest to add for testing conda forge packaging this may make it easier to debug as well.

Many thanks!

Tobias-Fischer commented 3 years ago

Indeed with Eigen 3.3.7 the error goes away - thanks for the hint. It would be great if we could dig deeper to see how to fix the error with Eigen 3.3.9.

The TinyXML2 error goes away using this little change:

diff --git a/src/loaders/xml_loader.cpp b/src/loaders/xml_loader.cpp
index 4abdc916..05983fbe 100644
--- a/src/loaders/xml_loader.cpp
+++ b/src/loaders/xml_loader.cpp
@@ -189,7 +189,9 @@ void XMLLoader::LoadXML(std::string file_name, Initializer& solver, Initializer&
     }

     std::vector<Initializer> initializers;
-    tinyxml2::XMLHandle root_tag = xml_file.RootElement()->FirstChild();
+    tinyxml2::XMLNode* root_tag_node = xml_file.RootElement()->FirstChild();
+    tinyxml2::XMLHandle root_tag(root_tag_node);
+    //tinyxml2::XMLHandle root_tag = xml_file.RootElement()->FirstChild();
     while (root_tag.ToNode())
     {
         if (root_tag.ToElement() == nullptr)

I am not sure whether this works in TinyXML2 7.0.0, too.

With these changes I can build exotica-core successfully.

/cc @wolfv

wxmerkt commented 3 years ago

Thanks for the pointers and the fix for the TinyXML2 issue. I looked through the Eigen changelog and there was indeed a change to Eigen::Ref in 3.3.8 (https://gitlab.com/libeigen/eigen/-/commit/7b93328baf13c860980665040ac490034821c9c9) though I am yet to identify if that is the change that causes the above issue. Once I get a Docker running to test I can verify possible fixes in our codebase.

wxmerkt commented 3 years ago

I just installed Eigen 3.3.9 on Ubuntu 20.04 system-wide to reproduce the initial Eigen error and it does not trigger - it compiles just fine using the changes for TinyXML2 you identified and we've included in #738. Is there a Dockerfile or GitHub Actions CI we can easily add here so I have a (failing) branch to test possible solutions against? I noticed the RViz-CI suggestion but it seems I would have to identify all system package requirements manually to include in the environment setup? Many thanks :-)

wxmerkt commented 3 years ago

@Tobias-Fischer - I was able to create a succinct reproduction case and find a workaround for both 3.3.7 and 3.3.9. It will break the default version on 16.04/Kinetic so we are going to EOL it early. The fix is in #740.

Reproduction case:

#include <Eigen/Core>
#include <iostream>

// typedef const Eigen::Ref<const Eigen::VectorXd>& VectorXdRefConst;
// typedef const Eigen::Ref<const Eigen::MatrixXd>& MatrixXdRefConst;
typedef Eigen::internal::ref_selector<Eigen::VectorXd>::type VectorXdRefConst;
typedef Eigen::internal::ref_selector<Eigen::MatrixXd>::type MatrixXdRefConst;

typedef typename Eigen::Ref<Eigen::VectorXd> VectorXdRef;
typedef typename Eigen::Ref<Eigen::MatrixXd> MatrixXdRef;

// typedef Eigen::internal::ref_selector<Eigen::VectorXd>::non_const_type VectorXdRef;
// typedef Eigen::internal::ref_selector<Eigen::MatrixXd>::non_const_type MatrixXdRef;

void Update(VectorXdRefConst a, VectorXdRef b, MatrixXdRef c) { std::cout << "Third is Matrix" << std::endl; }
void Update(VectorXdRefConst a, VectorXdRefConst b, VectorXdRef c) { std::cout << "Third is Vector" << std::endl; }

int main()
{
    Eigen::VectorXd x = Eigen::VectorXd::Random(10);
    Eigen::VectorXd u = Eigen::VectorXd::Random(10);
    Eigen::VectorXd phi = Eigen::VectorXd::Random(10);
    Eigen::MatrixXd J = Eigen::MatrixXd::Random(3, 20);

    // working
    Update(x, u, phi);
    Update(x, u, phi.segment(0, 2));

    // workaround #0 is to explicitly instantiate the VectorXdRefConst
    Update(VectorXdRefConst(x), VectorXdRefConst(u), phi.segment(0,2));

    // workaround #1 is to use `eval()`. This may cause temporaries to be created?
    Update(x, phi.segment(0, 2).eval(), J.middleRows(0, 2));

    // broken
    // Update(x, phi.segment(0, 2), J.middleRows(0, 2));
    Update(x, VectorXdRef(phi.segment(0, 2)), J.middleRows(0, 2));  // workaround
    Update(x, phi.segment(0, 2), MatrixXdRef(J.middleRows(0, 2)));  // workaround

    // works
    // Update(x, phi.segment(0, 2).eval(), J.middleRows(0, 2));
    // Update(x, VectorXdRef(phi.segment(0,2)), J.middleRows(0, 2));
}

and you can compile it with:

g++ test.cpp `pkg-config --cflags eigen3`

There are likely more correct ways of handling this in Eigen and I'd be happy to learn about them.

I tried compiling more packages with Conda and next up is the known pybind11_catkin mapping/replacement/release to sort out. I have local workarounds for it (by falling back to pybind11) but it complicates the current CMakeLists.

One issue I noticed is with running ninja install for packages that contain catkin_python_setup / install Python packages using the setup.py from catkin which is described in more detail here: https://github.com/ros/catkin/issues/863

Tobias-Fischer commented 3 years ago

Hi @wxmerkt - that's great news, thanks a lot! Maybe @traversaro has more insights on the Eigen issues?

Re pybind11: Let's chat in https://github.com/RoboStack/ros-noetic/issues/19 Re catkin_python_setup for future reference: see https://github.com/RoboStack/ros-noetic/issues/98

Feel free to join us in https://gitter.im/RoboStack/Lobby

traversaro commented 3 years ago

Maybe @traversaro has more insights on the Eigen issues?

I quickly checked the issue, but to be honest I did not fully grasped what caused the issue, so I guess @wxmerkt workaround is ok!