PyKDL bindings on Precise are flaky

[smits]

We've just witnessed the following behavior with the python bindings generated with sip 4.13.2 on Ubuntu Precise:

Digging deeper revealed that the underlying C++ object is correct but that the generation of the Python KDL.Vector object is wrong:

A: [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           1,           2,           3]]
B: [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           0,           0,           0]]
C: [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           1,           2,           3]]
(B*A): [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           1,           2,           3]]
C.p: [           1,           2,           3]
(B * A).p: [           0,           2,           3]
(B*A): [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           1,           2,           3]]
(B*A)[0][3]: 1.0
(B*A)[1][3]: 2.0
(B*A)[2][3]: 3.0
(B*A).p[0]: 0.0

This does not happen with bindings generated with sip 4.15.x on Ubuntu Precise or Trusty:

python test.py 
A: [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           1,           2,           3]]
B: [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           0,           0,           0]]
C: [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           1,           2,           3]]
(B*A): [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           1,           2,           3]]
C.p: [           1,           2,           3]
(B * A).p: [           1,           2,           3]
(B*A): [[           1,           0,           0;
            0,           1,           0;
            0,           0,           1]
[           1,           2,           3]]
(B*A)[0][3]: 1.0
(B*A)[1][3]: 2.0
(B*A)[2][3]: 3.0
(B*A).p[0]: 1.0

I currently see no easy way to resolve this on Ubuntu Precise except for updating your sip to 4.15. I personally used the following ppa for that: ppa:pali/kubuntu-backports

[daenny]

I just did some other tests:
Generating the code up to SIP version 4.16.7 it works as expected.
From 4.16.8 up to 4.18 SIP generates code, but has issues:
#129

Using SIP 4.19.7 creates code, but importing the PyKDL library then actually segfaults...

[MatthijsBurgh]

I have asked the PyQt mailing list, https://www.riverbankcomputing.com/mailman/listinfo/pyqt, for support.
The specific thread: http://python.6.x6.nabble.com/sip-Accessing-members-results-in-incorrect-missing-data-td5266521.html
@smits Could you please contribute?

[MatthijsBurgh]

With the developer of SIP, we have fixed a few bugs in SIP. This will fix both the incorrect data, this issue and #129, and the memory leak of #157.
The relevant fixes:
https://www.riverbankcomputing.com/hg/sip/rev/137b9be794a1
https://www.riverbankcomputing.com/hg/sip/rev/6a057b2d8537
https://www.riverbankcomputing.com/hg/sip/rev/699facc95914
https://www.riverbankcomputing.com/hg/sip/rev/82ec38dc0e63
For details see the mailing list: http://python.6.x6.nabble.com/sip-Accessing-members-results-in-incorrect-missing-data-td5266521.html

So you can either build python_orocos_kdl from source, with SIP from source. A new release, probably 4.19.21, will be released soon.

The debians are not maintained by the developer of SIP, but the Debian Python Modules Team (DPMT). Once the new SIP release is out, I will request them to update python-sip for all ubuntu versions 16+.

[MatthijsBurgh]

Bug report filed on LaunchPad to get python-sip updated to 4.19.21 on 16.04 and 18.04, https://bugs.launchpad.net/ubuntu/+source/sip4/+bug/1861601

[MatthijsBurgh]

In my opinion a new released python-sip is needed to have a new release of python_orocos_kdl. There is not much happening on LaunchPad, so extra contributions by all of you is needed to get a new release.
Please contribute: @smits @traversaro @mrana6 @tfoote @zchen24 @mvieth @luzpaz @francisco-miguel-almeida @ahoarau @jbohren

[zchen24]

Interesting, are you open to other wrapping libraries? I did some work wrapping KDL using pybind11 a while ago and it works reasonably well. https://github.com/zchen24/orocos_kinematics_dynamics/tree/feature/replace-sip-with-pybind11

…

On Fri, Feb 7, 2020 at 00:26 Matthijs van der Burgh < ***@***.***> wrote: In my opinion a new released python-sip is needed to have a new release of python_orocos_kdl. There is not much happening on LaunchPad <https://bugs.launchpad.net/ubuntu/+source/sip4/+bug/1861601>, so extra contributions by all of you is needed to get a new release. Please contribute: @smits <https://github.com/smits> @traversaro <https://github.com/traversaro> @mrana6 <https://github.com/mrana6> @tfoote <https://github.com/tfoote> @zchen24 <https://github.com/zchen24> @mvieth <https://github.com/mvieth> @luzpaz <https://github.com/luzpaz> @francisco-miguel-almeida <https://github.com/francisco-miguel-almeida> @ahoarau <https://github.com/ahoarau> @jbohren <https://github.com/jbohren> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#41?email_source=notifications&email_token=AAVG6T5GT3AODTQYW2ZKESTRBULHZA5CNFSM4AYXZ4Y2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELCDRTA#issuecomment-583284940>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVG6T6OS5W36NY6ZLLKI5TRBULHZANCNFSM4AYXZ4YQ> .

[MatthijsBurgh]

@zchen24 I cherry-picked your commit and did some extra fixes, but I get a segmentation fault with PyBind11 v2.4.3, see https://travis-ci.com/MatthijsBurgh/orocos_kinematics_dynamics/builds/148004083
The segmentation faults happen in the Pickle tests

