add support of CASADI #813

jcarpent · 2019-06-06T18:29:39Z

@mkatliar @jmirabel I've just opened a new PR for checking the integration of Pinocchio inside CASADI. This is still work in progress but I will try to add the different tests that @mkatliar is adding.

Do not hesitate to add your contributions.

Related to #809

jcarpent · 2019-06-06T20:00:23Z

Minor info: I have found some bugs. I will try to push a fix by tomorrow.

jmirabel

As a remark, I think support of external libraries like casADi, AutoDiff, CppAD should not be added this way. I would put all the template specialization code into separate files.

There are good reasons for this:

putting the specialization into the main files prevents user to provide their own specialization, e.g. for an application specific case or to fix a bugged specialization.
it should be provided not as a complete but rather as a start. User may need to complete it. As far as I know (correct me if I am wrong), the main developers of Pinocchio haven't use auto-diff a lot. Using CasADi automatic differentiation is not possible #809 is one example.

src/math/casadi.hpp

jmirabel · 2019-06-07T07:33:02Z

Another good reason is that it would give a good example of how one can use another auto-diff module.

jcarpent · 2019-06-07T07:35:03Z

As a remark, I think support of external libraries like casADi, AutoDiff, CppAD should not be added this way. I would put all the template specialization code into separate files.

There are good reasons for this:
* putting the specialization into the main files prevents user to provide their own specialization, e.g. for an application specific case or to fix a bugged specialization.

* it should be provided not as a complete but rather as a start. User may need to complete it. As far as I know (correct me if I am wrong), the main developers of Pinocchio haven't use auto-diff a lot. #809 is one example.

I do agree with all these points, but I do not have any great solution for that.
Can you suggest something better?

Maybe we can think to something like the unsupported directory of Eigen.
The only thing to keep in mind is that for some reasons, we need to include CppAD headers before Eigen's ones.

jcarpent · 2019-06-07T07:36:12Z

I suggest to first make everything in such a way Pinocchio works fine with Casadi, which requires some works. And to propose a clean way of integrating all AutoDiff frameworks later. It will be easier for me.

jcarpent · 2019-06-07T07:37:35Z

By the way, at least CppAD should be an optional dependency because it is the framework I use to do code generation.

jmirabel · 2019-06-07T07:42:57Z

The only thing to keep in mind is that for some reasons, we need to include CppAD headers before Eigen's ones.

I believe this is an easy task which can be the responsibility of the user. I don't think this is the responsibility of Pinocchio.

Can you suggest something better?

Add a file casadi.hpp anywhere in Pinocchio, with the following properties:

contains all the template specializations.
does not include casadi
add a line which won't compile if casadi is not included, with a comment explaining that the user must include casadi himself, before including any Pinocchio/Eigen headers.

Eventually, it could be a folder with several files.
Would this work ?

jcarpent · 2019-06-07T07:54:21Z

Yes, I guess. I will try to do it after fixing all the unit test for the integration of CASADI.

jmirabel · 2019-06-07T07:56:30Z

Thank you for your work !

mkatliar · 2019-06-07T13:34:04Z

A stupid question, how can I add my contributions to this pull request (instead of creating another one)?

jcarpent · 2019-06-07T14:35:00Z

A stupid question, how can I add my contributions to this pull request (instead of creating another one)?

You're now a collaborator of the project. You should be able to push on the current PR. @mkatliar Thanks for your contributions.

mkatliar · 2019-06-07T14:49:34Z

FYI, I am currently working on making Eigen::LLT work with casadi::SX.

mkatliar · 2019-06-07T16:49:48Z

@jcarpent I never did push to a pull request before, so maybe I am doing something wrong here. I added git@github.com:jcarpent/pinocchio.git as a remote, then checked out topic/casadi, made some changes and committed. Then following these instructions I do:

$ git push origin topic/casadi 
ERROR: Permission to stack-of-tasks/pinocchio.git denied to mkatliar.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

What is wrong?

jcarpent · 2019-06-07T18:00:35Z

You did not accept the invitation to join the project that you should have received few hours ago.

src/math/casadi.hpp

jcarpent · 2019-06-10T07:32:51Z

unittest/casadi-basic.cpp

