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

Enable qpOASES on Windows and fix Windows build #327

Merged
merged 15 commits into from
Jun 18, 2023

Conversation

johnwason
Copy link
Contributor

This PR adds qpOAESES as an external package on windows. The tests were failing because OSQP is not a great solver for trajopt. The Windows action is updated to use vcpkg master to avoid an error in the most recent release of vcpkg.

@johnwason
Copy link
Contributor Author

Two tests are failing with segfaults on Windows:

The following tests FAILED:
	  2 - NumericalIKTest.numerical_ik1 (Exit code 0xc0000409
)
	 17 - SimpleCollisionTest.spheres (Exit code 0xc0000409
)

This doesn't happen locally so I am not sure exactly what is going on. I suspect there is a subtle memory error somewhere that happens on all machines, but only in certain circumstances causes a problem. Maybe analyze with valgrind to make sure there isn't a problem in the core libraries?

@Levi-Armstrong
Copy link
Contributor

The tests were failing because OSQP is not a great solver for trajopt.

Why is this case?

@johnwason
Copy link
Contributor Author

The unit tests filter out OSQP here:

auto getAvailableSolvers = []() {

I disabled the filter and osqp does fail the test.

@Levi-Armstrong
Copy link
Contributor

Why is OSQP failing on windows but not on linux?

@johnwason
Copy link
Contributor Author

Linux has other solvers so it doesn’t use osqp. Windows only had osqp so these tests were failing because no solvers were available after osqp was filtered out.

@Levi-Armstrong
Copy link
Contributor

Levi-Armstrong commented Jun 6, 2023

The default should OSQP so it should not select another solver. I see the test was removing OSQP but because windows has no other solver it was failing.

trajopt_ext/qpoases/CMakeLists.txt Outdated Show resolved Hide resolved
trajopt_sco/src/qpoases_interface.cpp Show resolved Hide resolved
@rjoomen
Copy link
Contributor

rjoomen commented Jun 14, 2023

I tested it and this also works on Linux (with some minor changes). I use Jammy/Humble, where qpOASES is not available, so this is a great PR, thanks.

trajopt_ext/qpoases/CMakeLists.txt Outdated Show resolved Hide resolved
trajopt_ext/qpoases/colcon.pkg Outdated Show resolved Hide resolved
trajopt_ext/qpoases/CMakeLists.txt Outdated Show resolved Hide resolved
trajopt_sco/src/qpoases_interface.cpp Show resolved Hide resolved
trajopt_ext/qpoases/CMakeLists.txt Outdated Show resolved Hide resolved
@Levi-Armstrong
Copy link
Contributor

@rjoomen Thanks for testing on Linux and the review!

@Levi-Armstrong
Copy link
Contributor

Okay, so I tested this locally and the unit test is failing because ipopt also uses qpOASES and it looks like it is using the new qpOASES which must not be the same version.

@Levi-Armstrong
Copy link
Contributor

So it is not using qpOASES but it is finding an extern "C" void DPOTRF function provided by qpoases.
image

@johnwason
Copy link
Contributor Author

johnwason commented Jun 17, 2023

On windows the segfault in numerical_ik_unit and sphere collision appears to be coming from the logger. I disabled logging and it seems to fix the issue, I am testing the numerical ik now.

@Levi-Armstrong
Copy link
Contributor

Yea, the segfault for the ipopt is occurring on the Ubuntu builds.

@johnwason
Copy link
Contributor Author

Can you rework the link libraries to the trajopt_ifopt module doesn't see qpoases? I can disable qpoases on Linux again to avoid this problem for now since it isn't important for Linux.

@Levi-Armstrong
Copy link
Contributor

If you replace the trajopt_sqp/test/CMakeLists.txt with the contents below it solve the issue.

find_package(GTest REQUIRED)
find_package(trajopt REQUIRED)

if(NOT TARGET GTest::GTest)
  add_library(GTest::GTest INTERFACE IMPORTED)
  set_target_properties(GTest::GTest PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIRS}")
  if(${GTEST_LIBRARIES})
    set_target_properties(GTest::GTest PROPERTIES INTERFACE_LINK_LIBRARIES "${GTEST_LIBRARIES}")
  else()
    if(MSVC)
      set_target_properties(GTest::GTest PROPERTIES INTERFACE_LINK_LIBRARIES "gtest.lib")
    else()
      set_target_properties(GTest::GTest PROPERTIES INTERFACE_LINK_LIBRARIES "libgtest.so")
    endif()
  endif()
endif()

if(NOT TARGET GTest::Main)
  add_library(GTest::Main INTERFACE IMPORTED)
  set_target_properties(GTest::Main PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIRS}")
  if(${GTEST_MAIN_LIBRARIES})
    set_target_properties(GTest::Main PROPERTIES INTERFACE_LINK_LIBRARIES "${GTEST_MAIN_LIBRARIES}")
  else()
    if(MSVC)
      set_target_properties(GTest::Main PROPERTIES INTERFACE_LINK_LIBRARIES "gtest_main.lib")
    else()
      set_target_properties(GTest::Main PROPERTIES INTERFACE_LINK_LIBRARIES "libgtest_main.so")
    endif()
  endif()
endif()

include(GoogleTest)

macro(add_gtest test_name test_file)
  add_executable(${test_name} ${test_file})
  target_compile_options(${test_name} PRIVATE ${TRAJOPT_COMPILE_OPTIONS_PRIVATE} ${TRAJOPT_COMPILE_OPTIONS_PUBLIC})
  target_compile_definitions(${test_name} PRIVATE ${TRAJOPT_COMPILE_DEFINITIONS}
                                                  TRAJOPT_IFOPT_DIR="${CMAKE_SOURCE_DIR}")
  target_cxx_version(${test_name} PRIVATE VERSION ${TRAJOPT_CXX_VERSION})
  target_clang_tidy(${test_name} ENABLE ${TRAJOPT_ENABLE_CLANG_TIDY})
  target_link_libraries(
    ${test_name}
    ${PROJECT_NAME}
    console_bridge
    ifopt::ifopt_ipopt
    trajopt::trajopt_ifopt
    trajopt::trajopt_test_data
    GTest::GTest
    GTest::Main)
  target_include_directories(${test_name} PRIVATE "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>"
                                                  ${GTEST_INCLUDE_DIRS})
  target_include_directories(${test_name} SYSTEM PRIVATE ${PCL_INCLUDE_DIRS})
  add_gtest_discover_tests(${test_name})
  add_dependencies(${test_name} ${PROJECT_NAME})
  add_dependencies(run_tests ${test_name})
endmacro()

add_gtest(${PROJECT_NAME}_expressions_unit expressions_unit.cpp)
add_gtest(${PROJECT_NAME}_joint_position_optimization_unit joint_position_optimization_unit.cpp)
add_gtest(${PROJECT_NAME}_joint_velocity_optimization_unit joint_velocity_optimization_unit.cpp)
add_gtest(${PROJECT_NAME}_joint_acceleration_optimization_unit joint_acceleration_optimization_unit.cpp)
add_gtest(${PROJECT_NAME}_joint_jerk_optimization_unit joint_jerk_optimization_unit.cpp)
add_gtest(${PROJECT_NAME}_cast_cost_attached_unit cast_cost_attached_unit.cpp)
add_gtest(${PROJECT_NAME}_cast_cost_octomap_unit cast_cost_octomap_unit.cpp)
add_gtest(${PROJECT_NAME}_cast_cost_unit cast_cost_unit.cpp)
add_gtest(${PROJECT_NAME}_cast_cost_world_unit cast_cost_world_unit.cpp)
add_gtest(${PROJECT_NAME}_numerical_ik_unit numerical_ik_unit.cpp)
add_gtest(${PROJECT_NAME}_planning_unit planning_unit.cpp)
add_gtest(${PROJECT_NAME}_simple_collision_unit simple_collision_unit.cpp)
add_gtest(${PROJECT_NAME}_cart_position_optimization_unit cart_position_optimization_unit.cpp)

# This is for comparison and should be moved to trajopt
add_executable(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit cart_position_optimization_trajopt_sco_unit.cpp)
target_compile_options(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit PRIVATE ${TRAJOPT_COMPILE_OPTIONS_PRIVATE} ${TRAJOPT_COMPILE_OPTIONS_PUBLIC})
target_compile_definitions(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit PRIVATE ${TRAJOPT_COMPILE_DEFINITIONS}
                                                TRAJOPT_IFOPT_DIR="${CMAKE_SOURCE_DIR}")
target_cxx_version(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit PRIVATE VERSION ${TRAJOPT_CXX_VERSION})
target_clang_tidy(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit ENABLE ${TRAJOPT_ENABLE_CLANG_TIDY})
target_link_libraries(
  ${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit
  ${PROJECT_NAME}
  console_bridge
  ifopt::ifopt_ipopt
  trajopt::trajopt_ifopt
  trajopt::trajopt_test_data
  trajopt::trajopt
  GTest::GTest
  GTest::Main)
target_include_directories(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit PRIVATE "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>"
                                                ${GTEST_INCLUDE_DIRS})
target_include_directories(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit SYSTEM PRIVATE ${PCL_INCLUDE_DIRS})
add_gtest_discover_tests(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit)
add_dependencies(${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit ${PROJECT_NAME})
add_dependencies(run_tests ${PROJECT_NAME}_cart_position_optimization_trajopt_sco_unit)

@Levi-Armstrong
Copy link
Contributor

The issue was all of the trajopt_sqp unit tests where linking against trajopt when it was not needed causing the issue.

@johnwason
Copy link
Contributor Author

Does tesseract planning still work or does it have similar issues?

@Levi-Armstrong
Copy link
Contributor

Nope, just ran a few trajopt_ifopt examples without any issues though we do not use ipopt for motion planning just testing in trajopt_sqp.

@johnwason
Copy link
Contributor Author

Issue #331 created for logger problem.

@johnwason
Copy link
Contributor Author

All checks are passing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants