From 00a04a1f5d91757b9876e5e4428cd52743fa7a47 Mon Sep 17 00:00:00 2001 From: Gerry Chen Date: Mon, 12 Apr 2021 19:08:29 -0400 Subject: [PATCH 1/3] Squashed 'wrap/' changes from 5ddaff8ba..bae34fac8 bae34fac8 Merge pull request #82 from borglab/feature/templated-base-class 6fd4053dd Merge pull request #84 from borglab/feature/print_redirectstdout 349784a38 comment about redirecting stdout dc4fd9371 updated docs b43f7c6d7 Merge pull request #80 from borglab/feature/multiple_interface_files 7d8c3faa7 redirect std::out for print_() a812a6110 print redirect stdout unit test 97f3b6994 support for templated base class 3e16564e3 test cases for templated base class 7b9d080f5 Revert "unit test for printing with keyformatter" 45e203195 Revert "funcitonal print passes unit test" 8b5d66f03 Revert "include functional and iostream" 34bfbec21 Revert "delete debug statement" dbe661c93 Revert "add includes to the example tpl file." f47e23f1a Revert "Revert "function to concatenate multiple header files together"" e667e2095 Revert "function to concatenate multiple header files together" 600c77bf4 add includes to the example tpl file. d5caca20e delete debug statement 4a554edb8 include functional and iostream e8ad2c26f funcitonal print passes unit test 077d5c4e7 unit test for printing with keyformatter d5a1e6dff function to concatenate multiple header files together git-subtree-dir: wrap git-subtree-split: bae34fac828acb6b17bfe37ebe78f7c9a156ed5a --- DOCS.md | 1 + cmake/GtwrapUtils.cmake | 34 ++++++++++++ gtwrap/interface_parser/classes.py | 13 +++-- gtwrap/pybind_wrapper.py | 5 ++ tests/expected/matlab/inheritance_wrapper.cpp | 52 +++++++++++++++++++ tests/expected/matlab/namespaces_wrapper.cpp | 9 ++++ .../expected/matlab/special_cases_wrapper.cpp | 9 ++++ tests/expected/python/class_pybind.cpp | 2 +- tests/expected/python/inheritance_pybind.cpp | 2 + tests/fixtures/inheritance.i | 3 ++ tests/test_interface_parser.py | 9 ++++ 11 files changed, 134 insertions(+), 5 deletions(-) diff --git a/DOCS.md b/DOCS.md index 00104c123b..c0c4310fa8 100644 --- a/DOCS.md +++ b/DOCS.md @@ -98,6 +98,7 @@ The python wrapper supports keyword arguments for functions/methods. Hence, the - Virtual inheritance - Specify fully-qualified base classes, i.e. `virtual class Derived : ns::Base {` where `ns` is the namespace. - Mark with `virtual` keyword, e.g. `virtual class Base {`, and also `virtual class Derived : ns::Base {`. + - Base classes can be templated, e.g. `virtual class Dog: ns::Animal {};`. This is useful when you want to inherit from specialized classes. - Forward declarations must also be marked virtual, e.g. `virtual class ns::Base;` and also `virtual class ns::Derived;`. - Pure virtual (abstract) classes should list no constructors in the interface file. diff --git a/cmake/GtwrapUtils.cmake b/cmake/GtwrapUtils.cmake index 8479015dcb..3c26e70ae5 100644 --- a/cmake/GtwrapUtils.cmake +++ b/cmake/GtwrapUtils.cmake @@ -103,3 +103,37 @@ macro(gtwrap_get_python_version) configure_python_variables() endmacro() + +# Concatenate multiple wrapper interface headers into one. +# The concatenation will be (re)performed if and only if any interface files +# change. +# +# Arguments: +# ~~~ +# destination: The concatenated master interface header file will be placed here. +# inputs (optional): All the input interface header files +function(combine_interface_headers + destination + #inputs + ) + # check if any interface headers changed + foreach(INTERFACE_FILE ${ARGN}) + if(NOT EXISTS ${destination} OR + ${INTERFACE_FILE} IS_NEWER_THAN ${destination}) + set(UPDATE_INTERFACE TRUE) + endif() + # trigger cmake on file change + set_property(DIRECTORY + APPEND + PROPERTY CMAKE_CONFIGURE_DEPENDS ${INTERFACE_FILE}) + endforeach() + # if so, then update the overall interface file + if (UPDATE_INTERFACE) + file(WRITE ${destination} "") + # append additional interface headers to end of gtdynamics.i + foreach(INTERFACE_FILE ${ARGN}) + file(READ ${INTERFACE_FILE} interface_contents) + file(APPEND ${destination} "${interface_contents}") + endforeach() + endif() +endfunction() diff --git a/gtwrap/interface_parser/classes.py b/gtwrap/interface_parser/classes.py index e2c28d8ed6..255c261371 100644 --- a/gtwrap/interface_parser/classes.py +++ b/gtwrap/interface_parser/classes.py @@ -279,7 +279,7 @@ def __init__(self, elif isinstance(m, Operator): self.operators.append(m) - _parent = COLON + Typename.rule("parent_class") + _parent = COLON + (TemplatedType.rule ^ Typename.rule)("parent_class") rule = ( Optional(Template.rule("template")) # + Optional(VIRTUAL("is_virtual")) # @@ -319,11 +319,16 @@ def __init__( self.is_virtual = is_virtual self.name = name if parent_class: + # If it is in an iterable, extract the parent class. if isinstance(parent_class, Iterable): - self.parent_class = parent_class[0] - else: - self.parent_class = parent_class + parent_class = parent_class[0] + # If the base class is a TemplatedType, + # we want the instantiated Typename + if isinstance(parent_class, TemplatedType): + parent_class = parent_class.typename + + self.parent_class = parent_class else: self.parent_class = '' diff --git a/gtwrap/pybind_wrapper.py b/gtwrap/pybind_wrapper.py index 4b532f6e13..1e0d412a0b 100755 --- a/gtwrap/pybind_wrapper.py +++ b/gtwrap/pybind_wrapper.py @@ -125,6 +125,11 @@ def _wrap_method(self, method, cpp_class, prefix, suffix, method_suffix=""): )) if method.name == 'print': + # Redirect stdout - see pybind docs for why this is a good idea: + # https://pybind11.readthedocs.io/en/stable/advanced/pycpp/utilities.html#capturing-standard-output-from-ostream + ret = ret.replace('self->', 'py::scoped_ostream_redirect output; self->') + + # __repr__() uses print's implementation: type_list = method.args.to_cpp(self.use_boost) if len(type_list) > 0 and type_list[0].strip() == 'string': ret += '''{prefix}.def("__repr__", diff --git a/tests/expected/matlab/inheritance_wrapper.cpp b/tests/expected/matlab/inheritance_wrapper.cpp index f12f6ce574..e68b3a6e4a 100644 --- a/tests/expected/matlab/inheritance_wrapper.cpp +++ b/tests/expected/matlab/inheritance_wrapper.cpp @@ -50,6 +50,8 @@ typedef std::set*> Collector_MyTemplatePoint static Collector_MyTemplatePoint2 collector_MyTemplatePoint2; typedef std::set*> Collector_MyTemplateMatrix; static Collector_MyTemplateMatrix collector_MyTemplateMatrix; +typedef std::set*> Collector_ForwardKinematicsFactor; +static Collector_ForwardKinematicsFactor collector_ForwardKinematicsFactor; void _deleteAllObjects() { @@ -141,6 +143,12 @@ void _deleteAllObjects() collector_MyTemplateMatrix.erase(iter++); anyDeleted = true; } } + { for(Collector_ForwardKinematicsFactor::iterator iter = collector_ForwardKinematicsFactor.begin(); + iter != collector_ForwardKinematicsFactor.end(); ) { + delete *iter; + collector_ForwardKinematicsFactor.erase(iter++); + anyDeleted = true; + } } if(anyDeleted) cout << "WARNING: Wrap modules with variables in the workspace have been reloaded due to\n" @@ -156,6 +164,7 @@ void _inheritance_RTTIRegister() { types.insert(std::make_pair(typeid(MyBase).name(), "MyBase")); types.insert(std::make_pair(typeid(MyTemplatePoint2).name(), "MyTemplatePoint2")); types.insert(std::make_pair(typeid(MyTemplateMatrix).name(), "MyTemplateMatrix")); + types.insert(std::make_pair(typeid(ForwardKinematicsFactor).name(), "ForwardKinematicsFactor")); mxArray *registry = mexGetVariable("global", "gtsamwrap_rttiRegistry"); if(!registry) @@ -555,6 +564,40 @@ void MyTemplateMatrix_Level_34(int nargout, mxArray *out[], int nargin, const mx out[0] = wrap_shared_ptr(boost::make_shared>(MyTemplate::Level(K)),"MyTemplateMatrix", false); } +void Test_set_container_35(int nargout, mxArray *out[], int nargin, const mxArray *in[]) +{ + checkArguments("set_container",nargout,nargin-1,1); + auto obj = unwrap_shared_ptr(in[0], "ptr_Test"); + boost::shared_ptr> container = unwrap_shared_ptr< std::vector >(in[1], "ptr_stdvectorTest"); + obj->set_container(*container); +} + +void ForwardKinematicsFactor_collectorInsertAndMakeBase_35(int nargout, mxArray *out[], int nargin, const mxArray *in[]) +{ + mexAtExit(&_deleteAllObjects); + typedef boost::shared_ptr Shared; + + Shared *self = *reinterpret_cast (mxGetData(in[0])); + collector_ForwardKinematicsFactor.insert(self); + + typedef boost::shared_ptr> SharedBase; + out[0] = mxCreateNumericMatrix(1, 1, mxUINT32OR64_CLASS, mxREAL); + *reinterpret_cast(mxGetData(out[0])) = new SharedBase(*self); +} + +void ForwardKinematicsFactor_deconstructor_37(int nargout, mxArray *out[], int nargin, const mxArray *in[]) +{ + typedef boost::shared_ptr Shared; + checkArguments("delete_ForwardKinematicsFactor",nargout,nargin,1); + Shared *self = *reinterpret_cast(mxGetData(in[0])); + Collector_ForwardKinematicsFactor::iterator item; + item = collector_ForwardKinematicsFactor.find(self); + if(item != collector_ForwardKinematicsFactor.end()) { + delete self; + collector_ForwardKinematicsFactor.erase(item); + } +} + void mexFunction(int nargout, mxArray *out[], int nargin, const mxArray *in[]) { @@ -672,6 +715,15 @@ void mexFunction(int nargout, mxArray *out[], int nargin, const mxArray *in[]) case 34: MyTemplateMatrix_Level_34(nargout, out, nargin-1, in+1); break; + case 35: + Test_set_container_35(nargout, out, nargin-1, in+1); + break; + case 36: + ForwardKinematicsFactor_collectorInsertAndMakeBase_35(nargout, out, nargin-1, in+1); + break; + case 37: + ForwardKinematicsFactor_deconstructor_37(nargout, out, nargin-1, in+1); + break; } } catch(const std::exception& e) { mexErrMsgTxt(("Exception from gtsam:\n" + std::string(e.what()) + "\n").c_str()); diff --git a/tests/expected/matlab/namespaces_wrapper.cpp b/tests/expected/matlab/namespaces_wrapper.cpp index 67bbb00c92..aba5c49eaa 100644 --- a/tests/expected/matlab/namespaces_wrapper.cpp +++ b/tests/expected/matlab/namespaces_wrapper.cpp @@ -55,6 +55,8 @@ typedef std::set*> Collector_MyTemplatePoint static Collector_MyTemplatePoint2 collector_MyTemplatePoint2; typedef std::set*> Collector_MyTemplateMatrix; static Collector_MyTemplateMatrix collector_MyTemplateMatrix; +typedef std::set*> Collector_ForwardKinematicsFactor; +static Collector_ForwardKinematicsFactor collector_ForwardKinematicsFactor; typedef std::set*> Collector_ns1ClassA; static Collector_ns1ClassA collector_ns1ClassA; typedef std::set*> Collector_ns1ClassB; @@ -158,6 +160,12 @@ void _deleteAllObjects() collector_MyTemplateMatrix.erase(iter++); anyDeleted = true; } } + { for(Collector_ForwardKinematicsFactor::iterator iter = collector_ForwardKinematicsFactor.begin(); + iter != collector_ForwardKinematicsFactor.end(); ) { + delete *iter; + collector_ForwardKinematicsFactor.erase(iter++); + anyDeleted = true; + } } { for(Collector_ns1ClassA::iterator iter = collector_ns1ClassA.begin(); iter != collector_ns1ClassA.end(); ) { delete *iter; @@ -209,6 +217,7 @@ void _namespaces_RTTIRegister() { types.insert(std::make_pair(typeid(MyBase).name(), "MyBase")); types.insert(std::make_pair(typeid(MyTemplatePoint2).name(), "MyTemplatePoint2")); types.insert(std::make_pair(typeid(MyTemplateMatrix).name(), "MyTemplateMatrix")); + types.insert(std::make_pair(typeid(ForwardKinematicsFactor).name(), "ForwardKinematicsFactor")); mxArray *registry = mexGetVariable("global", "gtsamwrap_rttiRegistry"); if(!registry) diff --git a/tests/expected/matlab/special_cases_wrapper.cpp b/tests/expected/matlab/special_cases_wrapper.cpp index b0ee1bcf83..d79a41ace5 100644 --- a/tests/expected/matlab/special_cases_wrapper.cpp +++ b/tests/expected/matlab/special_cases_wrapper.cpp @@ -58,6 +58,8 @@ typedef std::set*> Collector_MyTemplatePoint static Collector_MyTemplatePoint2 collector_MyTemplatePoint2; typedef std::set*> Collector_MyTemplateMatrix; static Collector_MyTemplateMatrix collector_MyTemplateMatrix; +typedef std::set*> Collector_ForwardKinematicsFactor; +static Collector_ForwardKinematicsFactor collector_ForwardKinematicsFactor; typedef std::set*> Collector_ns1ClassA; static Collector_ns1ClassA collector_ns1ClassA; typedef std::set*> Collector_ns1ClassB; @@ -169,6 +171,12 @@ void _deleteAllObjects() collector_MyTemplateMatrix.erase(iter++); anyDeleted = true; } } + { for(Collector_ForwardKinematicsFactor::iterator iter = collector_ForwardKinematicsFactor.begin(); + iter != collector_ForwardKinematicsFactor.end(); ) { + delete *iter; + collector_ForwardKinematicsFactor.erase(iter++); + anyDeleted = true; + } } { for(Collector_ns1ClassA::iterator iter = collector_ns1ClassA.begin(); iter != collector_ns1ClassA.end(); ) { delete *iter; @@ -244,6 +252,7 @@ void _special_cases_RTTIRegister() { types.insert(std::make_pair(typeid(MyBase).name(), "MyBase")); types.insert(std::make_pair(typeid(MyTemplatePoint2).name(), "MyTemplatePoint2")); types.insert(std::make_pair(typeid(MyTemplateMatrix).name(), "MyTemplateMatrix")); + types.insert(std::make_pair(typeid(ForwardKinematicsFactor).name(), "ForwardKinematicsFactor")); mxArray *registry = mexGetVariable("global", "gtsamwrap_rttiRegistry"); if(!registry) diff --git a/tests/expected/python/class_pybind.cpp b/tests/expected/python/class_pybind.cpp index 68f7a42e43..43705778bb 100644 --- a/tests/expected/python/class_pybind.cpp +++ b/tests/expected/python/class_pybind.cpp @@ -55,7 +55,7 @@ PYBIND11_MODULE(class_py, m_) { .def("create_ptrs",[](Test* self){return self->create_ptrs();}) .def("create_MixedPtrs",[](Test* self){return self->create_MixedPtrs();}) .def("return_ptrs",[](Test* self, std::shared_ptr p1, std::shared_ptr p2){return self->return_ptrs(p1, p2);}, py::arg("p1"), py::arg("p2")) - .def("print_",[](Test* self){ self->print();}) + .def("print_",[](Test* self){ py::scoped_ostream_redirect output; self->print();}) .def("__repr__", [](const Test &a) { gtsam::RedirectCout redirect; diff --git a/tests/expected/python/inheritance_pybind.cpp b/tests/expected/python/inheritance_pybind.cpp index 6f34d39b64..d6cd77ca0c 100644 --- a/tests/expected/python/inheritance_pybind.cpp +++ b/tests/expected/python/inheritance_pybind.cpp @@ -54,6 +54,8 @@ PYBIND11_MODULE(inheritance_py, m_) { .def("return_ptrs",[](MyTemplate* self, const std::shared_ptr p1, const std::shared_ptr p2){return self->return_ptrs(p1, p2);}, py::arg("p1"), py::arg("p2")) .def_static("Level",[](const gtsam::Matrix& K){return MyTemplate::Level(K);}, py::arg("K")); + py::class_, std::shared_ptr>(m_, "ForwardKinematicsFactor"); + #include "python/specializations.h" diff --git a/tests/fixtures/inheritance.i b/tests/fixtures/inheritance.i index e104c5b992..ddf9745dfc 100644 --- a/tests/fixtures/inheritance.i +++ b/tests/fixtures/inheritance.i @@ -22,3 +22,6 @@ virtual class MyTemplate : MyBase { static This Level(const T& K); }; + + +virtual class ForwardKinematicsFactor : gtsam::BetweenFactor {}; diff --git a/tests/test_interface_parser.py b/tests/test_interface_parser.py index 23f931fddc..d3c2ad52aa 100644 --- a/tests/test_interface_parser.py +++ b/tests/test_interface_parser.py @@ -388,6 +388,15 @@ def test_class_inheritance(self): ret.parent_class.namespaces) self.assertTrue(ret.is_virtual) + ret = Class.rule.parseString( + "class ForwardKinematicsFactor : gtsam::BetweenFactor {};" + )[0] + self.assertEqual("ForwardKinematicsFactor", ret.name) + self.assertEqual("BetweenFactor", ret.parent_class.name) + self.assertEqual(["gtsam"], ret.parent_class.namespaces) + self.assertEqual("Pose3", ret.parent_class.instantiations[0].name) + self.assertEqual(["gtsam"], ret.parent_class.instantiations[0].namespaces) + def test_include(self): """Test for include statements.""" include = Include.rule.parseString( From 2257a37184473662c7f7ef9cb3add3f2fa637845 Mon Sep 17 00:00:00 2001 From: Gerry Chen Date: Mon, 12 Apr 2021 21:50:23 -0400 Subject: [PATCH 2/3] include pybind::iostream to the python wrapper cpp template --- python/gtsam/gtsam.tpl | 1 + python/gtsam_unstable/gtsam_unstable.tpl | 1 + 2 files changed, 2 insertions(+) diff --git a/python/gtsam/gtsam.tpl b/python/gtsam/gtsam.tpl index 60b3d1fd05..26c220a8f2 100644 --- a/python/gtsam/gtsam.tpl +++ b/python/gtsam/gtsam.tpl @@ -13,6 +13,7 @@ #include #include #include +#include #include "gtsam/config.h" #include "gtsam/base/serialization.h" #include "gtsam/nonlinear/utilities.h" // for RedirectCout. diff --git a/python/gtsam_unstable/gtsam_unstable.tpl b/python/gtsam_unstable/gtsam_unstable.tpl index 111e46d5e7..356f37c427 100644 --- a/python/gtsam_unstable/gtsam_unstable.tpl +++ b/python/gtsam_unstable/gtsam_unstable.tpl @@ -13,6 +13,7 @@ #include #include #include +#include #include "gtsam/base/serialization.h" #include "gtsam/nonlinear/utilities.h" // for RedirectCout. From 24755c1845575ac5df79e3ac336356b02911a2fb Mon Sep 17 00:00:00 2001 From: Gerry Chen Date: Mon, 12 Apr 2021 22:17:37 -0400 Subject: [PATCH 3/3] documentation about wrap update instructions --- matlab/README.md | 2 ++ python/README.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/matlab/README.md b/matlab/README.md index 39053364d3..25628b6338 100644 --- a/matlab/README.md +++ b/matlab/README.md @@ -8,6 +8,8 @@ The interface to the toolbox is generated automatically by the wrap tool which d The tool generates matlab proxy objects together with all the native functions for wrapping and unwrapping scalar and non scalar types and objects. The interface generated by the wrap tool also redirects the standard output stream (cout) to matlab's console. +For instructions on updating the version of the [wrap library](https://github.com/borglab/wrap) included in GTSAM to the latest version, please refer to the [wrap README](https://github.com/borglab/wrap/blob/master/README.md#git-subtree-and-contributing) + ## Ubuntu If you have a newer Ubuntu system (later than 10.04), you must make a small modification to your MATLAB installation, due to MATLAB being distributed with an old version of the C++ standard library. Delete or rename all files starting with `libstdc++` in your MATLAB installation directory, in paths: diff --git a/python/README.md b/python/README.md index 6e2a30d2e4..c485d12bdc 100644 --- a/python/README.md +++ b/python/README.md @@ -4,6 +4,8 @@ This is the Python wrapper around the GTSAM C++ library. We use our custom [wrap library](https://github.com/borglab/wrap) to generate the bindings to the underlying C++ code. +For instructions on updating the version of the [wrap library](https://github.com/borglab/wrap) included in GTSAM to the latest version, please refer to the [wrap README](https://github.com/borglab/wrap/blob/master/README.md#git-subtree-and-contributing) + ## Requirements - If you want to build the GTSAM python library for a specific python version (eg 3.6),