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

Initial attempt at building OpenCL wheel for Windows #3051

Closed
wants to merge 13 commits into from

Conversation

itamarst
Copy link
Contributor

@itamarst itamarst commented May 5, 2020

Still untested on actual CI, but this should create a wheel that uses OpenCL, for #2263 based on local testing. I don't quite understand how the wheels get published from this build, but in any case that requires PyPI credentials so you'll have to finish that up.

Tasks:

  • Build wheel that uses OpenCL.
  • Make tests pass
  • Switch to using OpenCL headers from Khronos, instead of the AMD SDK
  • Maybe make bdist_wheel support GPU install options, rather than using env variables
  • Add new PyPI project (lightgbm-opencl maybe?)
  • Make sure release process uploads the new wheel to the new PyPI project

Someone with more access than I have will likely need to do last two items.

@msftclas
Copy link

msftclas commented May 5, 2020

CLA assistant check
All CLA requirements met.

@itamarst itamarst closed this May 5, 2020
@itamarst itamarst reopened this May 5, 2020
@itamarst itamarst changed the title Initial attempt at building OpenCL wheel. Initial attempt at building OpenCL wheel for Windows May 5, 2020
@itamarst
Copy link
Contributor Author

itamarst commented May 5, 2020

Looks like this iteration won't work at the very least because the --gpu etc. arguments won't work with bdist_wheel.

@itamarst
Copy link
Contributor Author

I have improved setup.py so build options can be set via environment variables, and then the GPU build uses that.

@itamarst itamarst marked this pull request as ready for review May 12, 2020 15:37
@itamarst
Copy link
Contributor Author

I am not sure why the DLL isn't being found in tests, when I build wheel locally it has the DLL, and logs for bdist_opencl show the DLL being added. Any ideas?

@StrikerRUS
Copy link
Collaborator

I have improved setup.py so build options can be set via environment variables, and then the GPU build uses that.

I believe that environment variables is not the best way to control installation options. Maybe there are some other methods?.. I remember we already had some proposals for bdist_wheel flags in #1863.

Linking #2263 here as the original issue.

For the general idea of having separate OpenCL wheels for Windows and possible compatibility issues ping @huanzhang12 .

Also I wonder how demanded this wheel will be, given total NVIDIA (read CUDA) domination.

@StrikerRUS StrikerRUS requested a review from huanzhang12 May 12, 2020 15:59
@itamarst
Copy link
Contributor Author

As I understand it there is no specifically CUDA lightgbm at the moment, though? On Nvidia you just use OpenCL installed by CUDA?

@itamarst
Copy link
Contributor Author

I can certainly change it from env variables to bdist_wheel options if that is preferred, although the benefit of env variables is that you can also use this with pip wheel which I don't think you can do with extra args to bdist_wheel.

@itamarst
Copy link
Contributor Author

I wonder if tests are failing because there's no OpenCL runtime installed...

@itamarst
Copy link
Contributor Author

