Bipedal-locomotion-framework: Error when using the python bindings on the master branch of blf

[paolo-viceconte]

I recently tried to use the most updated version of blf (master branch), encountering issues in the exploitation of the python bindings.

More specifically, if:

• a new RL docker container is created
• the missing dependencies for blf (i.e. spdlog, manif and ospq-eigen) are installed (spdlog via apt, devel branch for manif and master branch for ospq-eigen)
• the master branch of blf is compiled and installed

Then a simple import bipedal_locomotion_framework within ipython returns:

terminate called after throwing an instance of 'pybind11::error_already_set'
  what():  SystemError: <built-in method __contains__ of dict object at 0x7fd340ab23c0> returned a result with an error set
Aborted (core dumped)

By using instead previous blf commits such as:

• the commit in ProjectsTagsStable.cmake (which doesn't require to install any further dependency in the docker container)
• the commit implementing the IK (which requires to install spdlog and osqp-eigen to compile all the modules of blf)

the exploitation of the python bindings does not create any issue.

I will try to understand exactly from which commit (and/or associated dependency installation) on such an error appears, so that It can be easier to solve.

cc @GiulioRomualdi @diegoferigo

[diegoferigo]

A good candidate for checking when this problem started to occur is https://github.com/dic-iit/bipedal-locomotion-framework/pull/238, on a similar setup I'm still using the commit tagged by the superbuild that does not yet include the changes of that PR.

Maybe related to the usage of py::module::import? See https://github.com/pybind/pybind11/issues/686. Can you check if you have the manifpy module in your system?

[paolo-viceconte]

Maybe related to the usage of py::module::import? See pybind/pybind11#686. Can you check if you have the manifpy module in your system?

Good catch @diegoferigo, manifpy was not in the system. By installing it via pip as for instance documented in https://github.com/artivis/manif/issues/209#issuecomment-799614436, the pybind11::error_already_set error disappears. Should this installation step be mentioned in https://github.com/dic-iit/bipedal-locomotion-framework#snake-python as a requirement for using blf from python?

Also the import of all the submodules import bipedal_locomotion_framework.all suggested in https://github.com/dic-iit/bipedal-locomotion-framework/pull/238#issuecomment-804047766 succeeds. Therefore, this issue can be considered solved.

[diegoferigo]

The expected behavior is that, after you import bipedal_locomotion_framework, a Python exception get raised instead of a C++ exception. I would expect an ImportError.

I'd leave this issue open, we have to figure out what's missing in our side to bind the C++ exception to the Python exception during the early stage of import.

[diegoferigo]

@paolo-viceconte can you please try if the following works? It changes these lines.

    @cmakeif FRAMEWORK_USE_manif
    {
        py::scoped_interpreter guard{};
        py::module::import("manifpy");
    }
    @endcmakeif FRAMEWORK_USE_manif

[paolo-viceconte]

@paolo-viceconte can you please try if the following works? It changes these lines.

    @cmakeif FRAMEWORK_USE_manif
    {
        py::scoped_interpreter guard{};
        py::module::import("manifpy");
    }
    @endcmakeif FRAMEWORK_USE_manif

If I try to compile blf with this modification, what I get is a error: 'scoped_interpreter' is not a member of 'py'.

[paolo-viceconte]

In the updated blf configuration (without this last modification proposed by @diegoferigo), I am also encountering the following issue: the call to the function add_contact mentioned in https://github.com/dic-iit/bipedal-locomotion-framework/pull/238#issuecomment-799494808 fails with

TypeError: add_contact(): incompatible function arguments. The following argument types are supported:
    1. (self: bipedal_locomotion_framework.bindings.contacts.ContactList, contact: bipedal_locomotion_framework.bindings.contacts.PlannedContact) -> bool
    2. (self: bipedal_locomotion_framework.bindings.contacts.ContactList, transform: manif::SE3<double>, activation_time: float, deactivation_time: float) -> bool

Invoked with: ContactList(0); kwargs: transform=<manifpy.SE3 object at 0x7fef9fe3d9b0>, activation_time=0.0, deactivation_time=4.0

This seems to me opposite to what described in https://github.com/dic-iit/bipedal-locomotion-framework/pull/238#issuecomment-799494808 and https://github.com/dic-iit/bipedal-locomotion-framework/pull/238#issuecomment-799502982.

[diegoferigo]

I just tried on the most updated RL image. I did the following:

sudo chown -R $(id -u):$(id -g) $ROBOTOLOGY_SUPERBUILD_SOURCE_DIR/build`
cd $ROBOTOLOGY_SUPERBUILD_SOURCE_DIR/src

cd bipedal-locomotion-framework
git checkout master
git pull

cd ../manif
git checkout devel
git pull

cd $ROBOTOLOGY_SUPERBUILD_SOURCE_DIR/build/src/manif
ninja install

cd $ROBOTOLOGY_SUPERBUILD_SOURCE_DIR/build/src/bipedal-locomotion-framework
ninja install

I short, I updated the superbuild to use manif@devel and blf, that automatically detects and enabel python support. There's not manifpy at this point. Note that, for a quick test, I bypassed the superbuild logic that would checkout the previous tagged commits defined in ProjectTags.

I cannot reproduce the original error. In fact, I get the following:

build/src/bipedal-locomotion-framework on  master via △ v3.20.1
❯ ipython
Python 3.8.8 | packaged by conda-forge | (default, Feb 20 2021, 16:22:27) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.22.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import bipedal_locomotion_framework
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-6c657a94a2ec> in <module>
----> 1 import bipedal_locomotion_framework

/usr/local/src/robotology-superbuild/build/src/bipedal-locomotion-framework/bipedal_locomotion_framework/__init__.py in <module>
----> 1 from . import bindings

ImportError: ModuleNotFoundError: No module named 'manifpy'

that is the expected behavior, i.e. a clear Python exception.

After installing manifpy with pip install git+https://github.com/artivis/manif@devel, I can successfully import bipedal_locomotion_framework and run the Python tests with pytest -sv $ROBOTOLOGY_SUPERBUILD_SOURCE_DIR/src/bipedal-locomotion-framework/bindings/python/Contacts/tests. Can you check that pytest succeeds in you case?

Edit: sorry, I used the wrong container without noticing, I'm testing RL now.

[diegoferigo]

Before I execed in the wrong container without noticing (too many open in the same time xD). I followed pretty much the same steps and I reproduced the original error. It's very likely related to an old pybind11 version shipped in Ubuntu Focal. In the previous comment I used the version gathered by pip that is the most updated (however you have to point CMake to the right folder).

Installing manif as commented above solves the import error.

TL;DR, I don't think we need to do much from the blf side. The cryptic error occurs only with outdated pybind11 versions (sadly, also that shipped in Focal). I'm not sure if we can find a better workaround @GiulioRomualdi.

[paolo-viceconte]

Thanks for the details on the procedure @diegoferigo. What is different in my current configuration wrt what you described is that I am compiling my custom folders of blf, manif, osqp-eigen in the git folder mounted within the docker container and then installing them in the /iit/sources/robotology-superbuild/build/install folder.

If I run the pytest you mention, some tests fail with the error mentioned in https://github.com/dic-iit/bipedal-locomotion-framework/issues/294#issuecomment-831907913.

If rather than installing manifpy as decribed in https://github.com/artivis/manif/issues/209#issuecomment-799614436, I use the pip install git+https://github.com/artivis/manif@devel command you suggested, the pytest succeeds.

[diegoferigo]

So, is everything fine? I guess we are in the limbo waiting that the manif bindings of the devel branch will land a release, and then we can also make a new blf release and sync the superbuild.

I remember some problem with spdlogs on Bionic, but I'm not aware of details @GiulioRomualdi @traversaro

[paolo-viceconte]

So, is everything fine? I guess we are in the limbo waiting that the manif bindings of the devel branch will land a release, and then we can also make a new blf release and sync the superbuild.

Yes, that error is solved. I will now try to update my scripts looking at the successfull tests for each module of the bindings. I will keep you updated here in case further issues appear.

[traversaro]

So, is everything fine? I guess we are in the limbo waiting that the manif bindings of the devel branch will land a release, and then we can also make a new blf release and sync the superbuild.

I remember some problem with spdlogs on Bionic, but I'm not aware of details @GiulioRomualdi @traversaro

Yes, everything was kind of blocked by https://github.com/TartanLlama/optional/pull/45, but at least that one unblocked now.

[paolo-viceconte]

Yes, that error is solved. I will now try to update my scripts looking at the successfull tests for each module of the bindings. I will keep you updated here in case further issues appear.

After a few modifications inspired by the python tests and by the commit on restructuring the python bindings in submodules, I was able to successfully run my scripts in the updated environment.

To summarize, the errors described in this issue are solved by installing manifpy via pip install git+https://github.com/artivis/manif@devel. If you agree, we can close this issue.

[diegoferigo]

Remember to update the import statement so that when the backward-compatible import that @GiulioRomualdi kindly implemented will be removed, you don't have to update your code.

If the problem is solved, go ahead and close the issue.

[paolo-viceconte]

Remember to update the import statement so that when the backward-compatible import that @GiulioRomualdi kindly implemented will be removed, you don't have to update your code.

I actually followed the approach used in the python tests, i.e. importing with import bipedal_locomotion_framework.bindings as blf and then calling each module separately (for instance blf.planners). This allows not to rely on the temporary import bipedal_locomotion_framework.all as blf documented in https://github.com/dic-iit/bipedal-locomotion-framework/pull/238#issuecomment-804047766.

If the problem is solved, go ahead and close the issue.

Closing. Thanks for the support!