add Pose2.align() to wrapper

[johnwlambert]

No description provided.

[dellaert]

Could you add a quick python unit test before merging though?

[johnwlambert]

Could you add a quick python unit test before merging though?

Hi @dellaert, sure, I added one. One thing for us to be aware of is that if we provide ab pairs to this function, we get back bTa, not aTb, as expected. I think it's the same for the Pose3.align() as well. Maybe something to address in a PR in the future (would require modifying c++ tests also)

[johnwlambert]

@ProfFan do you know what's the preferred return type in .i if the c++ return type is boost::optional<gtsam::Pose2>?

[ProfFan]

@johnwlambert Good question! I haven't thought of that, but the problem is with the MATLAB bindings. I think adding an OptionalXXX typedef and ignore it in Python bindings, and adding a specialization for such an optional type would be the solution.

[dellaert]

@johnwlambert Good question! I haven't thought of that, but the problem is with the MATLAB bindings. I think adding an OptionalXXX typedef and ignore it in Python bindings, and adding a specialization for such an optional type would be the solution.

Hmmm. @ProfFan can we not return None in case there is no value? Python 3 types allow for an Optional[] type, so seems like maybe pybind could support it? @johnwlambert did you check that?

[varunagrawal]

The wrapper supports boost::optional. We use it in GTDynamics in multiple places. You could just return boost::optional<Pose2>.

[ProfFan]

@varunagrawal Yea but I don't think we support it in the MATLAB-specific code?

[varunagrawal]

I agree but it shouldn't error out for the matlab wrapper. I tried it locally and it compiles without issues.

If someone has a problem in the future, they can raise an issue and we'll look into it then. :)

[johnwlambert]

Thanks for taking a look all. It looks like including boost::optional in the .i file leads to the following errors:

======================================================================
ERROR: test_align (test_Pose2.TestPose2)
Ensure estimation of the Pose2 element to align two 2d point clouds succeeds.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/gtsam/gtsam/python/gtsam/tests/test_Pose2.py", line 61, in test_align
    bTa = gtsam.align(ab_pairs)
TypeError: Unable to convert function return value to a Python type! The signature was
	(pairs: std::vector<std::pair<Eigen::Matrix<double, 2, 1, 0, 2, 1>, Eigen::Matrix<double, 2, 1, 0, 2, 1> >, std::allocator<std::pair<Eigen::Matrix<double, 2, 1, 0, 2, 1>, Eigen::Matrix<double, 2, 1, 0, 2, 1> > > >) -> boost::optional<gtsam::Pose2>

Did you forget to `#include <pybind11/stl.h>`? Or <pybind11/complex.h>,
<pybind11/functional.h>, <pybind11/chrono.h>, etc. Some automatic
conversions are optional and require extra headers to be included
when compiling your pybind11 module.

[varunagrawal]

Add #include <pybind11/stl.h> to preamble/geometry.h.
You also need to add the PYBIND_MAKE_OPAQUE macro for Point2Pairs.

I'm adding a commit since there is actually some non-trivial stuff going on here.

[ProfFan]

Maybe stl_bind.h is enough? You still need the type caster though.

[varunagrawal]

The optional_caster is defined in pybind11/stl.h, so we do need it. 🙂

[johnwlambert]

Thanks @varunagrawal and @ProfFan. I guess we are good to merge this now then?

[dellaert]

Looks awesome, I'll merge!

[varunagrawal]

Yep