PointCloudLibrary / pcl

Point Cloud Library (PCL)
https://pointclouds.org/
Other
9.98k stars 4.62k forks source link

[common/pcl_macros.h] Overloading conflict with "void aligned_free(void *)" also present in Eigen3 lib. #4734

Closed mnobrecastro closed 3 years ago

mnobrecastro commented 3 years ago

Describe the bug

Good evening everyone! I am not sure this "ambiguous call" issue has been raised before, but while I was attempting to use the GeneralizedEigenSolver(const MatrixType&, const MatrixType&, bool) from the Eigen lib (which belongs to the <Eigen/Eigenvalues> module), I came across this overloading conflict of the function 'void aligned_free(void *)' which seems to be declared in both PCL and Eigen.

pcl/common/include/pcl/pcl_macros.h (lines 401-415)

inline void
aligned_free (void* ptr)
{
#if   defined (MALLOC_ALIGNED) || defined (HAVE_POSIX_MEMALIGN)
  std::free (ptr);
#elif defined (HAVE_MM_MALLOC)
  _mm_free (ptr);
#elif defined (_MSC_VER)
  _aligned_free (ptr);
#elif defined (ANDROID)
  free (ptr);
#else
  #error aligned_free not supported on your platform
#endif
}

Eigen\src\Core\util\Memory.h (lines 173-181/328-335)

/** \internal Frees memory allocated with aligned_malloc. */
EIGEN_DEVICE_FUNC inline void aligned_free(void *ptr)
{
  #if (EIGEN_DEFAULT_ALIGN_BYTES==0) || EIGEN_MALLOC_ALREADY_ALIGNED
    std::free(ptr);
  #else
    handmade_aligned_free(ptr);
  #endif
}

[...]

/** \internal Deletes objects constructed with aligned_new
  * The \a size parameters tells on how many objects to call the destructor of T.
  */
template<typename T> EIGEN_DEVICE_FUNC inline void aligned_delete(T *ptr, std::size_t size)
{
  destruct_elements_of_array<T>(ptr, size);
  aligned_free(ptr);
}

Context

Due to the nature of the problem I am working on, I expect to use some depth data to solve a generalized eigenvalue problem of the type Av=λBv, where A and B are two matrices and where the λ and v correspond, respectively to the generalized eigenvalues and eigenvectors of that matrix pair. Given the singular nature of the B matrix that I must use, other alternatives such as the GeneralizedSelfAdjointEigenSolver cannot help me reach the solution. It must be done by the GeneralizedEigenSolver. Furthermore, I have never previously had any issues with the #include <Eigen/Eigenvalues> module before, so this bug is directly triggered by dependency concerning the GeneralizedEigenSolver which has to do with the use/conversion of the std::complex numbers in its source, even though the method only works with fully Real matrices A and B.

Expected behavior

Being able to reproduce the example shown in the GeneralizedEigenSolver's page:

 Eigen::GeneralizedEigenSolver<Eigen::MatrixXf> ges;
 Eigen::MatrixXf A = Eigen::MatrixXf::Random(4,4);
 Eigen::MatrixXf B = Eigen::MatrixXf::Random(4,4);
 ges.compute(A, B);

Please note that the results below may change given that the matrices A and B are randomly generated:

The (complex) numerators of the generalized eigenvalues are: (-0.512648,-0.935419) (-0.512648,0.935419) (-0.634525,0) (0.443328,0)
The (real) denominators of the generalized eigenvalues are: 0.656403 0.656403 -1.13469 0.271399
The (complex) generalized eigenvalues are (alphas./beta): (-0.780997,-1.42507) (-0.780997,1.42507) (0.559207,-0) (1.63349,0)

Current Behavior

The solution does not build due to the above mention overloading of the two 'void aligned_free(void *)' functions:

1>------ Rebuild All started: Project: ZERO_CHECK, Configuration: Release x64 ------
1>Checking Build System
2>------ Rebuild All started: Project: pcl-ges, Configuration: Release x64 ------
2>Building Custom Rule C:/pcl-ges/CMakeLists.txt
2>pcl-ges.cpp
2>C:\Program Files (x86)\Eigen3\include\eigen3\Eigen\src\Core\util/Memory.h(334,1): error C2668: 'aligned_free': ambiguous call to overloaded function
2>C:\Program Files (x86)\PCL\include\pcl-1.11\pcl/pcl_macros.h(402,1): message : could be 'void aligned_free(void *)' [found using argument-dependent lookup]
2>C:\Program Files (x86)\Eigen3\include\eigen3\Eigen\src\Core\util/Memory.h(174,31): message : or       'void Eigen::internal::aligned_free(void *)'
2>C:\Program Files (x86)\Eigen3\include\eigen3\Eigen\src\Core\util/Memory.h(334,1): message : while trying to match the argument list '(T *)'
2>        with
2>        [
2>            T=std::complex<float>
2>        ]

[...]

