kzampog / cilantro

A lean C++ library for working with point cloud data
MIT License
1.01k stars 206 forks source link

Problems when the same program runs differently time. #50

Closed CarlLyt closed 4 years ago

CarlLyt commented 4 years ago

Hi , this is a awesome project, I am studying it recently. But recently there has been a problem that bothers me . I use Qt creator programming , and I created a window to display multiple point clouds together, Sometimes it runs correctly, sometimes it just quits the program after Visualizer quick popup. just like a visualizer window quick popup which no point cloud data and exit . Qt only show me : The program has unexpectedly finished. Terminal show like everything goes right : Press to close this window...

There is show window that i want

image

When not running successfully, it just like image

This is the problematic code segment image

My running environment is ubuntu16 . Can you give me some suggestions? This is very uncomfortable to debug the code

kzampog commented 4 years ago

Hi,

I cannot spot anything particularly suspicious in that code snippet. One suggestion would be to check the sizes of targetdata, srcdata, and tfset (and e.g. check that tfset has at at least size 2). Another one would be to verify the type of tfset -- it might not be using the right allocator if it's e.g. std::vector<RigidTransform<float,3>>. If that could be the case, try changing the type of tfset to RigidTransformSet<float,3> instead (if you are not using that already). If none of the above works, I would need a minimal self-contained example (and possibly data) that reproduces the problem.

Thanks for your interest in cilantro! :D

CarlLyt commented 4 years ago

Thank you for your reply, The tfset type is std::unordered_map<int, Eigen::Matrix4f> ,

cilantro::RigidTransform3f tf_est = icp.estimate().getTransform(); Eigen::Matrix4f mat = tf_est.matrix(); tfset.insert(std::make_pair(i,mat));

and then i transform use

cilantro::RigidTransform<float,3> tf; tf = tfset[1]; auto src_t = srcdata[1].transformed(tf);

The reason i do this is because i can not push_back a cilantro::RigidTransform3f variable like

std::vector<cilantro::RigidTransform3f> test; cilantro::RigidTransform3f tf_est = icp.estimate().getTransform(); test.push_back(tf_est);

It make my program crash directly I find a interesting thing that use the following method to make my program more stable, and still crash very few . The result of this is also correct

//std::vector<cilantro::RigidTransform3f> test; cilantro::SimpleCombinedMetricRigidICP3f icp(dst.points, dst.normals, src.points); std::vector<Eigen::Matrix4f> GroupP; for(...){ ... cilantro::RigidTransform3f tf_est = icp.estimate().getTransform(); Eigen::Matrix4f mat = tf_est.matrix(); //save data //test.push_back(tf_est); GroupP.push_back(mat); ... } ... cilantro::RigidTransform<float,3> tf; tf = GroupP[1]; auto src_t = srcdata[1].transformed(tf);

kzampog commented 4 years ago

Hi,

The problem is most likely this one: https://eigen.tuxfamily.org/dox/group__TopicStlContainers.html i.e. you have to specify an aligned allocator as cilantro::RigidTransform3f stores a 4x4 matrix.

If you need a vector container, you can try cilantro::RigidTransformSet3f (which extends std::vector and uses the proper allocator). It should behave similarly to std::vector<cilantro::RigidTransform3f>.

If you need an unordered_map with Eigen::Matrix4f values, try changing the type to: std::unordered_map<int, Eigen::Matrix4f, std::hash<int>, std::equal_to<int>, Eigen::aligned_allocator<std::pair<const int, Eigen::Matrix4f>>>

I hope one of these works! :) Using newer versions of Eigen with C++17 is supposed to eliminate the need to worry about allocators in such scenarios.

CarlLyt commented 4 years ago

Hi, Wow~ It works for me. Super thanks. I follow your suggestion and i changed to the following like:

//std::unordered_map<int, cilantro::RigidTransform<float,3>> tfmap; std::unordered_map<int, cilantro::RigidTransform<float,3>, std::hash, std::equal_to, Eigen::aligned_allocator<std::pair<const int, cilantro::RigidTransform<float,3> >>> tfmap; //std::unordered_map<int, cilantro::PointCloud3f> plydata; std::unordered_map<int, cilantro::PointCloud3f, std::hash, std::equal_to, Eigen::aligned_allocator<std::pair<const int, cilantro::PointCloud3f >>> plydata;

