Project

General

Profile

Actions

Pull request #164

closed

Update HPP to 4.1

Added by Guilhem Saurel almost 6 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Repository URL:
https://github.com/nim65s/robotpkg
Repository branch:
hpp41

Description

Hi,

Here are some commits to update HPP to v4.1:
https://github.com/nim65s/robotpkg/commits/hpp41

Could you review it ?

I'm especially not proud of the patch that add qpOASES.pc, so if anyone has a better idea, welcome !


Files

build.log (38.4 KB) build.log Anthony Mallet, 2018-06-06 15:35
Actions #1

Updated by Anthony Mallet almost 6 years ago

I'm especially not proud of the patch that add qpOASES.pc, so if
anyone has a better idea, welcome !

Yes, you cannot do that - it won't work if the qpOASES is installed by
other means than robotpkg (e.g. manually, or system package, or ...).

Why not just define a QPOASES_PREFIX in the CMakeFiles that need
qpOASES, use find_library, and let robotpkg or users define it at
configure time? (maybe with a reasonable default like
CMAKE_PREFIX_PATH).

Actions #2

Updated by Guilhem Saurel almost 6 years ago

Le 31/05/2018 à 19:12, Anthony Mallet a écrit :

Why not just define a QPOASES_PREFIX in the CMakeFiles that need
qpOASES, use find_library, and let robotpkg or users define it at
configure time? (maybe with a reasonable default like
CMAKE_PREFIX_PATH).

That's better, thanks. I added this in hpp-constraints' patch-aa, and
updated the branch hpp41.

Actions #3

Updated by Guilhem Saurel almost 6 years ago

JFYI, I just fixed a few other stuff in some PLIST, and updated this branch again.

Actions #4

Updated by Anthony Mallet almost 6 years ago

