Xtra-Computing / thundersvm

ThunderSVM: A Fast SVM Library on GPUs and CPUs
Apache License 2.0
1.55k stars 215 forks source link

Drop older cmake support #198

Closed emmenlau closed 4 years ago

emmenlau commented 4 years ago

Currently the minimum required cmake is 2.x on Unix. The way this is implemented is actually causing some problems with cmake. On macOS the variables CMAKE_C_COMPILER_ID and CMAKE_CXX_COMPILER_ID are set to Clang instead of AppleClang. This may be is a sign that cmake got confused about the project, or also caused by https://github.com/conan-io/cmake-conan/issues/33.

In any case, I think nowadays it is quite preferable to write modern cmake style to make the project very portable, easy to maintain and easy to integrate with package managers. Could the support for cmake <= 3.4 be dropped on all platforms? This would also make it easier to use modern cmake constructs.

I can provide a PR to drop older cmake support, if this is interesting?

emmenlau commented 4 years ago

When I require cmake >= 3.4 then it becomes much easier to add OpenMP support for macOS. I only have to specify -DOpenMP_C_INCLUDE_DIRS=... and -DOpenMP_CXX_INCLUDE_DIRS=... to the path of omp.h. No other settings are required, OpenMP is successfully detected.

See https://github.com/Kitware/CMake/commit/409891baf77b909d414add64268d1e5530ff6086#diff-ae0a179d6813ec707caeada5cab5e63e for details.

zeyiwen commented 4 years ago

Thanks for the suggestion! ThunderSVM previously only supported cmake >= 3.x. The support for 2.x was introduced to fix this issue (#32) two years ago. I think it is reasonable to drop the support for 2.x. You are welcome to open an PR.