Finally I want to ask two questions:

There is my CMakeList.txt:

 cmake_minimum_required(VERSION 3.9)
project(cilantroRegistration)

# Build setup
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/")
set(CMAKE_BUILD_TYPE "Release")
set(CMAKE_CXX_STANDARD 17)
find_package(Pangolin QUIET)
if(Pangolin_FOUND)
    set(HAVE_PANGOLIN ON)
    set(DEPENDS_Pangolin "find_dependency(Pangolin)")
endif()
find_package(cilantro)
include_directories(${cilantro_INCLUDE_DIRS})
set(OTHER_INCLUDE_DIRS ${Pangolin_INCLUDE_DIRS})
set(OTHER_LIBRARIES ${Pangolin_LIBRARIES})
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/config.hpp.in" "${CMAKE_CURRENT_BINARY_DIR}/include/${PROJECT_NAME}/config.hpp")
#find_package(OpenMP)
#set(DEPENDS_OpenMP "find_dependency(OpenMP)")
# Packages
find_package(Eigen3 REQUIRED NO_MODULE)
# below is a workaround for some buggy Eigen exports which managed to get into some distros
if(NOT TARGET Eigen3::Eigen)
    message("Eigen has buggy exported package, but headers are found: ${EIGEN3_INCLUDE_DIRS}. Applying workaround.")
    add_library(Eigen INTERFACE IMPORTED GLOBAL)
    target_include_directories(Eigen INTERFACE "${EIGEN3_INCLUDE_DIRS}")
    add_library(Eigen3::Eigen ALIAS Eigen)
endif()
#file(GLOB_RECURSE THIRD_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/src/3rd_party/*.c ${CMAKE_CURRENT_SOURCE_DIR}/src/3rd_party/*.cpp)
#add_library(${PROJECT_NAME}  ${LIB_HEADERS} ${LIB_SOURCES})
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile.in ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile @ONLY)

add_executable(fullregistration main.cpp )
if (ENABLE_NATIVE_BUILD_OPTIMIZATIONS)
    target_compile_options(fullregistration PUBLIC -O3 -march=native -mtune=native)
else()
    target_compile_options(fullregistration PUBLIC -O3)
endif()
target_link_libraries (fullregistration
    ${OTHER_LIBRARIES}
    ${cilantro_LIBRARIES}
    Eigen3::Eigen
)
kzampog commented 4 years ago

Hi,

Glad it worked :)

You don't need to worry about the allocator when storing cilantro::PointCloud3f in STL containers, as it doesn't contain fixed-size vectorizable Eigen objects (just dynamic arrays), i.e. std::unordered_map<int, cilantro::PointCloud3f> should be fine.

The CMake message about Eigen while configuring is because of the Eigen package in Ubuntu 16.04 (it's fixed in 18.04), but it's nothing to worry about.

Regarding the CMakeLists.txt, the following minimal version should also work:

cmake_minimum_required(VERSION 3.9)
project(cilantroRegistration)

set(CMAKE_CXX_STANDARD 17)

find_package(cilantro REQUIRED)

option(ENABLE_NATIVE_BUILD_OPTIMIZATIONS "Enable native build optimization flags" ON)

add_executable(fullregistration main.cpp)
target_link_libraries(fullregistration ${cilantro_LIBRARIES})
if (ENABLE_NATIVE_BUILD_OPTIMIZATIONS)
    target_compile_options(fullregistration PUBLIC -O3 -march=native -mtune=native)
else()
    target_compile_options(fullregistration PUBLIC -O3)
endif()

Unfortunately, I haven't used QtCreator in a really long time. It looks like it is able to work with CMake projects, so it might simply be a matter of the IDE reading the right build/configuration dir.

Cheers!

CarlLyt commented 4 years ago

Thank you for answering me, I wish you all the best in your work and life :D

kzampog commented 4 years ago

No problem at all, happy I could help! Thank you, you too! :D

CarlLyt commented 4 years ago

hi, As far as i think,std::unordered_map<int, cilantro::PointCloud3f> will not work very well when I pack by class. I found in Eigen to use EIGEN_MAKE_ALIGNED_OPERATOR_NEW ,but it doesn't work. So,are there any better suggestions for saving point cloud data?