Still having issues:

  • Trivial: hpp_tutorial -> hpp-tutorial. The packages names should
    not have `_' to match debian policy (not required by robotpkg
    itself, but better for consistency).
  • Triggered by pinocchio-1.2.8, in
    include/pinocchio/spatial/explog.hpp line 42, where Eigen::internal
    is used.

template <typename D> Eigen::Matrix<
typename D::Scalar,3,1,
Eigen::internal::traits<D>::Options>

I don't see why this is necessary, it should probably be dropped since
it's used only to create an identity matrix. At least it breaks with
eigen-3.3.4, which says:
error: 'Options' is not a member of 'Eigen::internal::traits<blah blah

Actions #6

Updated by Guilhem Saurel almost 6 years ago

Hi,

Thanks for the review !

I updated the branch with a `PKGBASE= $(subst _,-,${HPP_PACKAGE})` in meta-pkgs/hpp/Makefile.common.

I forwarded pinocchio's issue there: https://github.com/stack-of-tasks/pinocchio/issues/471

Actions #7

Updated by Anonymous almost 6 years ago

I tried Pinocchio with Eigen 3.3.4 and did not encounter any compilation issue due to Eigen::internal::traits<D>::Options.
Anthony, can you give much more details on which OS and packages this error appears?

Actions #8

Updated by Anthony Mallet almost 6 years ago

On Wednesday 6 Jun 2018, at 14:46, Justin Carpentier wrote:

I tried Pinocchio with Eigen 3.3.4 and did not encounter any
compilation issue due to Eigen::internal::traits<D>::Options.
Anthony, can you give much more details on which OS and packages this
error appears?

Attached is the build log of hpp-pinocchio-4.1 using eigen-3.3.4.

But in any case, using undocumented stuff from eigen is bound to break
sooner or later. Why not just write the se3::exp3() pinocchio function
using MatrixBase as documented in the eigen manual?

Actions #9

Updated by Anthony Mallet almost 6 years ago

Note that the Jlog3() function in the same file (include/pinocchio/spatial/explog.hpp) does the right thing.

Actions #10

Updated by Guilhem Saurel over 5 years ago

Hi,

I think the issue about pinocchio & Eigen::internal can either be handled in a patch if someone wants to write it, or left behind as an upstream issue.

Is there anything else left holding this PR ?

Actions #11

Updated by Anthony Mallet over 5 years ago

OK. I have a local patch for math/pinocchio. It's currently limited to
patching the exp3() method, but could be extended on demand. So far so
good.

I also added local patches for hpp-constraints, for compat with C++11
and boost::list_of. Painful, but no big deal.

Next issue is hpp-model. It's requiring hpp-fcl04-0.4.2, which conflicts
with hpp-fcl-0.5 required by pinocchio :)

Any idea?

Actions #12

Updated by Guilhem Saurel over 5 years ago

hpp-model is obsolete and can be removed. I thought it was already done, sorry :/

Actions #13

Updated by Guilhem Saurel over 5 years ago

Also, we can probably remove hpp-fcl04 now

Actions #14

Updated by Anthony Mallet over 5 years ago

Now there are issues with optimization/roboptim-core-cfsqp-plugin

It's failing badly with "undefined method" errors ...

Actions #15

Updated by Guilhem Saurel over 5 years ago

We did not updated roboptim plugins with roboptim-core & roboptim-trajectory, because I don't have access to the sources, they are not included in bulk builds, and about CFSQP, even the upstream website is down, according to http://roboptim.net/solvers.html

I don't really know what to do here.

Actions #16

Updated by Anthony Mallet over 5 years ago

On Wednesday 11 Jul 2018, at 19:07, Guilhem Saurel wrote:

We did not updated roboptim plugins with roboptim-core &
roboptim-trajectory, because I don't have access to the sources, they
are not included in bulk builds, and about CFSQP, even the upstream
website is down, according to http://roboptim.net/solvers.html

I don't really know what to do here.

If the site is down, given that this was a proprietary software,
nobody is able to install it then. So it should probably be dropped as
well.

But how did you test this then?

Actions #17

Updated by Guilhem Saurel over 5 years ago

I had no issues with it, because there are no current package relying on this plugin.

My test was to install hpp_tutorial, which depends (directly or indirectly) on everything that we need.

A grep shows that hpp-walkfootplanner still depends on roboptim-core-cfsqp-plugin, but hpp-walkfootplanner is also obsolete.

I am trying to make a graph of all the pakages we need: https://homepages.laas.fr/gsaurel/graph.svg (public) / http://rainboard.laas.fr/graph.svg (only accessible from LAAS, but can be more up-to-date)

I can also see other obsolete packages in path:
hpp-dynamic-obstacle, hpp-environment-data, hpp-fcl04, hpp-gik, hpp-hik, hpp-kwsio, hpp-kwsplus, hpp-localstepper, hpp-model, hpp-model-urdf, hpp-worldmodelplanner, jrl-walkgen

I thought I already had done a list of obsolete HPP packages that we had to remove, but clearly, those ones are still in this branch :/

Actions #18

Updated by Anthony Mallet over 5 years ago

On Thursday 12 Jul 2018, at 09:53, Guilhem Saurel wrote:

I had no issues with it, because there are no current package relying
on this plugin.

Well, meta-pkgs/hpp depends on path/hpp-walkfootplanner

A grep shows that hpp-walkfootplanner still depends on
roboptim-core-cfsqp-plugin, but hpp-walkfootplanner is also
obsolete.

OK, so I'll remove it from meta-pkgs/hpp as well.

I can also see other obsolete packages in path:
hpp-dynamic-obstacle,

Obselete packages need not to be removed as long as they cause no
problem. (it's actually even better to keep them).

But ok, I can remove those:

hpp-environment-data, hpp-fcl04, hpp-gik, hpp-hik, hpp-kwsio,
hpp-kwsplus, hpp-localstepper, hpp-model, hpp-model-urdf,
hpp-worldmodelplanner, jrl-walkgen

Actions #19

Updated by Anthony Mallet over 5 years ago

I added a number of fixes, mostly fixing the API of eigen >= 3.2.2,
removing some doxygen stuff that makes doxygen segfault.

You should probably check that this did not introduce software bugs.

Actions #20

Updated by Guilhem Saurel over 5 years ago

Thanks a lot !

About dynamic-graph-corba, I can confirm that it is also obsolete, so we can remove it too.

Actions #21

Updated by Anthony Mallet over 5 years ago

  • Status changed from New to Closed

On Friday 20 Jul 2018, at 15:17, Guilhem Saurel wrote:

About dynamic-graph-corba, I can confirm that it is also obsolete, so
we can remove it too.

OK, I will do that.

Thanks for the updates!

Actions

Also available in: Atom PDF