Looks like tests are likely passing now for the OpenCL build, (the docs test failure are unrelated since I haven't touched docs). So remaining questions are:

  1. Will you accept this at all?
  2. If so, are environment variables OK or do you want a larger refactor to how setup.py works?
  3. Will you take on the work of publishing separate PyPI package?

@StrikerRUS
Copy link
Collaborator

@itamarst

the docs test failure are unrelated since I haven't touched docs

Sorry for the inconvenience! I just re-run it.

If so, are environment variables OK or do you want a larger refactor to how setup.py works?

Please give me some time to read about other possible ways.

I believe we should set device parameter for this wheel to gpu. At least it should be done for the CI test purposes. But for the final wheel it also makes sense, I think.
Refer to GPU *nix tests:

LightGBM/.ci/test.sh

Lines 126 to 127 in 42ebb4e

sed -i'.bak' 's/std::string device_type = "cpu";/std::string device_type = "gpu";/' $BUILD_DIRECTORY/include/LightGBM/config.h
grep -q 'std::string device_type = "gpu"' $BUILD_DIRECTORY/include/LightGBM/config.h || exit -1 # make sure that changes were really done

@itamarst
Copy link
Contributor Author

Thanks, good point re device. I will be working on this again next week sometime.

@itamarst
Copy link
Contributor Author

@StrikerRUS I've made the OpenCL wheel have GPU device by default. Have you decided how you feel about env variables? My feeling is that a larger setup.py refactor would be useful, but is perhaps orthogonal to this issue.

@StrikerRUS
Copy link
Collaborator

@itamarst

I've made the OpenCL wheel have GPU device by default.

Thanks! But it seems that now all tests are failing.

Have you decided how you feel about env variables

Sorry, haven't had a chance to look into this yet.

@StrikerRUS
Copy link
Collaborator

Have you decided how you feel about env variables

Sorry, haven't had a chance to look into this yet.

Seems that we can make bdist_wheel command work with our arguments by inheriting wheel command class.

diff --git a/python-package/setup.py b/python-package/setup.py
index 73f123ba..68834dde 100644
--- a/python-package/setup.py
+++ b/python-package/setup.py
@@ -16,6 +16,60 @@ from setuptools import find_packages, setup
 from setuptools.command.install import install
 from setuptools.command.install_lib import install_lib
 from setuptools.command.sdist import sdist
+from wheel.bdist_wheel import bdist_wheel
+
+
+class CustomBdistWheel(bdist_wheel):
+
+    user_options = bdist_wheel.user_options + [
+        ('mingw', 'm', 'Compile with MinGW'),
+        ('gpu', 'g', 'Compile GPU version'),
+        ('mpi', None, 'Compile MPI version'),
+        ('nomp', None, 'Compile version without OpenMP support'),
+        ('hdfs', 'h', 'Compile HDFS version'),
+        ('bit32', None, 'Compile 32-bit version'),
+        ('precompile', 'p', 'Use precompiled library'),
+        ('boost-root=', None, 'Boost preferred installation prefix'),
+        ('boost-dir=', None, 'Directory with Boost package configuration file'),
+        ('boost-include-dir=', None, 'Directory containing Boost headers'),
+        ('boost-librarydir=', None, 'Preferred Boost library directory'),
+        ('opencl-include-dir=', None, 'OpenCL include directory'),
+        ('opencl-library=', None, 'Path to OpenCL library')
+    ]
+
+    def initialize_options(self):
+        bdist_wheel.initialize_options(self)
+        self.mingw = 0
+        self.gpu = 0
+        self.boost_root = None
+        self.boost_dir = None
+        self.boost_include_dir = None
+        self.boost_librarydir = None
+        self.opencl_include_dir = None
+        self.opencl_library = None
+        self.mpi = 0
+        self.hdfs = 0
+        self.precompile = 0
+        self.nomp = 0
+        self.bit32 = 0
+
+    def run(self):
+        install = self.distribution.get_command_obj('install')
+        install.user_options = self.user_options
+        install.mingw = self.mingw
+        install.gpu = self.gpu
+        install.boost_root = self.boost_root
+        install.boost_dir = self.boost_dir
+        install.boost_include_dir = self.boost_include_dir
+        install.boost_librarydir = self.boost_librarydir
+        install.opencl_include_dir = self.opencl_include_dir
+        install.opencl_library = self.opencl_library
+        install.mpi = self.mpi
+        install.hdfs = self.hdfs
+        install.precompile = self.precompile
+        install.nomp = self.nomp
+        install.bit32 = self.bit32
+        bdist_wheel.run(self)
 
 
 def find_lib():
@@ -98,6 +152,7 @@ def compile_cpp(use_mingw=False, use_gpu=False, use_mpi=False,
     os.chdir(os.path.join(CURRENT_DIR, "build_cpp"))
 
     logger.info("Starting to compile the library.")
+    print(boost_dir)
 
     cmake_cmd = ["cmake", "../compile/"]
     if use_gpu:
@@ -285,6 +340,7 @@ if __name__ == "__main__":
               'install': CustomInstall,
               'install_lib': CustomInstallLib,
               'sdist': CustomSdist,
+              'bdist_wheel': CustomBdistWheel,
           },
           packages=find_packages(),
           include_package_data=True,

Refer to https://github.com/pypa/wheel/blob/master/src/wheel/bdist_wheel.py.

Note that code above is only POC and there is a lot of room for better code, e.g. removing duplicated list of options and setting of properties.

Also, we can make these changes optional to not force users install wheel in case they are installing LightGBM via ordinary setup.py install, like here:
https://github.com/unbit/uwsgi/blob/085015b281ff3d0e81fa52cc9c2fc6c4e3917393/setup.py#L95-L99

Below are some checks:

python setup.py install
running install
INFO:LightGBM:Starting to compile the library.
None
INFO:LightGBM:Starting to compile with MSBuild from existing solution file.
...

python setup.py install --boost-dir=ALERT
running install
INFO:LightGBM:Starting to compile the library.
ALERT
INFO:LightGBM:Starting to compile with MSBuild from existing solution file.
...

python setup.py bdist_wheel --plat-name=win-amd64 --universal
running bdist_wheel
running build
running build_py
running egg_info
writing lightgbm.egg-info\PKG-INFO
writing dependency_links to lightgbm.egg-info\dependency_links.txt
writing requirements to lightgbm.egg-info\requires.txt
writing top-level names to lightgbm.egg-info\top_level.txt
reading manifest file 'lightgbm.egg-info\SOURCES.txt'
reading manifest template 'MANIFEST.in'
no previously-included directories found matching 'build'
warning: no files found matching '*.txt'
warning: no files found matching '*.so' under directory 'lightgbm'
warning: no files found matching '*.so' under directory 'compile'
warning: no files found matching '*.dll' under directory 'compile\Release'
warning: no files found matching '*' under directory 'compile\compute'
warning: no files found matching '*.dll' under directory 'compile\windows\x64\DLL'
warning: no previously-included files matching '*.py[co]' found anywhere in distribution
writing manifest file 'lightgbm.egg-info\SOURCES.txt'
installing to build\bdist.win-amd64\wheel
running install
INFO:LightGBM:Starting to compile the library.
None
INFO:LightGBM:Starting to compile with MSBuild from existing solution file.
...

python setup.py bdist_wheel --plat-name=win-amd64 --universal --boost-dir=ALERT
running bdist_wheel
running build
running build_py
running egg_info
writing lightgbm.egg-info\PKG-INFO
writing dependency_links to lightgbm.egg-info\dependency_links.txt
writing requirements to lightgbm.egg-info\requires.txt
writing top-level names to lightgbm.egg-info\top_level.txt
reading manifest file 'lightgbm.egg-info\SOURCES.txt'
reading manifest template 'MANIFEST.in'
no previously-included directories found matching 'build'
warning: no files found matching '*.txt'
warning: no files found matching '*.so' under directory 'lightgbm'
warning: no files found matching '*.so' under directory 'compile'
warning: no files found matching '*.dll' under directory 'compile\Release'
warning: no files found matching '*' under directory 'compile\compute'
warning: no files found matching '*.dll' under directory 'compile\windows\x64\DLL'
warning: no previously-included files matching '*.py[co]' found anywhere in distribution
writing manifest file 'lightgbm.egg-info\SOURCES.txt'
installing to build\bdist.win-amd64\wheel
running install
INFO:LightGBM:Starting to compile the library.
ALERT
INFO:LightGBM:Starting to compile with MSBuild from existing solution file.
...

@itamarst
Copy link
Contributor Author

itamarst commented Jun 9, 2020

Replaced by #3143.

@itamarst itamarst closed this Jun 9, 2020
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants