Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matlab Wrapper Function 'matlab_wrap' Not Working For GPMP2 #136

Closed
mattking-smith opened this issue Dec 10, 2021 · 10 comments
Closed

Matlab Wrapper Function 'matlab_wrap' Not Working For GPMP2 #136

mattking-smith opened this issue Dec 10, 2021 · 10 comments
Assignees
Labels
bug Something isn't working cmake CMake issue matlab

Comments

@mattking-smith
Copy link
Contributor

mattking-smith commented Dec 10, 2021

Description

I am trying to use the wrap repository to generate the MATLAB interface to the GPMP2 repository, but I am running into an error when trying to run the matlab_wrap() function. I have been able to create this MATLAB interface with the help of @gchenfc (which I currently have working on a development computer), but that was some time ago and I am worried possibly there may have been changes to the wrapper implementation.

Steps to reproduce error

  1. In the command prompt run:
git clone https://github.com/gtrll/gpmp2.git 
cd gpmp2 && mkdir build && cd build
sudo cmake -DGPMP2_BUILD_MATLAB_TOOLBOX:=ON ..
  1. However, I found that that this produced and error due to and outdated CMakeLists.txt file.
    Specifically, in lines 66-74 of the GPMP2 CMakeLists.txt has:
# Wrapping to MATLAB
if(GPMP2_BUILD_MATLAB_TOOLBOX)
  # wrap
  include(GtsamMatlabWrap)
  wrap_and_install_library(gpmp2.h ${PROJECT_NAME} "${CMAKE_CURRENT_SOURCE_DIR}" "")
  
  # install matlab functions and scripts
  add_subdirectory(matlab)
endif()

So I changed these lines in the CMakeLists.txt file to the following:

find_package(gtwrap REQUIRED)

# Wrapping to MATLAB
if(GPMP2_BUILD_MATLAB_TOOLBOX)

  include(MatlabWrap)
  matlab_wrap(gpmp2.h ${PROJECT_NAME} "${CMAKE_CURRENT_SOURCE_DIR}" "" "")
  
  # install matlab functions and scripts
  add_subdirectory(matlab)
endif()

However, after making this change, calling sudo cmake -DGPMP2_BUILD_MATLAB_TOOLBOX:=ON .. in the command prompt results in the error during the cmake generation:

CMake Error at CMakeLists.txt:71 (matlab_wrap):
  matlab_wrap Function invoked with incorrect arguments for function named:
  matlab_wrap

Outstanding Question

Am I using the matlab_wrap() function incorrectly? On my computer with the work GPMP2 + GTSAM + Matlab toolbox, my CMakeLists.txt file appears as the way I modified the CMakeLists.txt.

Has there been a modification to the way I am suppose to use matlab_wrap()?

Any thoughts on this @gchenfc?

Environment

Linux OS: Ubuntu 20.04
MATLAB version: 2021a
Standard GPMP2 + MATLAB Toolbox build.

@varunagrawal
Copy link
Collaborator

Yes the CMake function has changed considerably. You should read the docstrings in the CMake file and see what needs to be updated. Should be pretty straightforward.

@varunagrawal
Copy link
Collaborator

I believe there is another argument added, which was done to support multiple .i files in Matlab.

This is pretty much a GPMP issue, but this issue can help us figure out various kinks.

@varunagrawal
Copy link
Collaborator

wrap_and_install_library(gpmp2.h ${PROJECT_NAME} "${CMAKE_CURRENT_SOURCE_DIR}" "")
should be updated to:
matlab_wrap(gpmp2.h ${PROJECT_NAME} "gtsam;${PROJECT_NAME}" "" "" "")

@mattking-smith
Copy link
Contributor Author

wrap_and_install_library(gpmp2.h ${PROJECT_NAME} "${CMAKE_CURRENT_SOURCE_DIR}" "") should be updated to: matlab_wrap(gpmp2.h ${PROJECT_NAME} "gtsam;${PROJECT_NAME}" "" "" "")

Thank you very much for this response, I'll try it out.

@mattking-smith
Copy link
Contributor Author

I believe there is another argument added, which was done to support multiple .i files in Matlab.

This is pretty much a GPMP issue, but this issue can help us figure out various kinks.

I am going to keep playing around with this so I can ultimately make a PR on the GPMP2 repository so it can work in harmony with the wrap code as is.

@mattking-smith
Copy link
Contributor Author

wrap_and_install_library(gpmp2.h ${PROJECT_NAME} "${CMAKE_CURRENT_SOURCE_DIR}" "") should be updated to: matlab_wrap(gpmp2.h ${PROJECT_NAME} "gtsam;${PROJECT_NAME}" "" "" "")

So I tried building with the CMakeLists.txt as you suggested and I was given the following error:

sudo cmake -DGPMP2_BUILD_MATLAB_TOOLBOX:=ON ..
.
.
.
-- Installing Matlab Toolbox to 
CMake Error at /usr/local/lib/cmake/gtwrap/MatlabWrap.cmake:360 (install):
  install DIRECTORY given no DESTINATION!
Call Stack (most recent call first):
  /usr/local/lib/cmake/gtwrap/MatlabWrap.cmake:93 (install_wrapped_library_internal)
  /usr/local/lib/cmake/gtwrap/MatlabWrap.cmake:68 (wrap_and_install_library)
  CMakeLists.txt:73 (matlab_wrap)


