From d7d35876f1ee67dc928414c298304f3e14e34a92 Mon Sep 17 00:00:00 2001 From: Jing Dong Date: Mon, 3 Oct 2016 19:11:44 -0400 Subject: [PATCH 01/10] jacobians for logmap and expmap --- gtsam/base/ProductLieGroup.h | 22 ++++++++++++++++------ tests/testLie.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/gtsam/base/ProductLieGroup.h b/gtsam/base/ProductLieGroup.h index 463b5f5d9..87ead88f0 100644 --- a/gtsam/base/ProductLieGroup.h +++ b/gtsam/base/ProductLieGroup.h @@ -138,17 +138,27 @@ public: return ProductLieGroup(g,h); } static ProductLieGroup Expmap(const TangentVector& v, ChartJacobian Hv = boost::none) { - if (Hv) throw std::runtime_error("ProductLieGroup::Expmap derivatives not implemented yet"); - G g = traits::Expmap(v.template head()); - H h = traits::Expmap(v.template tail()); + Jacobian1 D_g_first; Jacobian2 D_h_second; + G g = traits::Expmap(v.template head(), Hv ? &D_g_first : 0); + H h = traits::Expmap(v.template tail(), Hv ? &D_h_second : 0); + if (Hv) { + Hv->setZero(); + Hv->template topLeftCorner() = D_g_first; + Hv->template bottomRightCorner() = D_h_second; + } return ProductLieGroup(g,h); } static TangentVector Logmap(const ProductLieGroup& p, ChartJacobian Hp = boost::none) { - if (Hp) throw std::runtime_error("ProductLieGroup::Logmap derivatives not implemented yet"); - typename traits::TangentVector v1 = traits::Logmap(p.first); - typename traits::TangentVector v2 = traits::Logmap(p.second); + Jacobian1 D_g_first; Jacobian2 D_h_second; + typename traits::TangentVector v1 = traits::Logmap(p.first, Hp ? &D_g_first : 0); + typename traits::TangentVector v2 = traits::Logmap(p.second, Hp ? &D_h_second : 0); TangentVector v; v << v1, v2; + if (Hp) { + Hp->setZero(); + Hp->template topLeftCorner() = D_g_first; + Hp->template bottomRightCorner() = D_h_second; + } return v; } ProductLieGroup expmap(const TangentVector& v) const { diff --git a/tests/testLie.cpp b/tests/testLie.cpp index a134a899c..2d8a0b975 100644 --- a/tests/testLie.cpp +++ b/tests/testLie.cpp @@ -102,6 +102,33 @@ TEST( testProduct, inverse ) { EXPECT(assert_equal(numericH1, actH1, tol)); } +/* ************************************************************************* */ +Product expmap_proxy(const Vector5& vec) { + return Product::Expmap(vec); +} +TEST( testProduct, Expmap ) { + Vector5 vec; + vec << 1, 2, 0.1, 0.2, 0.3; + + Matrix actH; + Product::Expmap(vec, actH); + Matrix numericH = numericalDerivative11(expmap_proxy, vec); + EXPECT(assert_equal(numericH, actH, tol)); +} + +/* ************************************************************************* */ +Vector5 logmap_proxy(const Product& p) { + return Product::Logmap(p); +} +TEST( testProduct, Logmap ) { + Product state(Point2(1, 2), Pose2(3, 4, 5)); + + Matrix actH; + Product::Logmap(state, actH); + Matrix numericH = numericalDerivative11(logmap_proxy, state); + EXPECT(assert_equal(numericH, actH, tol)); +} + //****************************************************************************** int main() { TestResult tr; From 5fa4abf99cb89c0c9af9dc198b49f5e936a4f6f4 Mon Sep 17 00:00:00 2001 From: Jing Dong Date: Thu, 8 Dec 2016 15:19:18 -0500 Subject: [PATCH 02/10] fix optional jacobians --- gtsam/base/VectorSpace.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gtsam/base/VectorSpace.h b/gtsam/base/VectorSpace.h index 5456ae7f5..43644b5c4 100644 --- a/gtsam/base/VectorSpace.h +++ b/gtsam/base/VectorSpace.h @@ -138,14 +138,14 @@ struct VectorSpaceImpl { } static Class Compose(const Class& v1, const Class& v2, ChartJacobian H1, - ChartJacobian H2) { + ChartJacobian H2 = boost::none) { if (H1) *H1 = Eye(v1); if (H2) *H2 = Eye(v2); return v1 + v2; } static Class Between(const Class& v1, const Class& v2, ChartJacobian H1, - ChartJacobian H2) { + ChartJacobian H2 = boost::none) { if (H1) *H1 = - Eye(v1); if (H2) *H2 = Eye(v2); return v2 - v1; From 21aa7a2e856c535b0aeaf3643032e94b12e5465d Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Tue, 17 Jan 2017 10:12:00 +0000 Subject: [PATCH 03/10] Fixed unrwapping of scalar references. --- wrap/Argument.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/wrap/Argument.cpp b/wrap/Argument.cpp index 52d9ca0b5..a47c6711c 100644 --- a/wrap/Argument.cpp +++ b/wrap/Argument.cpp @@ -75,16 +75,17 @@ void Argument::matlab_unwrap(FileWriter& file, const string& matlabName) const { string cppType = type.qualifiedName("::"); string matlabUniqueType = type.qualifiedName(); + bool isNotScalar = !Argument::isScalar(); if (is_ptr && type.category != Qualified::EIGEN) // A pointer: emit an "unwrap_shared_ptr" call which returns a pointer file.oss << "boost::shared_ptr<" << cppType << "> " << name << " = unwrap_shared_ptr< "; - else if (is_ref && type.category != Qualified::EIGEN) + else if (is_ref && isNotScalar && type.category != Qualified::EIGEN) // A reference: emit an "unwrap_shared_ptr" call and de-reference the pointer file.oss << cppType << "& " << name << " = *unwrap_shared_ptr< "; else - // Not a pointer or a reference: emit an "unwrap" call + // Not a pointer, or a reference to a scalar type. Therefore, emit an "unwrap" call // unwrap is specified in matlab.h as a series of template specializations // that know how to unpack the expected MATLAB object // example: double tol = unwrap< double >(in[2]); @@ -92,7 +93,7 @@ void Argument::matlab_unwrap(FileWriter& file, const string& matlabName) const { file.oss << cppType << " " << name << " = unwrap< "; file.oss << cppType << " >(" << matlabName; - if( (is_ptr || is_ref) && type.category != Qualified::EIGEN) + if( (is_ptr || is_ref) && isNotScalar && type.category != Qualified::EIGEN) file.oss << ", \"ptr_" << matlabUniqueType << "\""; file.oss << ");" << endl; } From f50c3c0d51fa7f4d015ec84e5a536a0cc4ffd918 Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Tue, 17 Jan 2017 16:53:28 +0000 Subject: [PATCH 04/10] Use INSTALL_NAME_DIR to embed names in the dylibs and avoid linker errors.y --- cmake/GtsamMatlabWrap.cmake | 3 ++- gtsam/CMakeLists.txt | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmake/GtsamMatlabWrap.cmake b/cmake/GtsamMatlabWrap.cmake index a9e04a01a..3165a307a 100644 --- a/cmake/GtsamMatlabWrap.cmake +++ b/cmake/GtsamMatlabWrap.cmake @@ -70,7 +70,8 @@ function(wrap_library_internal interfaceHeader linkLibraries extraIncludeDirs ex set(mexModuleExt mexglx) endif() elseif(APPLE) - set(mexModuleExt mexmaci64) + set(mexModuleExt mexmaci64) + set(CMAKE_INSTALL_DIR_NAME ${GTSAM_TOOLBOX_INSTALL_PATH}) elseif(MSVC) if(CMAKE_CL_64) set(mexModuleExt mexw64) diff --git a/gtsam/CMakeLists.txt b/gtsam/CMakeLists.txt index 8c1d8bb43..63528d3b4 100644 --- a/gtsam/CMakeLists.txt +++ b/gtsam/CMakeLists.txt @@ -113,6 +113,10 @@ if (GTSAM_BUILD_STATIC_LIBRARY) PREFIX "lib" COMPILE_DEFINITIONS GTSAM_IMPORT_STATIC) endif() + if(APPLE) # Set the + set_target_properties(gtsam PROPERTIES + INSTALL_DIR_NAME ${CMAKE_INSTALL_PREFIX}/lib) + endif() install(TARGETS gtsam EXPORT GTSAM-exports ARCHIVE DESTINATION lib) list(APPEND GTSAM_EXPORTED_TARGETS gtsam) set(GTSAM_EXPORTED_TARGETS "${GTSAM_EXPORTED_TARGETS}" PARENT_SCOPE) From 2a4aa76e7e1d8693c43a10fc59b2ba0873838b1a Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Wed, 18 Jan 2017 18:58:48 +0000 Subject: [PATCH 05/10] Added instructions on the iostream and the wrapper. Also added instructions on how to enable the toolbox.y --- matlab/README-gtsam-toolbox.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/matlab/README-gtsam-toolbox.txt b/matlab/README-gtsam-toolbox.txt index 6f74f9806..127eb57ed 100644 --- a/matlab/README-gtsam-toolbox.txt +++ b/matlab/README-gtsam-toolbox.txt @@ -7,8 +7,12 @@ http://borg.cc.gatech.edu/projects/gtsam ================================================================================ This is the GTSAM MATLAB toolbox, a MATLAB wrapper around the GTSAM C++ -library. +library. To build it, enable GTSAM_INSTALL_MATLAB_TOOLBOX in CMake. +The interface is created automatically by the wrap tool, which +directly parses C++ header files and generates matlab proxy objects +and wrapping and unwrapping code. The wrap tool also redirects the +standard "cout" stream to matlab's console. ---------------------------------------- Note about newer Ubuntu versions unsupported by MATLAB (later than 10.04) From f802099bfdcbeec4239ad9283e507324ee83d9e2 Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Wed, 18 Jan 2017 19:13:25 +0000 Subject: [PATCH 06/10] Tidied up the text to make it a bit clearer / less ambiguous.y --- matlab/README-gtsam-toolbox.txt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/matlab/README-gtsam-toolbox.txt b/matlab/README-gtsam-toolbox.txt index 127eb57ed..a1691be32 100644 --- a/matlab/README-gtsam-toolbox.txt +++ b/matlab/README-gtsam-toolbox.txt @@ -9,10 +9,12 @@ http://borg.cc.gatech.edu/projects/gtsam This is the GTSAM MATLAB toolbox, a MATLAB wrapper around the GTSAM C++ library. To build it, enable GTSAM_INSTALL_MATLAB_TOOLBOX in CMake. -The interface is created automatically by the wrap tool, which -directly parses C++ header files and generates matlab proxy objects -and wrapping and unwrapping code. The wrap tool also redirects the -standard "cout" stream to matlab's console. +The interface to the toolbox is generated automatically by the wrap +tool which directly parses C++ header files. 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. ---------------------------------------- Note about newer Ubuntu versions unsupported by MATLAB (later than 10.04) From d8d7c5618a11076a1a56a7e3232d4e195f70b1c8 Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Thu, 19 Jan 2017 01:49:12 +0000 Subject: [PATCH 07/10] Generate an error and exit if trying to wrap a non-const scalar reference. --- wrap/Argument.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/wrap/Argument.cpp b/wrap/Argument.cpp index a47c6711c..6badf7794 100644 --- a/wrap/Argument.cpp +++ b/wrap/Argument.cpp @@ -77,6 +77,12 @@ void Argument::matlab_unwrap(FileWriter& file, const string& matlabName) const { string matlabUniqueType = type.qualifiedName(); bool isNotScalar = !Argument::isScalar(); + // We cannot handle scalar non const references + if (!isNotScalar && is_ref && !is_const) { + cerr << "Cannot wrap a scalar non-const reference" << endl; + exit(-1); + } + if (is_ptr && type.category != Qualified::EIGEN) // A pointer: emit an "unwrap_shared_ptr" call which returns a pointer file.oss << "boost::shared_ptr<" << cppType << "> " << name From d1422ac9216af6fa53b49932cb258a195c1b22ba Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Thu, 19 Jan 2017 09:28:04 +0000 Subject: [PATCH 08/10] Reverted change to make files to make the pull request clean. --- cmake/GtsamMatlabWrap.cmake | 3 +-- gtsam/CMakeLists.txt | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/cmake/GtsamMatlabWrap.cmake b/cmake/GtsamMatlabWrap.cmake index 3165a307a..a9e04a01a 100644 --- a/cmake/GtsamMatlabWrap.cmake +++ b/cmake/GtsamMatlabWrap.cmake @@ -70,8 +70,7 @@ function(wrap_library_internal interfaceHeader linkLibraries extraIncludeDirs ex set(mexModuleExt mexglx) endif() elseif(APPLE) - set(mexModuleExt mexmaci64) - set(CMAKE_INSTALL_DIR_NAME ${GTSAM_TOOLBOX_INSTALL_PATH}) + set(mexModuleExt mexmaci64) elseif(MSVC) if(CMAKE_CL_64) set(mexModuleExt mexw64) diff --git a/gtsam/CMakeLists.txt b/gtsam/CMakeLists.txt index 63528d3b4..8c1d8bb43 100644 --- a/gtsam/CMakeLists.txt +++ b/gtsam/CMakeLists.txt @@ -113,10 +113,6 @@ if (GTSAM_BUILD_STATIC_LIBRARY) PREFIX "lib" COMPILE_DEFINITIONS GTSAM_IMPORT_STATIC) endif() - if(APPLE) # Set the - set_target_properties(gtsam PROPERTIES - INSTALL_DIR_NAME ${CMAKE_INSTALL_PREFIX}/lib) - endif() install(TARGETS gtsam EXPORT GTSAM-exports ARCHIVE DESTINATION lib) list(APPEND GTSAM_EXPORTED_TARGETS gtsam) set(GTSAM_EXPORTED_TARGETS "${GTSAM_EXPORTED_TARGETS}" PARENT_SCOPE) From 6a109aca9bae0b858099f59d084a0254841397ac Mon Sep 17 00:00:00 2001 From: Simon Julier Date: Fri, 20 Jan 2017 01:58:59 +0000 Subject: [PATCH 09/10] Throw an exception rather than call exit. --- wrap/Argument.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wrap/Argument.cpp b/wrap/Argument.cpp index 6badf7794..01da3a756 100644 --- a/wrap/Argument.cpp +++ b/wrap/Argument.cpp @@ -79,8 +79,7 @@ void Argument::matlab_unwrap(FileWriter& file, const string& matlabName) const { // We cannot handle scalar non const references if (!isNotScalar && is_ref && !is_const) { - cerr << "Cannot wrap a scalar non-const reference" << endl; - exit(-1); + throw std::runtime_error("Cannot unwrap a scalar non-const reference"); } if (is_ptr && type.category != Qualified::EIGEN) From 5482f1f5eb720c9668373910ca6a78289fffc12a Mon Sep 17 00:00:00 2001 From: Ellon Mendes Date: Thu, 9 Jun 2016 13:55:36 +0200 Subject: [PATCH 10/10] [python] Make python library hidden by renaming gtsampy.so to _gtsampy.so This commit also fixes a naming problem of the python .so module (_libgtsam_python.so -> _gtsampy.so) --- python/gtsam/__init__.py | 2 +- python/handwritten/CMakeLists.txt | 6 +++--- python/handwritten/exportgtsam.cpp | 2 +- python/setup.py.in | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/python/gtsam/__init__.py b/python/gtsam/__init__.py index 2e95ea33a..2d7ac72f7 100644 --- a/python/gtsam/__init__.py +++ b/python/gtsam/__init__.py @@ -1 +1 @@ -from gtsampy import * +from _gtsampy import * diff --git a/python/handwritten/CMakeLists.txt b/python/handwritten/CMakeLists.txt index fd022dd17..689354b4e 100644 --- a/python/handwritten/CMakeLists.txt +++ b/python/handwritten/CMakeLists.txt @@ -12,7 +12,7 @@ endforeach() add_library(gtsam_python SHARED exportgtsam.cpp ${gtsam_python_srcs}) string(TOUPPER "${CMAKE_BUILD_TYPE}" build_type_toupper) set_target_properties(gtsam_python PROPERTIES - OUTPUT_NAME gtsampy + OUTPUT_NAME _gtsampy PREFIX "" ${build_type_toupper}_POSTFIX "" SKIP_BUILD_RPATH TRUE @@ -31,11 +31,11 @@ target_link_libraries(gtsam_python # Cause the library to be output in the correct directory. # TODO: Change below to work on different systems (currently works only with Linux) -set(output_path ${CMAKE_CURRENT_BINARY_DIR}/../gtsam/_libgtsam_python.so) +set(output_path ${CMAKE_CURRENT_BINARY_DIR}/../gtsam/_gtsampy.so) add_custom_command( OUTPUT ${output_path} DEPENDS gtsam_python COMMAND ${CMAKE_COMMAND} -E copy $ ${output_path} - COMMENT "Copying extension module to python/gtsam/_libgtsam_python.so" + COMMENT "Copying extension module to python/gtsam/_gtsampy.so" ) add_custom_target(copy_gtsam_python_module ALL DEPENDS ${output_path}) \ No newline at end of file diff --git a/python/handwritten/exportgtsam.cpp b/python/handwritten/exportgtsam.cpp index 94dc10e56..8fa7e0fdd 100644 --- a/python/handwritten/exportgtsam.cpp +++ b/python/handwritten/exportgtsam.cpp @@ -62,7 +62,7 @@ void registerNumpyEigenConversions(); //-----------------------------------// -BOOST_PYTHON_MODULE(gtsampy){ +BOOST_PYTHON_MODULE(_gtsampy){ // NOTE: We need to call import_array1() instead of import_array() to support both python 2 // and 3. The reason is that BOOST_PYTHON_MODULE puts all its contents in a function diff --git a/python/setup.py.in b/python/setup.py.in index d3b5fcde4..8b2de7352 100644 --- a/python/setup.py.in +++ b/python/setup.py.in @@ -11,5 +11,5 @@ setup(name='gtsam', package_dir={ '': '${CMAKE_CURRENT_SOURCE_DIR}' }, packages=['gtsam', 'gtsam_utils', 'gtsam_examples', 'gtsam_tests'], #package_data={'gtsam' : ['_libgtsam_python.so']}, # location of .so file is relative to package_dir - data_files=[('${PY_INSTALL_FOLDER}/gtsam/', ['gtsam/_libgtsam_python.so'])], # location of .so file relative to setup.py + data_files=[('${PY_INSTALL_FOLDER}/gtsam/', ['gtsam/_gtsampy.so'])], # location of .so file relative to setup.py )