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

Build failure #737

Closed
Tobias-Fischer opened this issue Apr 6, 2021 · 9 comments
Closed

Build failure #737

Tobias-Fischer opened this issue Apr 6, 2021 · 9 comments

Comments

@Tobias-Fischer
Copy link
Contributor

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: 6e68ada

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
Copy link
Collaborator

wxmerkt commented Apr 6, 2021

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:

  • OS version
  • Compiler and version
  • Eigen version
  • TinyXML version

Thank you!

@Tobias-Fischer
Copy link
Contributor Author

Tobias-Fischer commented Apr 6, 2021

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

  • OS version: conda-forge relies on CentOS6, but all libraries are provided through conda-forge so the OS version should not really matter.
  • Compiler: g++ 9.3.0 (same errors with g++ 7.5.0 and Clang11)
  • Eigen version: 3.3.9
  • TinyXML(1) version: 2.6.2 (although the error is related to TinyXML2 so I don't think this should matter)
  • TinyXML2: 8.0.0

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

@wxmerkt
Copy link
Collaborator

wxmerkt commented Apr 6, 2021

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):

  • Ubuntu 20.04
  • g++ 9.3.0 - same as yours (stock Ubuntu)
  • Eigen version 3.3.7 (stock Ubuntu). Perhaps the treatment of Eigen::Ref<const ...> got more strict with RowVector vs Vector?
  • TinyXML2: 7.0.0 (stock Ubuntu). I suspect the return pointer may have changed in the API between 7 and 8.

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
Copy link
Contributor Author

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 added a commit that referenced this issue Apr 6, 2021
@wxmerkt
Copy link
Collaborator

wxmerkt commented Apr 6, 2021

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
Copy link
Collaborator

wxmerkt commented Apr 6, 2021

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 added a commit that referenced this issue Apr 6, 2021
@wxmerkt
Copy link
Collaborator

wxmerkt commented Apr 7, 2021

@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: ros/catkin#863

@Tobias-Fischer
Copy link
Contributor Author

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

Re pybind11: Let's chat in RoboStack/ros-noetic#19
Re catkin_python_setup for future reference: see RoboStack/ros-noetic#98

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

@traversaro
Copy link

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!

@wxmerkt wxmerkt closed this as completed in a1f8434 Apr 8, 2021
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

No branches or pull requests

3 participants