CMake Error at /usr/local/lib/cmake/gtwrap/MatlabWrap.cmake:365 (install):
  install TARGETS given no LIBRARY DESTINATION for module target
  "gpmp2_matlab_wrapper".
Call Stack (most recent call first):
  /usr/local/lib/cmake/gtwrap/MatlabWrap.cmake:93 (install_wrapped_library_internal)
  /usr/local/lib/cmake/gtwrap/MatlabWrap.cmake:68 (wrap_and_install_library)
  CMakeLists.txt:73 (matlab_wrap)

So then I tried running the command:
sudo cmake -DGPMP2_BUILD_MATLAB_TOOLBOX:=ON -DGTSAM_TOOLBOX_INSTALL_PATH= $HOME/gpmp2_toolbox ..
But this did not make a difference and I ended up with the same error. So finally I tried manually editing the CMakeCashe.txt file generated by the CMakeLists.txt file because I saw that the GTSAM toolbox path was not set, i.e. GTSAM_TOOLBOX_INSTALL_PATH:= to the following: GTSAM_TOOLBOX_INSTALL_PATH:PATH=/home/matt/gpmp2_toolbox, but this again did not fix the issue.

Any idea what arguments I am missing here?

@gchenfc
Copy link
Member

gchenfc commented Dec 10, 2021

I think the arguments are ok, just the cmake variable should be WRAP_TOOLBOX_INSTALL_PATH instead of GTSAM_TOOLBOX_INSTALL_PATH. You can check in gtsam's matlab.cmake for reference:

https://github.com/borglab/gtsam/blob/866d6b1fa1a68a8f4afdb084178fda07493d914d/matlab/CMakeLists.txt#L25-L34

if(NOT GTSAM_TOOLBOX_INSTALL_PATH)
  set(GTSAM_TOOLBOX_INSTALL_PATH "${CMAKE_INSTALL_PREFIX}/gtsam_toolbox")
endif()


set(WRAP_MEX_BUILD_STATIC_MODULE ${GTSAM_MEX_BUILD_STATIC_MODULE})
set(WRAP_BUILD_MEX_BINARY_FLAGS ${GTSAM_BUILD_MEX_BINARY_FLAGS})
set(WRAP_TOOLBOX_INSTALL_PATH ${GTSAM_TOOLBOX_INSTALL_PATH})
set(WRAP_CUSTOM_MATLAB_PATH ${GTSAM_CUSTOM_MATLAB_PATH})
set(WRAP_BUILD_TYPE_POSTFIXES ${GTSAM_BUILD_TYPE_POSTFIXES})

Perhaps you can add something similar in gpmp2:

if(NOT GPMP2_TOOLBOX_INSTALL_PATH)
  set(GPMP2_TOOLBOX_INSTALL_PATH "${CMAKE_INSTALL_PREFIX}/gpmp2_toolbox")
endif()

set(WRAP_TOOLBOX_INSTALL_PATH ${GTSAM_TOOLBOX_INSTALL_PATH})

By the way just for future debugging tips, if you look at MatlabWrap.cmake around line 360, you'll see that a little bit earlier it says
message(STATUS "Installing Matlab Toolbox to ${WRAP_TOOLBOX_INSTALL_PATH}")
and you can see in your output this is an empty string. Then in line 360 it's using
DESTINATION ${WRAP_TOOLBOX_INSTALL_PATH}
so this is cmake's INSTALL command complaining that you didn't give a destination (empty string)

@mattking-smith
Copy link
Contributor Author

By the way just for future debugging tips, if you look at MatlabWrap.cmake around line 360, you'll see that a little bit earlier it says message(STATUS "Installing Matlab Toolbox to ${WRAP_TOOLBOX_INSTALL_PATH}") and you can see in your output this is an empty string. Then in line 360 it's using DESTINATION ${WRAP_TOOLBOX_INSTALL_PATH} so this is cmake's INSTALL command complaining that you didn't give a destination (empty string)

Noting this comment, I have successfully been able to use the current wrapper library with GPMP2 to wrap it into the MATLAB environment. However before we close this issue, I just want to make sure that the MATLAB segmentation fault I am experiencing (outlined in gtrll/gpmp2#54) is not due to the wrapper, but on the GPMP2 side of things.

If either of you could check out the steps I outline in gtrll/gpmp2#54 and the code I had posted in
gtrll/gpmp2#55 due to compiling issues definition changes in GTSAM's Eigen and Pose Transform, I'd appreciate it. Again, doing so would allow us to close this issue.

@mattking-smith
Copy link
Contributor Author

mattking-smith commented Dec 14, 2021

I have been able to successfully wrap and rebuild GPMP2 with this current wrapper version. I am going to close this issue, as the segmentation faults were a result of a change in the gpmp2.h file between different versions, as noted in gtrll/gpmp2#54.

@varunagrawal
Copy link
Collaborator

Amazing stuff! @gchenfc is the man and soon @mattking-smith will be our resident Matlab wrapper expert. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cmake CMake issue matlab
Projects
None yet
Development

No branches or pull requests

3 participants