Python Bindings discussion

[Edwinem]

@artivis

I wanted to keep the PR more for review. So making a separate post.

Since I have some time over the Christmas break is there anything specifically you would like me to work on. Just so I don't overlap with your work. From reading your PR I think we are missing the following.

• Create bindings for eigen classes. (Needed so SE3 can accept Eigen::Quaternion ...)
• SE3 specifics
• SE2 specifics
• Other classes specifics
• Python packaging
• Examples

The other two things are numpy<-> eigen compatability(beyond setting Numpy to 'F'), and documentation. But I think we should hold off on those until the rest of the library is up and running. They also warrant some discussion as there are multiple ways to implement them.

[artivis]

I have just pushed a bunch more bindings that address some of the group-wise API (constructors, accessors etc) together with few more test.

First let me say that your idea of using EIGEN_DEFAULT_TO_ROW_MAJOR seems like the way to go. I have tried it and while the current tests are still passing, it allows to remove that ugly numpy option F. Shout out to you.

Now, concerning the remaining work and how we could split that, let me start with my number one concern which is precisely the Eigen/Geometry classes bindings. I have some work done on that front, but then googling around I realized that -of course- there were already many such bindings (e.g. eigenpy or your own manif fork), some also using pybind11 (e.g. in Drake). My feeling is thus that this really should live upstream in pybind11 rather than hosting yet another implementation of the binding.
Therefore the concern is twofold, how to get this upstream without too much pain, and what would a proper implementation look like. The Class_es path or rather the 'casters' one, like the current Eigen and numpy. I'm not even sure at the moment if this would be possible for the Geometry classes too. I didn't see a quaternion implementation in numpy (let along AngleAxis/Isometry etc)...
Note that, if for any reason going the upstream wasn't doable, I still think these bindings should live in a separate repository.
What's your opinion on this matter?

Going to your list of tasks, I'd say that everything but the last 2 items depends on the above paragraph and are on hold until it is resolved. Fortunately the last two items are rather independent. A proper Python packaging (setup.py/pip) would be really nice especially to hit the Python crowd: e.g. a pip install . in manif source directory.
The last item is necessary to document and showcase the library. This would essentially be about porting the examples/ folder to python and some 'how to get started' kind of page- maybe in the wiki. Not mentioning writing some more doc and unit tests.

I don't know if you are interested in any on those (3) tasks, please let me know if you'd like to tackle any.

[Edwinem]

No problem. The whole Row major vs Column major is to me one of the most frustrating cases of incompatible standards. There are also some other options we could maybe explore depending on how you want the library to be used, but that is the easiest solution for now.

Regarding the Eigen/Geometry stuff. I don't believe it is ever going to get added to pybind11. It doesn't fit the purpose of the library.

I am of the opinion that we should just add it straight to manif bindings.

1. It makes development easier.
2. The geometry classes are really more placeholders to pass data to manif.
3. Does separate repository mean a 2nd python package?
   • Cause now we are introducing a dependency.
   • The end user has to import from 2 packages rather than just 1.
   • Resources for what should be about 200-300 lines of code.
   • A missing purpose of the 2nd library. If we are just providing a couple of wrappers on quaternions and Isometries, then I would argue there are better pure Python libraries that provide the same functionality.

Regarding implementation I believe it has to be with Class_. As you mentioned numpy doesn't contain Quaternions or other geometric structures. And if we do any fancy type casting stuff, then we are going to run into ambiguity problems of when should it cast to a matrix, vs an isometry.

I can get started on the examples, and I'll look into the packaging(though have never done this before).

Regarding documentation, I know Pytorch does some interesting tricks with it. It might be worth looking into as it could maybe unify the C++ and Python documentation.

[artivis]

There are also some other options we could maybe explore depending on how you want

Ideally manif should support both but that's a heavy change which I'd like to address at the same time as other changes proposed here. So not anytime soon I'm afraid.

Concerning Eigen/Geometry, I understand what you are saying. But I also don't think that their place is in manif for the very same reasons you raised (not it's purpose, they really are placeholders). One could use manif without the whole Eigen/Geometry-related API, it may be less convenient but doable. Having these bindings in a separate package has the advantage that they could be used in other projects too. It also could be a soft dependency of manif: if it is not found, do not expose the related API.

I leave the example to you 👍. I think it would be nice to have at least one of each, that is one *_localization.py, one *_sam.py and *_sam_selfcalib.py. Duplicating them for different group may not be necessary and the others may not be so interesting.

I'll look into what Pytorch does for the doc for inspiration.

[GiulioRomualdi]

Hi all, Sorry to jump in.
In bipedal-locomotion-framework (a library we are developing in the DIC lab) we started working on Python bindings as well ami-iit/bipedal-locomotion-framework#134

In this file, you could find a working example of pybind11 with manif::SE3d object. Here you can find a python example where we used a SE3d manif object.

[artivis]

Hi @GiulioRomualdi,
Thanks for the links. Note that Python bindings for manif are already implemented using pybind11 in #179. This PR is somewhat ready to merge, I only need to find time to clean it up a little.