[zchen24]

@MatthijsBurgh Wow, I did not expect that you will just run the continuous integration test on it. 👍
Regarding the status of Pybind11, the wrapping of the KDL library is pretty complete, but there could be some corner cases that are missing, and the Pickle test failure is expected as this feature has not been implemented yet. See https://pybind11.readthedocs.io/en/stable/advanced/classes.html#pickling-support. Besides pickle support, I also have a concern regarding the unit test coverage. I am happy to spend some time this weekend and might need your help with testing.

[traversaro]

There is not much happening on LaunchPad, so extra contributions by all of you is needed to get a new release.
Please contribute: @smits @traversaro @mrana6 @tfoote @zchen24 @mvieth @luzpaz @francisco-miguel-almeida @ahoarau @jbohren

Hi @MatthijsBurgh, I have not used orocos_kdl in a long time, but I would be happy to give an hand if it is necessary. However, it is not clear to me how we can help. In particular, I do not think it is possible given how Ubuntu handles its releases to have a package to be update to a completely new version. If you were able to backport the fix as a patch, you could try to start the SRU process (https://wiki.ubuntu.com/StableReleaseUpdates) but even that is quite a long process (see for example ros/urdfdom#119). If you need this fixed in your setups in the short or even medium term, I would suggest to stop using the system python-sip and python_orocos_kdl, and ship your own version, either compiling them from source or providing updated versions via a PPA.

[MatthijsBurgh]

There is not much happening on LaunchPad, so extra contributions by all of you is needed to get a new release.
Please contribute: @smits @traversaro @mrana6 @tfoote @zchen24 @mvieth @luzpaz @francisco-miguel-almeida @ahoarau @jbohren

Hi @MatthijsBurgh, I have not used orocos_kdl in a long time, but I would be happy to give an hand if it is necessary. However, it is not clear to me how we can help. In particular, I do not think it is possible given how Ubuntu handles its releases to have a package to be update to a completely new version. If you were able to backport the fix as a patch, you could try to start the SRU process (https://wiki.ubuntu.com/StableReleaseUpdates) but even that is quite a long process (see for example ros/urdfdom#119). If you need this fixed in your setups in the short or even medium term, I would suggest to stop using the system python-sip and python_orocos_kdl, and ship your own version, either compiling them from source or providing updated versions via a PPA.

I have a solution for my own setup. But I would like to come up with a robust solution for the entire ROS community.
So the options:

1. Get a SRU for python-sip, see https://bugs.launchpad.net/ubuntu/+source/sip4/+bug/1861601
2. Replace SIP with PyBind11, see PyBind11 instead of SIP #219
3. Release a updated version of python-sip via the ROS servers. Is this a possibility @traversaro?

[MatthijsBurgh]

PR for PyKDL based on PyBind11 is open: #229

python-sip fix is also pending for approval in ubuntu launchpad, so that should also be released in a few weeks.

[traversaro]

Release a updated version of python-sip via the ROS servers. Is this a possibility @traversaro?

Sorry, I did not saw this mention back in time. I am not related in any way with the maintenance of the maintenance of debian packages on ROS servers, but think in the past the attitude was to try to avoid to release updated version of packages that are released in Ubuntu repositories in the ROS repositories, to avoid hard to debug conflicts or problems. I remember some discussion on a GitHub issue on this, but I at the moment I can't find it.

[MatthijsBurgh]

A fixed version of python-sip is now available on updates both for Xenial and Bionic.

[traversaro]

Thanks for the hard work @MatthijsBurgh !

[jspricke]

I guess the next step is to have a new release, @MatthijsBurgh ?
Can you do that @smits ?

[MatthijsBurgh]

@jspricke A new release on SIP is possible or merge #229 and release a PyBind11 based PyKDL. But both are up to @smits and @meyerj.

[smits]

I'll try to release in the coming days. AFAIU the release is only required to regenerate the python bindings, right? Since we're going with option 1, in which a fixed python-sip will be available during build time?

[jspricke]

We are interested in a new release from master to have #212 in there as well.

[MatthijsBurgh]

@smits, Yes, new python bindings need to be generated based on the updated python-sip release. You could use the same commit as the current PyKDL release, but there have been many changes since the last release, march 2018. So I would suggest an entire new release of both KDL and PyKDL.

PS. in #229 there are some fixes to the SIP version of PyKDL. I could create a separate PR for those commits, so these could be include in this release. Are you open for that?

[smits]

@MatthijsBurgh we have to maintain binary compatibility for the existing ROS releases, so I can create a new release to be incorporated into the next ROS release but not for kinetic/melodic etc. unless we try to cherrypick the binary compatible fixes for 1.3 and 1.4

[MatthijsBurgh]

I think cherrypicking it all is a lot of work. So just keep it to new python bindings based on the same release. People will just need to build from source, if they want a newer version of (Py)KDL.

[MatthijsBurgh]

@smits could you create new package releases on kinetic and melodic based on the same commits as the current releases, but which use the new version of sip? This would fix this bug and #129 and #157.

[MatthijsBurgh]

16.04 has reached end-of-life, problem still on 18.04. That requires a re-release of the same version, but with updated sip.