@@ -52,8 +49,8 @@ BOOST_AUTO_TEST_CASE(test_example)
  {
    for (Eigen::Index j = 0; j < A.cols(); ++j)
    {
-      A(i, j) = 10 * i + j;
-      B(i, j) = -10 * i - j;
+      A(i, j) = 10. * static_cast<double>(i) + static_cast<double>(j);


Did you have some compiler warnings here?

Which compiler are you using?

g++ (Ubuntu 8.3.0-6ubuntu1) 8.3.0

mkatliar · 2019-06-10T07:35:03Z

On my system, I get the following output from cmake when trying to build the topic/casadi branch:

[cmake] eigen3 >= 3.0.5 is required.
[cmake] Checking for module 'eigen3 >= 3.0.5'
[cmake]   Found eigen3 , version 3.3.7
[cmake] Pkg-config module eigen3 v3.3.7 has been detected with success.
[cmake] urdfdom >= 0.2.0 is required.
[cmake] Checking for module 'urdfdom >= 0.2.0'
[cmake]   Found urdfdom , version 1.0.3
[cmake] Pkg-config module urdfdom v1.0.3 has been detected with success.
[cmake] cppad >= 20180000.0 is required.
[cmake] Checking for module 'cppad >= 20180000.0'
[cmake]   Found cppad , version 20190601
[cmake] Pkg-config module cppad v20190601 has been detected with success.
[cmake] casadi >= 3.4.5 is required.
[cmake] Checking for module 'casadi >= 3.4.5'
[cmake]   No package 'casadi' found
[cmake] CMake Error at /usr/share/cmake-3.13/Modules/FindPkgConfig.cmake:452 (message):
[cmake]   A required package was not found
[cmake] Call Stack (most recent call first):
[cmake]   /usr/share/cmake-3.13/Modules/FindPkgConfig.cmake:622 (_pkg_check_modules_internal)
[cmake]   cmake/pkg-config.cmake:298 (PKG_CHECK_MODULES)
[cmake]   cmake/pkg-config.cmake:510 (ADD_DEPENDENCY)
[cmake]   CMakeLists.txt:120 (ADD_REQUIRED_DEPENDENCY)
[cmake] 
[cmake] 
[cmake] Configuring incomplete, errors occurred!
[cmake] See also "/home/mkatliar/software/pinocchio/build/CMakeFiles/CMakeOutput.log".
[cms-driver] Error during CMake configure: [cmake-server] Configuration failed.

But it instead of

ADD_REQUIRED_DEPENDENCY("casadi >= 3.4.5")

I do

find_package(CasADi 3.4.5 REQUIRED)
set(CASADI_FOUND True)

then cmake does not complain.

I have built casadi from sources and installed in /usr/local.

How can we fix this?

src/math/casadi.hpp

jcarpent · 2019-06-10T07:38:43Z

@mkatliar I have a basic question, why do you need to specialize Eigen::LLT, is it because you have some warnings when compiling `pinocchio::aba?

jcarpent · 2019-06-10T07:40:25Z

On my system, I get the following output from cmake when trying to build the topic/casadi branch:

[cmake] eigen3 >= 3.0.5 is required.
[cmake] Checking for module 'eigen3 >= 3.0.5'
[cmake]   Found eigen3 , version 3.3.7
[cmake] Pkg-config module eigen3 v3.3.7 has been detected with success.
[cmake] urdfdom >= 0.2.0 is required.
[cmake] Checking for module 'urdfdom >= 0.2.0'
[cmake]   Found urdfdom , version 1.0.3
[cmake] Pkg-config module urdfdom v1.0.3 has been detected with success.
[cmake] cppad >= 20180000.0 is required.
[cmake] Checking for module 'cppad >= 20180000.0'
[cmake]   Found cppad , version 20190601
[cmake] Pkg-config module cppad v20190601 has been detected with success.
[cmake] casadi >= 3.4.5 is required.
[cmake] Checking for module 'casadi >= 3.4.5'
[cmake]   No package 'casadi' found
[cmake] CMake Error at /usr/share/cmake-3.13/Modules/FindPkgConfig.cmake:452 (message):
[cmake]   A required package was not found
[cmake] Call Stack (most recent call first):
[cmake]   /usr/share/cmake-3.13/Modules/FindPkgConfig.cmake:622 (_pkg_check_modules_internal)
[cmake]   cmake/pkg-config.cmake:298 (PKG_CHECK_MODULES)
[cmake]   cmake/pkg-config.cmake:510 (ADD_DEPENDENCY)
[cmake]   CMakeLists.txt:120 (ADD_REQUIRED_DEPENDENCY)
[cmake] 
[cmake] 
[cmake] Configuring incomplete, errors occurred!
[cmake] See also "/home/mkatliar/software/pinocchio/build/CMakeFiles/CMakeOutput.log".
[cms-driver] Error during CMake configure: [cmake-server] Configuration failed.

But it instead of

ADD_REQUIRED_DEPENDENCY("casadi >= 3.4.5")

I do

find_package(CasADi 3.4.5 REQUIRED)
set(CASADI_FOUND True)

then cmake does not complain.

I have built casadi from sources and installed in /usr/local.

How can we fix this?

I have forked casadi to allow it to handle pkgconfig configuration. I have made a PR so far.
You can see it here: casadi/casadi#2446

mkatliar · 2019-06-10T07:41:59Z

@mkatliar I have a basic question, why do you need to specialize Eigen::LLT, is it because you have some warnings when compiling `pinocchio::aba?

@jcarpent because I have errors when compiling pinocchio::aba with casadi::SX as a scalar type.

mkatliar · 2019-06-10T07:48:05Z

I have forked casadi to allow it to handle pkgconfig configuration. I have made a PR so far.
You can see it here: casadi/casadi#2446

@jcarpent casadi installs its /usr/local/lib/cmake/casadi/casadi-config.cmake file, after which cmake can find casadi automatically. Do we really need to rely on the pkgconfig mechanism here?

jcarpent · 2019-06-10T07:50:55Z

@mkatliar I have a basic question, why do you need to specialize Eigen::LLT, is it because you have some warnings when compiling `pinocchio::aba?

@jcarpent because I have errors when compiling pinocchio::aba with casadi::SX as a scalar type.

This is expected. Some computations involve the usage of Eigen::LLT. I suggest to specialize these computations according to the Scalar types: if the Scalar type is classic (float, double, long double), then we need to call the classic LLT, otherwise, we have to the inversion (without relying on LLT, and without any comparisons).

mkatliar · 2019-06-10T07:53:35Z

This is expected. Some computations involve the usage of Eigen::LLT. I suggest to specialize these computations according to the Scalar types: if the Scalar type is classic (float, double, long double), then we need to call the classic LLT, otherwise, we have to the inversion (without relying on LLT, and without any comparisons).

@jcarpent this makes sense to me. Could you then write the specialized code for these computations, since you are much more familiar with the algorithms? I can help you on the casadi side if you need.

jcarpent · 2019-06-10T07:54:51Z

Yes, I will do the specialization of the algorithms.

jcarpent · 2019-06-10T08:40:54Z

I have forked casadi to allow it to handle pkgconfig configuration. I have made a PR so far.
You can see it here: casadi/casadi#2446

@jcarpent casadi installs its /usr/local/lib/cmake/casadi/casadi-config.cmake file, after which cmake can find casadi automatically. Do we really need to rely on the pkgconfig mechanism here?

A lot of projects (among them Pinocchio) rely on pkg-config for dependency check. I think it would be worthy to add this feature to CASADI, even if, for the time being, we can use the CMake approach.

jcarpent · 2019-06-11T07:43:34Z

@mkatliar I have done most of the job to get Casadi working with Pinocchio.
The remaining thing is to make the clean splitting between autodiff frameworks and Pinocchio core functionalities.

@mkatliar Do you also need the analytical derivatives?

mkatliar · 2019-06-11T19:17:37Z

@mkatliar Do you also need the analytical derivatives?

@jcarpent you mean if I need the abaDerivatives() etc. to work with casadi types? At the moment no, because if aba() works with casadi types then I can obtain the analytical derivatives from casadi.

…space

jcarpent · 2019-06-22T15:00:40Z

@mkatliar I think I'm done with this PR. Everything works well on my computer. Can you try it from your side?

jcarpent · 2019-06-24T09:02:22Z

Following the discussion in #814, I will merge it and make the rebase for #814.

jcarpent mentioned this pull request Jun 6, 2019

Using CasADi automatic differentiation is not possible #809

Closed

jmirabel reviewed Jun 7, 2019

View reviewed changes

src/math/casadi.hpp Outdated Show resolved Hide resolved

jcarpent force-pushed the topic/casadi branch 2 times, most recently from 5972120 to bb7911c Compare June 7, 2019 12:27

jcarpent commented Jun 10, 2019

View reviewed changes

src/math/casadi.hpp Outdated Show resolved Hide resolved

jcarpent added 21 commits June 21, 2019 22:44

math/quaternion: correct include

4f2e072

math: remove useless include

411fb55

math: fwd of TaylorSeriesExpansion

e5c9da4

casadi: correct order of inclusion

49dfc40

test/casadi: correct inclusion order

22c5d7b

core: add if_then_else overload

db1615d

casadi: overload if_then_else

b5ad436

core: fwd of internal::traits<T>

ea28481

cmake: only process dependencies if they have been activated

85a1c94

math/quaternion: remove the use of Eigen::internal::nested

5d80c87

utils: correct ifndef

774e5dc

liegroup: use assignQuaternion

5f17dcb

liegroup: use Scalar(0)

260eaec

liegroup: make compliant the code for casadi

90af3a3

math/quaternion: make compliant the code with casadi

38c583a

spatial/explog: make compliant the code with casadi (use if_then_else)

28f0c83

spatial/explog: use the right operator

7d64a6e

liegroup: adapt exp to work with casadi

8d3e7c1

test/casadi: add test on integrate

97c7a92

test/casadi: test against RNEA derivatives

c72f5a2

spatial/explog: make compliant the code with casadi (use if_then_else)

6b45654

jcarpent force-pushed the topic/casadi branch from 78eceee to 6b45654 Compare June 21, 2019 20:46

jcarpent added 2 commits June 22, 2019 00:41

all: remove SE3GroupAction and MotionAlgebraAction from internal name…

cdb2cbb

…space

test/casadi: add aba derivatives

355275a

jcarpent changed the title ~~WIP: add support of CASADI~~ add support of CASADI Jun 22, 2019

jmirabel mentioned this pull request Jun 24, 2019

Implementation of Mimic joints #814

Merged

jcarpent merged commit 699bbf4 into stack-of-tasks:devel Jun 24, 2019

GiulioRomualdi mentioned this pull request Jul 16, 2020

Investigate the possibility to use Automatic Differentiation in the Framework ami-iit/bipedal-locomotion-framework#74

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support of CASADI #813

add support of CASADI #813

jcarpent commented Jun 6, 2019

jcarpent commented Jun 6, 2019 •

edited

jmirabel left a comment

jmirabel commented Jun 7, 2019

jcarpent commented Jun 7, 2019

jcarpent commented Jun 7, 2019

jcarpent commented Jun 7, 2019

jmirabel commented Jun 7, 2019 •

edited

jcarpent commented Jun 7, 2019

jmirabel commented Jun 7, 2019

mkatliar commented Jun 7, 2019

jcarpent commented Jun 7, 2019

mkatliar commented Jun 7, 2019

mkatliar commented Jun 7, 2019 •

edited

jcarpent commented Jun 7, 2019

jcarpent Jun 10, 2019

mkatliar Jun 10, 2019

jcarpent Jun 10, 2019

mkatliar Jun 13, 2019

mkatliar commented Jun 10, 2019 •

edited

jcarpent commented Jun 10, 2019

jcarpent commented Jun 10, 2019

mkatliar commented Jun 10, 2019 •

edited

mkatliar commented Jun 10, 2019

jcarpent commented Jun 10, 2019

mkatliar commented Jun 10, 2019 •

edited

jcarpent commented Jun 10, 2019

jcarpent commented Jun 10, 2019

jcarpent commented Jun 11, 2019

mkatliar commented Jun 11, 2019

jcarpent commented Jun 22, 2019

jcarpent commented Jun 24, 2019

add support of CASADI #813

add support of CASADI #813

Conversation

jcarpent commented Jun 6, 2019

jcarpent commented Jun 6, 2019 • edited

jmirabel left a comment

Choose a reason for hiding this comment

jmirabel commented Jun 7, 2019

jcarpent commented Jun 7, 2019

jcarpent commented Jun 7, 2019

jcarpent commented Jun 7, 2019

jmirabel commented Jun 7, 2019 • edited

jcarpent commented Jun 7, 2019

jmirabel commented Jun 7, 2019

mkatliar commented Jun 7, 2019

jcarpent commented Jun 7, 2019

mkatliar commented Jun 7, 2019

mkatliar commented Jun 7, 2019 • edited

jcarpent commented Jun 7, 2019

jcarpent Jun 10, 2019

Choose a reason for hiding this comment

mkatliar Jun 10, 2019

Choose a reason for hiding this comment

jcarpent Jun 10, 2019

Choose a reason for hiding this comment

mkatliar Jun 13, 2019

Choose a reason for hiding this comment

mkatliar commented Jun 10, 2019 • edited

jcarpent commented Jun 10, 2019

jcarpent commented Jun 10, 2019

mkatliar commented Jun 10, 2019 • edited

mkatliar commented Jun 10, 2019

jcarpent commented Jun 10, 2019

mkatliar commented Jun 10, 2019 • edited

jcarpent commented Jun 10, 2019

jcarpent commented Jun 10, 2019

jcarpent commented Jun 11, 2019

mkatliar commented Jun 11, 2019

jcarpent commented Jun 22, 2019

jcarpent commented Jun 24, 2019

jcarpent commented Jun 6, 2019 •

edited

jmirabel commented Jun 7, 2019 •

edited

mkatliar commented Jun 7, 2019 •

edited

mkatliar commented Jun 10, 2019 •

edited

mkatliar commented Jun 10, 2019 •

edited

mkatliar commented Jun 10, 2019 •

edited