Strict requirement of GCC-7 on OSX

[synapticarbors]

I've recently been working on packaging LightGBM-python for conda via conda-forge and ran into some issues since the CMake file strictly requires gcc-7 on OSX. Since gcc-7 was not available on the conda-forge build infrastructure, I patched the CMakeLists.txt to remove the restriction and then used Clang 4.0.0. I was able to build LightGBM without any issues and I could run a simple set of tests.

I'm wondering if there any issues in approaching the compilation in this way? If not and clang is a valid compiler choice, then it would seem desirable to have a more flexible way of choosing the compiler rather than forcing gcc-7.

Any feedback would be appreciated. The pull request for the conda recipe can be found here if anyone is interested in looking at it.

conda-forge/staged-recipes#3257

Also if any member of the LightGBM team is interested in being a co-maintainer of the recipe (i.e. having Github permission to modify the feedstock in order to update versions. See https://conda-forge.org/docs/recipe.html#maintainer-role), please let me know. I'm happy to maintain the recipe on my own and keep it up-to-date.

[guolinke]

The reason is the openmp support. It seems the native clang in osx cannot support openmp.

Refer to :
#3
#118

Openmp is important since the speed will be very slow without it.
If you can fix the openmp issue with clang, i think it is good to switch back.

[synapticarbors]

I seem to have it working with clang as the version that we use on conda-forge is newer than the system clang on most osx systems and supports openmp. I have not done any formal performance testing of the resulting build though.

[guolinke]

@synapticarbors I don't have a mac to check it. can you verify this ? If it can work, you can create a PR here to fix it.
@wxchan @henry0312 can you also help to check this ?

[jakirkham]

The native toolchain on macOS doesn't have OpenMP except through the ancient (and likely to be removed) gcc compiler. That compiler is 4.2 and has no modern C++ support (i.e. no C++11 or beyond). So it wouldn't fit the version constraints here and may not be usable due to its age anyways.

This all being said, users can typically get a compiler from their package manager of choice (e.g. Homebrew, MacPorts, Fink, conda, etc.) that will fix these issues. However that compiler will not likely be gcc 7 (maybe some lower version 4 or 5 would be more likely) and it may not be gcc at all if users are trying to stick close to the system toolchain which is clang-based.

In short, it would be nice to relax the version and possibly also the compiler type. Maybe using the FindOpenMP CMake module would be a more reasonable alternative that would ensure the OpenMP requirement is met without making extra requirements of the user. Thoughts?

[chivee]

@jakirkham Well explained, I think we can

1. lower the gcc version to 5 or 6.
2. Using Clang first if user's Clang version > 3.1( since which openMP has been officially supported), to meet the conda requirements

[jakirkham]

Any reason not to allow gcc 4? We use gcc 4.8.2 to build packages for Linux in conda-forge, which AFAIK worked ok for LightGBM. Other than that the proposed plan sounds good to me.

[henry0312]

can you also help to check this ?

Sorry, I've been busy today, so I'll check later 😨

[chivee]

@jakirkham 4.8.2 is enough for lightGBM. I'll lower the gcc version

[chivee]

@synapticarbors as for the conda-forge, It's reasonable to using Clang 4.0 for the build. But we need to find a way to make it flexible enough for both CI and normal users since Cmake can't switch the default compiler for the normal OsX users.

[guolinke]

@henry0312 @wxchan @synapticarbors
any solutions ?

[henry0312]

I'm sorry for late response.

@synapticarbors

If you want to use clang, you can just run CXX=clang++ CC=clang cmake ..

I don't think a patch such lile the below is needed.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8fcc960..5fd16f8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,10 +1,5 @@
 cmake_minimum_required(VERSION 2.8)

-if(APPLE)
-    SET(CMAKE_CXX_COMPILER "g++-7")
-    SET(CMAKE_C_COMPILER "gcc-7")
-endif()
-
 PROJECT(lightgbm)

 OPTION(USE_MPI "MPI based parallel learning" OFF)

Can conda-forge set CC and CXX?

[henry0312]

you can just run CXX=clang++ CC=clang cmake ..

you can also run cmake -DCMAKE_CXX_COMPILER=clang++- -DCMAKE_C_COMPILER=clang ..

[guolinke]

@henry0312
if using

if(APPLE)
    SET(CMAKE_CXX_COMPILER "g++-7")
    SET(CMAKE_C_COMPILER "gcc-7")
endif()

, -DCMAKE_CXX_COMPILER=clang++- -DCMAKE_C_COMPILER=clang won't be override ?

[henry0312]

I'm sorry, I was wrong.

we need a patch like the below:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8fcc960..5fd16f8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,10 +1,5 @@
 cmake_minimum_required(VERSION 2.8)

-if(APPLE)
-    SET(CMAKE_CXX_COMPILER "g++-7")
-    SET(CMAKE_C_COMPILER "gcc-7")
-endif()
-
 PROJECT(lightgbm)

 OPTION(USE_MPI "MPI based parallel learning" OFF)

After applied the patch, we can run CXX=clang++ CC=clang cmake .. or cmake -DCMAKE_CXX_COMPILER=clang++- -DCMAKE_C_COMPILER=clang ...

Actually, the CMakeLists.txt hadn't have either CMAKE_CXX_COMPILER nor CMAKE_C_COMPILE before f23e608.
cf. https://github.com/Microsoft/LightGBM/commits/master/CMakeLists.txt

I think it's good to remove them from CMakeLists.txt.

[guolinke]

@henry0312
Okay.
Do we need to change the installation document for the normal osx user ?
And the python/R installation is not easy to change the DCMAKE_CXX_COMPILER=clang++- -DCMAKE_C_COMPILER=clang .

[henry0312]

@guolinke
Sure.
However, OSX's system Clang doesn't have OpenMP, so we need to install other clang or gcc.

[usptact]

On MacOS High Sierra with MacPorts installed, I did the following:

1. Install clang-5.0 using MacPorts
2. Inside the /build directory, run
   cmake -DCMAKE_CXX_COMPILER=clang++-mp-5.0 -DCMAKE_C_COMPILER=clang-mp-5.0 ..
3. To build the python package, go to /python_package directory and modify the setup.py script.
   You need to modify the function compile_cpp() at the very end that checks the case for other OS (including Mac). Before the silent_call(...), add the following two lines:

cmake_cmd.append("-DCMAKE_CXX_COMPILER=clang++-mp-5.0")
cmake_cmd.append("-DCMAKE_C_COMPILER=clang-mp-5.0")

4. Run sudo python setup.py install. Enjoy