2>C:\Program Files (x86)\Eigen3\include\eigen3\Eigen\src\Core/PlainObjectBase.h(100): message : see reference to class template instantiation 'Eigen::MatrixBase<Derived>' being compiled
2>        with
2>        [
2>            Derived=Eigen::Matrix<std::complex<float>,2,2,0,2,2>
2>        ]
2>C:\Program Files (x86)\Eigen3\include\eigen3\Eigen\src\Core/Matrix.h(180): message : see reference to class template instantiation 'Eigen::PlainObjectBase<Eigen::Matrix<std::complex<float>,2,2,0,2,2>>' being compiled
2>C:\Program Files (x86)\Eigen3\include\eigen3\Eigen\src\Eigenvalues/GeneralizedEigenSolver.h(394): message : see reference to class template instantiation 'Eigen::Matrix<std::complex<float>,2,2,0,2,2>' being compiled
2>C:\Program Files (x86)\Eigen3\include\eigen3\Eigen\src\Eigenvalues/GeneralizedEigenSolver.h(288): message : while compiling class template member function 'Eigen::GeneralizedEigenSolver<Eigen::MatrixXf> &Eigen::GeneralizedEigenSolver<Eigen::MatrixXf>::compute(const Eigen::Matrix<float,-1,-1,0,-1,-1> &,const Eigen::Matrix<float,-1,-1,0,-1,-1> &,bool)'
2>C:\pcl-ges\pcl-ges.cpp(202): message : see reference to function template instantiation 'Eigen::GeneralizedEigenSolver<Eigen::MatrixXf> &Eigen::GeneralizedEigenSolver<Eigen::MatrixXf>::compute(const Eigen::Matrix<float,-1,-1,0,-1,-1> &,const Eigen::Matrix<float,-1,-1,0,-1,-1> &,bool)' being compiled
2>C:\pcl-ges\pcl-ges.cpp(201): message : see reference to class template instantiation 'Eigen::GeneralizedEigenSolver<Eigen::MatrixXf>' being compiled
2>Done building project "pcl-ges.vcxproj" -- FAILED.

To Reproduce

CMakeLists.txt

cmake_minimum_required(VERSION 3.0 FATAL_ERROR)

project(pcl-ges)

find_package(PCL 1.11 REQUIRED)
include_directories(${PCL_INCLUDE_DIRS})
link_directories(${PCL_LIBRARY_DIRS})
add_definitions(${PCL_DEFINITIONS})

add_executable (pcl-ges pcl-ges.cpp)
target_link_libraries (pcl-ges ${PCL_LIBRARIES})

pcl-ges.cpp

#include <iostream>
#include <pcl/point_types.h> // including something random from the PCL lib, will always trigger this.
#include <Eigen/Eigenvalues>

int main()
{
    Eigen::GeneralizedEigenSolver<Eigen::MatrixXf> ges;
    Eigen::MatrixXf A = Eigen::MatrixXf::Random(4,4);
    Eigen::MatrixXf B = Eigen::MatrixXf::Random(4,4);
    ges.compute(A, B);
    std::cout << "The (complex) numerators of the generalzied eigenvalues are: " << ges.alphas().transpose() << std::endl;
    std::cout << "The (real) denominatore of the generalzied eigenvalues are: " << ges.betas().transpose() << std::endl;
    std::cout << "The (complex) generalzied eigenvalues are (alphas./beta): " << ges.eigenvalues().transpose() << std::endl;

    return 0;
}

Environment:

Possible Solution

I adopted this temporary fix on the Eigen side by prepending the namespace "Eigen::internal::" to the aligned_free(ptr) on line 334 as suggested by the compiler, but it perhaps would be more practical to have a similar namespace solution for the aligned_free(ptr) on the PCL side. I leave this temp suggestion, but please let me know whether is there another fix just on the PCL side. Thank you very much for your time!

Eigen\src\Core\util\Memory.h (lines 328-335)

/** \internal Deletes objects constructed with aligned_new
  * The \a size parameters tells on how many objects to call the destructor of T.
  */
template<typename T> EIGEN_DEVICE_FUNC inline void aligned_delete(T *ptr, std::size_t size)
{
  destruct_elements_of_array<T>(ptr, size);
  Eigen::internal::aligned_free(ptr);  //<---- temporary fix by including the respective namespace.
}
mvieth commented 3 years ago

Thank you for this fantastic and very informative bug report! It seems to me like putting aligned_free (and aligned_malloc) in the pcl namespace would be the best solution. It would be great if you could try that out and see if it fixes the problem for you.

kunaltyagi commented 3 years ago

aligned_{malloc,free} are used in PCL for just recognition (linemod). With some refactoring, we might be able to eliminate that. For now, a temporary fix would be to move the free functions to pcl namespace.

I believe, this is an oversight (of keeping the functions outside the namespace, perhaps since the header is for macros only) on our part.

mnobrecastro commented 3 years ago

Thank you for the quick help and for the kind words. The namespace pcl fix on the PCL side works indeed for both aligned_{malloc,free}:

pcl/common/include/pcl/pcl_macros.h (lines 379-415)

namespace pcl {

inline void*
aligned_malloc(std::size_t size) {...}

inline void
aligned_free(void* ptr) {...}

} // namespace pcl

As a quick note, before applying the pcl fix, I managed somehow to trigger another aligned_free overloading error while using the GeneralizedEigenSolver in my main project. This time in a different line from that I had temporarily fixed on the Eigen side. After looking at the Eigen\src\Core\util\Memory.h script, I noticed that sometimes they prefix the aligned_{malloc,free} calls with their "Eigen::internal::" namespace, but sometimes they don't. More problems may arise with other libs depending on Eigen.

Thank you for your time, once more!

kunaltyagi commented 3 years ago

@mnobrecastro would you be interested in getting the PR started?

mnobrecastro commented 3 years ago

Dear @mvieth and @kunaltyagi, I have done the PR as agreed. Thank you guys!