borglab / wrap

Wrapper generator for C++ projects with multiple language support
Other
30 stars 10 forks source link

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

Closed mattking-smith closed 2 years ago

mattking-smith commented 2 years ago

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 ..
  2. 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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 https://github.com/gtrll/gpmp2/issues/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 https://github.com/gtrll/gpmp2/issues/54 and the code I had posted in https://github.com/gtrll/gpmp2/pull/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 commented 2 years ago

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 https://github.com/gtrll/gpmp2/issues/54.

varunagrawal commented 2 years ago

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