#196 Merged at 3752d8a
Repository
EllonPaiva
Branch
feature/py_wrap
Repository
gtborg
Branch
feature/py_wrap
Author
  1. Ellon Paiva
Reviewers
Description

I've developed a little more on top of feature/py_wrap. Just sharing so you can check out the changes.

Comments (40)

  1. Frank Dellaert

    Ellon, could you past an example of the generated file here so we can have some context as to what the code produces? Also, Jenkins seems to think the code does not compile right now, or does it for you?

    1. Ellon Paiva author

      The code produced from a20918a3211b can be found below.

      My code here is compiling. I Imagine Jenkins cannot compile the code because de dependencies of numpy_eigen. Is there a way to access the build output from jenkins to see why it failed?

      EDIT: Removed generated code. Updated one below.

  2. Ellon Paiva author

    I just updated the pull request with few more stuff.

    • I wrapped Rot3 and Unit3 too. Not all functions yet because I need to deal properly with overloads, but I hope I'll do it soon.
    • I added gtborg/numpy_eigen as a 3rdparty library. At the end it was easy (no compilation, just to include the proper directory given the option to build python module). I think gtsam/3rdparty/numpy_eigen need a cleanup though.

    I'll put below the current code being generated by the wrapper. I'm testing the module simply by calling ipython from gtsam/python, importing the module and calling the methods I'm wrapping.

    EDIT: Removed outdated generated code.

  3. Comment deleted
    1. Ellon Paiva author

      So now I wrapped several methods of Pose3. I have the overloading being done using Boost Python macros. I'm using the structures defined on Signature.h to handle the use of these macros and creation of function pointers.

      Several methods have OptionalJacobian as parameter with default value. It is possible to wrap methods that accept OptionalJacobians if the methods aren't overloaded. If they're overloaded we cannot wrap it yet. The reason is that to wrap overloaded methods we need to create a valid function pointer, and (I think) we need to wrap OptionalJacobian first, and I didn't realize how to do it yet. I'll check it soon.

      You can find the current generated code below. The file gtsampy.h has some TODO comments that are interesting too.

      #include <boost/python.hpp>
      
      #include <numpy_eigen/NumpyEigenConverter.hpp>
      
      #include <gtsam/geometry/Point2.h>
      #include <gtsam/geometry/Point3.h>
      #include <gtsam/geometry/Pose3.h>
      #include <gtsam/geometry/Rot2.h>
      #include <gtsam/geometry/Rot3.h>
      #include <gtsam/geometry/Unit3.h>
      
      using namespace boost::python;
      using namespace gtsam;
      
      // Prototypes used to perform overloading
      gtsam::Rot3 (*AxisAngle_0)(const Vector3&, double) = &Rot3::AxisAngle;
      gtsam::Rot3 (*AxisAngle_1)(const gtsam::Point3&, double) = &Rot3::AxisAngle;
      gtsam::Rot3 (*Rodrigues_0)(const Vector3&) = &Rot3::Rodrigues;
      gtsam::Rot3 (*Rodrigues_1)(double, double, double) = &Rot3::Rodrigues;
      gtsam::Rot3 (*RzRyRx_0)(double, double, double) = &Rot3::RzRyRx;
      gtsam::Rot3 (*RzRyRx_1)(const Vector&) = &Rot3::RzRyRx;
      BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(Pose3_equals_overloads_0, equals, 1, 2)
      bool (Pose3::*equals_0)(const gtsam::Pose3&, double) const = &Pose3::equals;
      
      BOOST_PYTHON_MODULE(libgtsampy_python)
      {
      
      import_array();
      NumpyEigenConverter<Vector>::register_converter();
      NumpyEigenConverter<Vector1>::register_converter();
      NumpyEigenConverter<Vector2>::register_converter();
      NumpyEigenConverter<Vector3>::register_converter();
      NumpyEigenConverter<Vector4>::register_converter();
      NumpyEigenConverter<Vector5>::register_converter();
      NumpyEigenConverter<Vector6>::register_converter();
      NumpyEigenConverter<Vector7>::register_converter();
      NumpyEigenConverter<Vector8>::register_converter();
      NumpyEigenConverter<Vector9>::register_converter();
      NumpyEigenConverter<Vector10>::register_converter();
      
      NumpyEigenConverter<Matrix>::register_converter();
      NumpyEigenConverter<Matrix2>::register_converter();
      NumpyEigenConverter<Matrix3>::register_converter();
      NumpyEigenConverter<Matrix4>::register_converter();
      NumpyEigenConverter<Matrix5>::register_converter();
      NumpyEigenConverter<Matrix6>::register_converter();
      NumpyEigenConverter<Matrix7>::register_converter();
      NumpyEigenConverter<Matrix8>::register_converter();
      NumpyEigenConverter<Matrix9>::register_converter();
      
      class_<Point2>("Point2")
        .def(init<>())
        .def(init<double,double>())
        .def(init<Vector>())
        .def("identity", &Point2::identity)
        .staticmethod("identity")
        .def("dist", &Point2::dist)
        .def("distance", &Point2::distance)
        .def("equals", &Point2::equals)
        .def("norm", &Point2::norm)
        .def("print", &Point2::print)
        .def("unit", &Point2::unit)
        .def("vector", &Point2::vector)
        .def("x", &Point2::x)
        .def("y", &Point2::y)
        .def(self * other<double>())
        .def(other<double>() * self)
        .def(self + self)
        .def(-self)
        .def(self - self)
        .def(self / other<double>())
        .def(self_ns::str(self))
        .def(repr(self))
        .def(self == self)
      ;
      
      class_<Point3>("Point3")
        .def(init<>())
        .def(init<double,double,double>())
        .def(init<Vector>())
        .def("identity", &Point3::identity)
        .staticmethod("identity")
        .def("add", &Point3::add)
        .def("cross", &Point3::cross)
        .def("dist", &Point3::dist)
        .def("distance", &Point3::distance)
        .def("dot", &Point3::dot)
        .def("equals", &Point3::equals)
        .def("norm", &Point3::norm)
        .def("normalize", &Point3::normalize)
        .def("print", &Point3::print)
        .def("sub", &Point3::sub)
        .def("vector", &Point3::vector)
        .def("x", &Point3::x)
        .def("y", &Point3::y)
        .def("z", &Point3::z)
        .def(self * other<double>())
        .def(other<double>() * self)
        .def(self + self)
        .def(-self)
        .def(self - self)
        .def(self / other<double>())
        .def(self_ns::str(self))
        .def(repr(self))
        .def(self == self)
      ;
      
      class_<Rot2>("Rot2")
        .def(init<>())
        .def(init<double>())
        .def("Expmap", &Rot2::Expmap)
        .staticmethod("Expmap")
        .def("Logmap", &Rot2::Logmap)
        .staticmethod("Logmap")
        .def("atan2", &Rot2::atan2)
        .staticmethod("atan2")
        .def("fromAngle", &Rot2::fromAngle)
        .staticmethod("fromAngle")
        .def("fromCosSin", &Rot2::fromCosSin)
        .staticmethod("fromCosSin")
        .def("fromDegrees", &Rot2::fromDegrees)
        .staticmethod("fromDegrees")
        .def("identity", &Rot2::identity)
        .staticmethod("identity")
        .def("relativeBearing", &Rot2::relativeBearing)
        .staticmethod("relativeBearing")
        .def("c", &Rot2::c)
        .def("degrees", &Rot2::degrees)
        .def("equals", &Rot2::equals)
        .def("matrix", &Rot2::matrix)
        .def("print", &Rot2::print)
        .def("rotate", &Rot2::rotate)
        .def("s", &Rot2::s)
        .def("theta", &Rot2::theta)
        .def("unrotate", &Rot2::unrotate)
      ;
      
      class_<Rot3>("Rot3")
        .def(init<>())
        .def(init<Point3,Point3,Point3>())
        .def(init<double,double,double,double,double,double,double,double,double>())
        .def(init<Matrix3>())
        .def(init<Matrix>())
        .def("AxisAngle", AxisAngle_0)
        .def("AxisAngle", AxisAngle_1)
        .staticmethod("AxisAngle")
        .def("Expmap", &Rot3::Expmap)
        .staticmethod("Expmap")
        .def("ExpmapDerivative", &Rot3::ExpmapDerivative)
        .staticmethod("ExpmapDerivative")
        .def("Logmap", &Rot3::Logmap)
        .staticmethod("Logmap")
        .def("LogmapDerivative", &Rot3::LogmapDerivative)
        .staticmethod("LogmapDerivative")
        .def("Rodrigues", Rodrigues_0)
        .def("Rodrigues", Rodrigues_1)
        .staticmethod("Rodrigues")
        .def("Rx", &Rot3::Rx)
        .staticmethod("Rx")
        .def("Ry", &Rot3::Ry)
        .staticmethod("Ry")
        .def("Rz", &Rot3::Rz)
        .staticmethod("Rz")
        .def("RzRyRx", RzRyRx_0)
        .def("RzRyRx", RzRyRx_1)
        .staticmethod("RzRyRx")
        .def("identity", &Rot3::identity)
        .staticmethod("identity")
        .def("AdjointMap", &Rot3::AdjointMap)
        .def("column", &Rot3::column)
        .def("conjugate", &Rot3::conjugate)
        .def("equals", &Rot3::equals)
        .def("localCayley", &Rot3::localCayley)
        .def("matrix", &Rot3::matrix)
        .def("print", &Rot3::print)
        .def("r1", &Rot3::r1)
        .def("r2", &Rot3::r2)
        .def("r3", &Rot3::r3)
        .def("retractCayley", &Rot3::retractCayley)
        .def("rpy", &Rot3::rpy)
        .def("slerp", &Rot3::slerp)
        .def("transpose", &Rot3::transpose)
        .def("xyz", &Rot3::xyz)
        .def(self * self)
        .def(self_ns::str(self))
        .def(repr(self))
      ;
      
      class_<Pose3>("Pose3")
        .def(init<>())
        .def(init<Pose3>())
        .def(init<Rot3,Point3>())
        .def(init<Matrix>())
        .def("identity", &Pose3::identity)
        .staticmethod("identity")
        .def("bearing", &Pose3::bearing)
        .def("equals", equals_0, Pose3_equals_overloads_0())
        .def("matrix", &Pose3::matrix)
        .def("print", &Pose3::print)
        .def("transform_from", &Pose3::transform_from)
        .def("x", &Pose3::x)
        .def("y", &Pose3::y)
        .def("z", &Pose3::z)
        .def(self * self)
        .def(self * other<Point3>())
        .def(self_ns::str(self))
        .def(repr(self))
      ;
      
      class_<Unit3>("Unit3")
        .def(init<>())
        .def(init<Point3>())
        .def(init<Vector3>())
        .def(init<double,double,double>())
        .def("Dim", &Unit3::Dim)
        .staticmethod("Dim")
        .def("FromPoint3", &Unit3::FromPoint3)
        .staticmethod("FromPoint3")
        .def("dim", &Unit3::dim)
        .def("distance", &Unit3::distance)
        .def("equals", &Unit3::equals)
        .def("error", &Unit3::error)
        .def("localCoordinates", &Unit3::localCoordinates)
        .def("point3", &Unit3::point3)
        .def("print", &Unit3::print)
        .def("retract", &Unit3::retract)
        .def("skew", &Unit3::skew)
        .def(other<double>() * self)
      ;
      
      }
      
  4. Ellon Paiva author

    Hello again,

    So, I arrived somehow in a "dead end" here... I would like to ask for some suggestions you may have to proceed implementing the wrapper.

    When I started doing it I wanted to have the following classes wrapped for python: Point3, Rot3, Pose3, BetweenFactor<Pose3>, ISAM2 and others that ISAM2 depends for working properly (NonlinearFactorGraph, Values, etc). Then I got involved on the details of the wrapper and I saw myself trying to wrap everything I see. It as proven to be not easy :) I want to go back on my original track, at least for now.

    Thus I tried to wrap BetweenFactor and I saw it has tons of dependencies, and I had the wrapper complain about missing dependency all the time. Could someone with a deeper experience on GTSAM give me a tip of which classes to wrap to have acces of BetweenFactor? And maybe for the other classes I mentioned before as well?

    I also tried to wrap the OptionalJacobian classes. I saw it is a templated class on int's

    template<int Rows, int Cols> class gtsam::OptionalJacobian< Rows, Cols >
    

    and that the wrapper grammar does not accept typedefs with non-types as template parameters, like this one:

    typedef gtsam::OptionalJacobian<2, 3> OptionalJacobian23;
    

    I though of changing TypeGrammar to allow numbers, but I was worried I would just mess something else that uses the TypeGrammar, and I also didn't see how a number would fit in "Qualified" class. Does anyone have an idea of how to do it?

    1. Frank Dellaert

      Hi Ellon

      BetweenFactor should be OK, because we wrap that in matlab.h. Take a look at gtsam.h for the context in which it is wrapped. I have little insight now on the implications for compilation/linking of the boost-python wrapper.

      OptionalJacobian is a can of worms, actually. At the moment there is no Expression support in the matlab wrapper either, and I have not thought deeply on how to provide it or whether it even makes sense. Why do you want to wrap it?

      Frank

      1. Ellon Paiva author

        Hi Frank,

        Ok, I'll look again the matlab interface file.

        About the OptionalJacobian, I don't really want to wrap it, but to make boost python call the correct overload sometimes I need to make a function pointer with the right signature, and some methods have arguments of OptionalJacobian type with default values, so I need the OptionalJacobian to create the signature, even if I won't really use the OptionalJacobian from python.

        Example: gtsam::Rot3 has the methods:

        Point3 rotate(const Point3& p, OptionalJacobian<3,3> H1 = boost::none,
                OptionalJacobian<3,3> H2 = boost::none) const;
        Unit3 rotate(const Unit3& p, OptionalJacobian<2,3> HR = boost::none,
                OptionalJacobian<2,2> Hp = boost::none) const;
        

        rotate has an overload with a "non-common sequence" of arguments. To tell boost python which method I want to call, I need to create 2 function pointers, like this:

        Point3 (Rot3::*rotate_0)(const gtsam::Point3&, OptionalJacobian<3,3>, OptionalJacobian<3,3>) const = &Rot3::rotate;
        Unit3 (Rot3::*rotate_1)(const gtsam::Unit33&, OptionalJacobian<2,3>, OptionalJacobian<2,2>) const = &Rot3::rotate;
        

        even if I want to expose only the versions of rotate that take only the first argument. But to write it automatically typedefs of OptionalJacobian need to be accepted by the wrapper grammar, and currently it does not accept values as template arguments.

        Of course I can always decide not to wrap these methods, even if I think it would be cool to expose the jacobians to python. I was just wondering about the implications of change the grammar to accept the typedefs I need. I will see if I find a commit before the addition of OptionalJacobian where I can rebase my python branch into and try to do this kind o wrapping there.

  5. Frank Dellaert

    Interesting. In the matlab wrapper this problem does not seem to occur, as we would just omit the optional Jacobian arguments in gtsam.h. The generated code just uses the default arguments (none) and that compiles fine. Is there no way to do the same here?

    1. Ellon Paiva author

      I don't think so. Overloading like these using boost python needs function pointers, that need the correct definition of the arguments. I think the wrapper would need to be updated to allow this kind of construction in the generated file.

      Anyway, I moved to handwritten wrapping of the classes I need. :)

  6. Ellon Paiva author

    Now I think I wrapped all I need to python. I may still add more classes and/or functionalities if I find out there's something not wrapped I need, but I don't think I will.

    In summary, the PR now contains:

    • some contributions for the automatic wrapper, but not fully implemented;
    • several classes wrapped by hand, including geometry basic types (Point, Rot, and Pose) and ISAM2
    • a python version of VisualISAM2Example.cpp (with plots using matplotlib)

    I suggest looking into the example (python/gtsam/examples/VisualISAM2Example.py) to check how is the systax of gtsam in python. I'm trying to be as much "pythonic" as possible.

    Could you tell me if it's good to be merged?

    Ellon

      1. Frank Dellaert

        All of my comments are about making the numpy wrap more minimal. I am amazed at the amount of wrapping you did. As far as I'm concerned, this is mergable in py_wrap, after addressing that....

        But more so, we could probably merge the handwritten wrapped classes into develop, and have the py_wrap branch concentrate on wrapping automatically. The two are separate projects, really.

        One way to do this is - merge develop in - create a new branch on develop and check it out - on the command line, do

        git checkout feature/py_wrap -- file_or_directory_to_copy
        

        We can then merge that into develop, after we make sure it does not break.

      2. Natesh Srinivasan

        The cmake file inside the "handwritten" folder is different style as compared to the other ones in the gtsam project. We could probably move the subdirlist macro into our cmake function. Another question I had was if the lib file that is generated inside the root build folder "libgtsampy_python.so" has to be under the python folder ? . I also see copies of it generated in the gtsam/python/gtsam folder that is outside the build folder. If these are issues I can fix these

        1. Ellon Paiva author

          Hi Natesh,

          The handwritten wrap is build into build/python/handwritten. The copy to python/gtsam follows the python wrapper compilation setup made by Andrew: the copy is needed there if you want to use the setup.py to install the python wrapper. As a matter of fact, I'm not installing the library, but using it locally by calling python from handwritten/python folder. I'll check today if the install procedure still works.

          About the CMake macro, I'm okay moving it somewhere. Which file would be the best candidate? Or should I create a new one?

          1. Natesh Srinivasan

            I see. perhaps we could copy the entire python folder into the build folder ? I m not sure what Frank thinks about this. The only reason I am bringing this up is that copying a .so file outside the build folder might not be ok for git check-ins. @Chris Beall, what do you think about this ?

            The macro can be moved inside the "GtsamPythonWrap.cmake". I think this might the most suitable place I can think of.

              1. Ellon Paiva author

                Right. I'll wait for an approach for this. I'm not very good with CMake scripting though, so it may be hard for me to implement the approach. Please be kind. :)

  7. Ellon Paiva author

    OK, following Frank's suggestion, I rebased the branch and left only things related with the automatic wrapper here. I'll create other pull request with the handwritten files.

    Note: the commit that adds numpy_eigen to 3rdparty will be common to both branches until py_wrap can be merged into develop.

  8. Chris Beall

    Sorry about the insane amount of spam. Bitbucket has been experiencing API failures the last few hours, which causes Jenkins to go crazy. I've disabled Jenkins, and cancelled the remainder of the queue.