From 6b5e80d500370de231fa7a3aecd536f2a2316e9b Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 27 Apr 2022 07:54:55 -0700 Subject: [PATCH 01/37] Add _skbuild to gitignore. --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index 972d491b86..1c37c197dd 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,10 @@ log .DS_Store dask-worker-space/ *.egg-info/ + +## scikit-build +_skbuild + ## eclipse .project .cproject From f38cfbd60dc4119f4404ed72e21cb6ec4d0997f5 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 27 Apr 2022 12:51:35 -0700 Subject: [PATCH 02/37] Initial version with skbuild. --- python/pylibraft/CMakeLists.txt | 66 ++++++ python/pylibraft/cmake/CMakeLists.txt | 16 ++ .../pylibraft/cmake/raft_python_helpers.cmake | 48 +++++ python/pylibraft/cmake/skbuild_patches.cmake | 48 +++++ .../pylibraft/distance/CMakeLists.txt | 21 ++ python/pylibraft/setup.py | 188 +++--------------- 6 files changed, 222 insertions(+), 165 deletions(-) create mode 100644 python/pylibraft/CMakeLists.txt create mode 100644 python/pylibraft/cmake/CMakeLists.txt create mode 100644 python/pylibraft/cmake/raft_python_helpers.cmake create mode 100644 python/pylibraft/cmake/skbuild_patches.cmake create mode 100644 python/pylibraft/pylibraft/distance/CMakeLists.txt diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt new file mode 100644 index 0000000000..48c0164c90 --- /dev/null +++ b/python/pylibraft/CMakeLists.txt @@ -0,0 +1,66 @@ +# ============================================================================= +# Copyright (c) 2022, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. +# ============================================================================= + +cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) + +set(raft_version 22.06.00) + +set(CYTHON_FLAGS + "--directive binding=True,embedsignature=True,always_allow_keywords=True" + CACHE STRING "The directives for Cython compilation.") + +project( + raft-python + VERSION ${raft_version} + LANGUAGES # TODO: Building Python extension modules via the python_extension_module requires the C + # language to be enabled here. The test project that is built in scikit-build to verify + # various linking options for the python library is hardcoded to build with C, so until + # that is fixed we need to keep C. + C + CXX + # TODO: The C++ RMM CMake configuration targets cuda_std_17 features, which prior to + # CMake 3.22 will also pull in the corresponding required languages even if this project + # does not actually require those languages. As a result, we need to include CUDA here. + # We can remove CUDA once we upgrade the minimum required CMake version to 3.22. + CUDA) + +option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulting to local files" + OFF) + +find_package(PythonExtensions REQUIRED) +find_package(Cython REQUIRED) + +# Ignore unused variable warning. +set(ignored_variable "${SKBUILD}") + +# If the user requested it we attempt to find RAFT. +if(FIND_RAFT_CPP) + find_package(RAFT ${raft_version}) +else() + set(RAFT_FOUND OFF) +endif() + +if(NOT RAFT_FOUND) + set(BUILD_TESTS OFF) + set(BUILD_BENCHMARKS OFF) + set(BUILD_TESTS OFF) + set(RAFT_COMPILE_LIBRARIES ON) + set(RAFT_COMPILE_DIST_LIBRARY ON) + #RAFT_COMPILE_NN_LIBRARY OFF + #RAFT_ENABLE_NN_DEPENDENCIES ON + add_subdirectory(../../cpp raft-cpp) +endif() + +add_subdirectory(cmake) +add_subdirectory(pylibraft/distance) diff --git a/python/pylibraft/cmake/CMakeLists.txt b/python/pylibraft/cmake/CMakeLists.txt new file mode 100644 index 0000000000..184099c83c --- /dev/null +++ b/python/pylibraft/cmake/CMakeLists.txt @@ -0,0 +1,16 @@ +# ============================================================================= +# Copyright (c) 2022, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. +# ============================================================================= + +include(raft_python_helpers.cmake) +include(skbuild_patches.cmake) diff --git a/python/pylibraft/cmake/raft_python_helpers.cmake b/python/pylibraft/cmake/raft_python_helpers.cmake new file mode 100644 index 0000000000..c72afb252e --- /dev/null +++ b/python/pylibraft/cmake/raft_python_helpers.cmake @@ -0,0 +1,48 @@ +# ============================================================================= +# Copyright (c) 2022, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. +# ============================================================================= + +#[=======================================================================[.rst: +add_cython_modules +------------------ + +Generate C(++) from Cython and create Python modules. + +.. code-block:: cmake + + add_cython_modules() + +Creates a Cython target for a module, then adds a corresponding Python +extension module. + +``ModuleName`` + The list of modules to build. + +#]=======================================================================] +function(add_cython_modules cython_modules) + foreach(cython_module ${cython_modules}) + add_cython_target(${cython_module} CXX PY3) + add_library(${cython_module} MODULE ${cython_module}) + python_extension_module(${cython_module}) + + # To avoid libraries being prefixed with "lib". + set_target_properties(${cython_module} PROPERTIES PREFIX "") + target_link_libraries(${cython_module} raft::raft) + + # Compute the install directory relative to the source and rely on installs being relative to + # the CMAKE_PREFIX_PATH for e.g. editable installs. + cmake_path(RELATIVE_PATH CMAKE_CURRENT_SOURCE_DIR BASE_DIRECTORY ${raft-python_SOURCE_DIR} + OUTPUT_VARIABLE install_dst) + install(TARGETS ${cython_module} DESTINATION ${install_dst}) + endforeach(cython_module ${cython_sources}) +endfunction() diff --git a/python/pylibraft/cmake/skbuild_patches.cmake b/python/pylibraft/cmake/skbuild_patches.cmake new file mode 100644 index 0000000000..9eb0cc8e25 --- /dev/null +++ b/python/pylibraft/cmake/skbuild_patches.cmake @@ -0,0 +1,48 @@ +# ============================================================================= +# Copyright (c) 2022, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. +# ============================================================================= + +#[=======================================================================[.rst: +_set_python_extension_symbol_visibility +--------------------------------------- + +The original version of this function in scikit-build runs a linker script to +modify the visibility of symbols. This version is a patch to avoid overwriting +the visibility of symbols because RAFT specifically overrides some symbol +visibility in order to share certain functional-local static variables. + +#]=======================================================================] +# TODO: Should we guard this based on a scikit-build version? Override this function to avoid +# scikit-build clobbering symbol visibility. +function(_set_python_extension_symbol_visibility _target) + if(PYTHON_VERSION_MAJOR VERSION_GREATER 2) + set(_modinit_prefix "PyInit_") + else() + set(_modinit_prefix "init") + endif() + message("_modinit_prefix:${_modinit_prefix}") + if("${CMAKE_C_COMPILER_ID}" STREQUAL "MSVC") + set_target_properties(${_target} PROPERTIES LINK_FLAGS "/EXPORT:${_modinit_prefix}${_target}") + elseif("${CMAKE_C_COMPILER_ID}" STREQUAL "GNU" AND NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin") + set(_script_path ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/${_target}-version-script.map) + file( + WRITE ${_script_path} + # Note: The change is to this script, which does not indiscriminately mark all non PyInit + # symbols as local. + "{global: ${_modinit_prefix}${_target}; };") + set_property( + TARGET ${_target} + APPEND_STRING + PROPERTY LINK_FLAGS " -Wl,--version-script=\"${_script_path}\"") + endif() +endfunction() diff --git a/python/pylibraft/pylibraft/distance/CMakeLists.txt b/python/pylibraft/pylibraft/distance/CMakeLists.txt new file mode 100644 index 0000000000..a44b2c1205 --- /dev/null +++ b/python/pylibraft/pylibraft/distance/CMakeLists.txt @@ -0,0 +1,21 @@ +# ============================================================================= +# Copyright (c) 2022, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except +# in compliance with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. +# ============================================================================= + +# Set the list of Cython files to build +set(cython_modules pairwise_distance) + +# Build all of the Cython targets +add_cython_modules("${cython_modules}") + +target_link_libraries(pairwise_distance raft::distance) diff --git a/python/pylibraft/setup.py b/python/pylibraft/setup.py index 290202403d..495cbd7433 100644 --- a/python/pylibraft/setup.py +++ b/python/pylibraft/setup.py @@ -14,166 +14,16 @@ # limitations under the License. # -import numpy -import os -import shutil -import sys -import sysconfig -# Must import in this order: -# setuptools -> Cython.Distutils.build_ext -> setuptools.command.build_ext -# Otherwise, setuptools.command.build_ext ends up inheriting from -# Cython.Distutils.old_build_ext which we do not want -import setuptools - -try: - from Cython.Distutils.build_ext import new_build_ext as _build_ext -except ImportError: - from setuptools.command.build_ext import build_ext as _build_ext - -from distutils.sysconfig import get_python_lib - -import setuptools.command.build_ext -from setuptools import find_packages, setup -from setuptools.extension import Extension - -from setuputils import clean_folder -from setuputils import get_environment_option -from setuputils import get_cli_option - -from pathlib import Path +from setuptools import find_packages +from skbuild import setup import versioneer -############################################################################## -# - Dependencies include and lib folder setup -------------------------------- - -install_requires = [ - 'cython' -] - -cuda_home = get_environment_option("CUDA_HOME") - -clean_artifacts = get_cli_option('clean') -single_gpu_build = get_cli_option('--singlegpu') - - -if not cuda_home: - cuda_home = ( - os.popen('echo "$(dirname $(dirname $(which nvcc)))"').read().strip() - ) - print("-- Using nvcc to detect CUDA, found at " + str(cuda_home)) -cuda_include_dir = os.path.join(cuda_home, "include") -cuda_lib_dir = os.path.join(cuda_home, "lib64") - -############################################################################## -# - Clean target ------------------------------------------------------------- - -if clean_artifacts: - print("-- Cleaning all Python and Cython build artifacts...") - - try: - setup_file_path = str(Path(__file__).parent.absolute()) - shutil.rmtree(setup_file_path + '/.pytest_cache', ignore_errors=True) - shutil.rmtree(setup_file_path + '/pylibraft.egg-info', - ignore_errors=True) - shutil.rmtree(setup_file_path + '/__pycache__', ignore_errors=True) - - clean_folder(setup_file_path + '/pylibraft') - shutil.rmtree(setup_file_path + '/build') - - except IOError: - pass - - # need to terminate script so cythonizing doesn't get triggered after - # cleanup unintendedly - sys.argv.remove("clean") - - if "--all" in sys.argv: - sys.argv.remove("--all") - - if len(sys.argv) == 1: - sys.exit(0) - - -############################################################################## -# - Cython extensions build and parameters ----------------------------------- - -libs = ['raft_distance', 'cudart', "cusolver", "cusparse", "cublas"] - -include_dirs = [cuda_include_dir, - numpy.get_include(), - "../../cpp/include/", - os.path.dirname(sysconfig.get_path("include"))] - -extensions = [ - Extension("*", - sources=["pylibraft/**/*.pyx"], - include_dirs=include_dirs, - library_dirs=[get_python_lib()], - runtime_library_dirs=[cuda_lib_dir, - get_python_lib(), - os.path.join(os.sys.prefix, "lib")], - libraries=libs, - language='c++', - extra_compile_args=['-std=c++17']) -] - - -class build_ext_no_debug(_build_ext): - - def build_extensions(self): - def remove_flags(compiler, *flags): - for flag in flags: - try: - compiler.compiler_so = list( - filter((flag).__ne__, compiler.compiler_so) - ) - except Exception: - pass - - # Full optimization - self.compiler.compiler_so.append("-O3") - - # Ignore deprecation declaration warnings - self.compiler.compiler_so.append("-Wno-deprecated-declarations") - - # No debug symbols, full optimization, no '-Wstrict-prototypes' warning - remove_flags( - self.compiler, "-g", "-G", "-O1", "-O2", "-Wstrict-prototypes" - ) - super().build_extensions() - - def finalize_options(self): - if self.distribution.ext_modules: - # Delay import this to allow for Cython-less installs - from Cython.Build.Dependencies import cythonize - - nthreads = getattr(self, "parallel", None) # -j option in Py3.5+ - nthreads = int(nthreads) if nthreads else None - self.distribution.ext_modules = cythonize( - self.distribution.ext_modules, - nthreads=nthreads, - force=self.force, - gdb_debug=False, - compiler_directives=dict( - profile=False, language_level=3, embedsignature=True - ), - ) - # Skip calling super() and jump straight to setuptools - setuptools.command.build_ext.build_ext.finalize_options(self) - - -cmdclass = dict() -cmdclass.update(versioneer.get_cmdclass()) -cmdclass["build_ext"] = build_ext_no_debug - - -############################################################################## -# - Python package generation ------------------------------------------------ - - +# TODO: We were previously forcing pylibraft to build with max optimization +# (-O3) instead of the usual -O2. Should we stick with that? If so, we'll need +# to bump that down into the new CMake files. setup(name='pylibraft', description="RAFT: Reusable Algorithms Functions and other Tools", version=versioneer.get_version(), @@ -184,18 +34,26 @@ def finalize_options(self): "Programming Language :: Python :: 3.7" ], author="NVIDIA Corporation", + # TODO: Replace with pyproject.toml. setup_requires=['cython'], - ext_modules=extensions, - package_data=dict.fromkeys( - find_packages(include=["pylibraft.distance", - "pylibraft.distance.includes", - "pylibraft.common", - "pylibraft.common.includes"]), - ["*.hpp", "*.pxd"], - ), + package_data={ + # Note: A dict comprehension with an explicit copy is necessary + # (rather than something simpler like a dict.fromkeys) because + # otherwise every package will refer to the same list and skbuild + # modifies it in place. + key: ["*.hpp", "*.pxd"] + for key in find_packages( + include=[ + "pylibraft.distance", + "pylibraft.distance.includes", + "pylibraft.common", + "pylibraft.common.includes" + ] + ) + }, packages=find_packages(include=['pylibraft', 'pylibraft.*']), - install_requires=install_requires, + install_requires=["cython"], license="Apache", - cmdclass=cmdclass, + cmdclass=versioneer.get_cmdclass(), zip_safe=False ) From 6287c1b9e9456adfaf838668b0a5ef888cca8cc0 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 27 Apr 2022 13:54:24 -0700 Subject: [PATCH 03/37] Rework add_cython_modules to support passing in the list of libraries. --- python/pylibraft/cmake/raft_python_helpers.cmake | 12 +++++++++--- python/pylibraft/pylibraft/distance/CMakeLists.txt | 5 ++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/python/pylibraft/cmake/raft_python_helpers.cmake b/python/pylibraft/cmake/raft_python_helpers.cmake index c72afb252e..44d5afb2cc 100644 --- a/python/pylibraft/cmake/raft_python_helpers.cmake +++ b/python/pylibraft/cmake/raft_python_helpers.cmake @@ -25,11 +25,15 @@ Generate C(++) from Cython and create Python modules. Creates a Cython target for a module, then adds a corresponding Python extension module. -``ModuleName`` +``cython_modules`` The list of modules to build. +``linked_libraries`` + The list of libraries that need to be linked into all modules. In RAPIDS, + this list usually contains (at minimum) the corresponding C++ libraries. + #]=======================================================================] -function(add_cython_modules cython_modules) +function(add_cython_modules cython_modules linked_libraries) foreach(cython_module ${cython_modules}) add_cython_target(${cython_module} CXX PY3) add_library(${cython_module} MODULE ${cython_module}) @@ -37,7 +41,9 @@ function(add_cython_modules cython_modules) # To avoid libraries being prefixed with "lib". set_target_properties(${cython_module} PROPERTIES PREFIX "") - target_link_libraries(${cython_module} raft::raft) + foreach(lib ${linked_libraries}) + target_link_libraries(${cython_module} ${lib}) + endforeach() # Compute the install directory relative to the source and rely on installs being relative to # the CMAKE_PREFIX_PATH for e.g. editable installs. diff --git a/python/pylibraft/pylibraft/distance/CMakeLists.txt b/python/pylibraft/pylibraft/distance/CMakeLists.txt index a44b2c1205..92614a3fcc 100644 --- a/python/pylibraft/pylibraft/distance/CMakeLists.txt +++ b/python/pylibraft/pylibraft/distance/CMakeLists.txt @@ -14,8 +14,7 @@ # Set the list of Cython files to build set(cython_modules pairwise_distance) +set(linked_libraries raft::raft raft::distance) # Build all of the Cython targets -add_cython_modules("${cython_modules}") - -target_link_libraries(pairwise_distance raft::distance) +add_cython_modules("${cython_modules}" "${linked_libraries}") From a0c5d5446640a479916ac207a1439106f3e55821 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 27 Apr 2022 14:05:04 -0700 Subject: [PATCH 04/37] Rework add_cython_modules to support passing in the base directory for installation. --- python/pylibraft/cmake/raft_python_helpers.cmake | 9 +++++++-- python/pylibraft/pylibraft/distance/CMakeLists.txt | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/python/pylibraft/cmake/raft_python_helpers.cmake b/python/pylibraft/cmake/raft_python_helpers.cmake index 44d5afb2cc..3f83574dfe 100644 --- a/python/pylibraft/cmake/raft_python_helpers.cmake +++ b/python/pylibraft/cmake/raft_python_helpers.cmake @@ -32,8 +32,13 @@ extension module. The list of libraries that need to be linked into all modules. In RAPIDS, this list usually contains (at minimum) the corresponding C++ libraries. +``install_base_directory`` + The source directory of the project. This directory is used to compute the + relative install path, which is necessary to propertly support differently + configured installations such as installing in place vs. out of place. + #]=======================================================================] -function(add_cython_modules cython_modules linked_libraries) +function(add_cython_modules cython_modules linked_libraries install_base_directory) foreach(cython_module ${cython_modules}) add_cython_target(${cython_module} CXX PY3) add_library(${cython_module} MODULE ${cython_module}) @@ -47,7 +52,7 @@ function(add_cython_modules cython_modules linked_libraries) # Compute the install directory relative to the source and rely on installs being relative to # the CMAKE_PREFIX_PATH for e.g. editable installs. - cmake_path(RELATIVE_PATH CMAKE_CURRENT_SOURCE_DIR BASE_DIRECTORY ${raft-python_SOURCE_DIR} + cmake_path(RELATIVE_PATH CMAKE_CURRENT_SOURCE_DIR BASE_DIRECTORY ${install_base_directory} OUTPUT_VARIABLE install_dst) install(TARGETS ${cython_module} DESTINATION ${install_dst}) endforeach(cython_module ${cython_sources}) diff --git a/python/pylibraft/pylibraft/distance/CMakeLists.txt b/python/pylibraft/pylibraft/distance/CMakeLists.txt index 92614a3fcc..bdfb3e5675 100644 --- a/python/pylibraft/pylibraft/distance/CMakeLists.txt +++ b/python/pylibraft/pylibraft/distance/CMakeLists.txt @@ -17,4 +17,5 @@ set(cython_modules pairwise_distance) set(linked_libraries raft::raft raft::distance) # Build all of the Cython targets -add_cython_modules("${cython_modules}" "${linked_libraries}") +# TODO: Depending on how project calls interact we can't rely on variables like CMAKE_PROJECT_NAME, otherwise we could use that to get the _SOURCE_DIR and it wouldn't have to be a parameter. Can we find an alternate approach here? +add_cython_modules("${cython_modules}" "${linked_libraries}" ${raft-python_SOURCE_DIR}) From f0dcb67cd9323ee245085eae78d7fac07ada2218 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 27 Apr 2022 14:45:45 -0700 Subject: [PATCH 05/37] Switch to using new rapids-cmake function for creating cython modules. --- cpp/CMakeLists.txt | 2 +- python/pylibraft/CMakeLists.txt | 5 ++ .../pylibraft/cmake/raft_python_helpers.cmake | 59 ------------------- .../pylibraft/distance/CMakeLists.txt | 2 +- 4 files changed, 7 insertions(+), 61 deletions(-) delete mode 100644 python/pylibraft/cmake/raft_python_helpers.cmake diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index fcdba2da16..900561a558 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -17,7 +17,7 @@ set(RAPIDS_VERSION "22.06") set(RAFT_VERSION "${RAPIDS_VERSION}.00") cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) -file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake +file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(rapids-cmake) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 48c0164c90..01d81f2548 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -16,6 +16,11 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) set(raft_version 22.06.00) +file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake + ${CMAKE_BINARY_DIR}/RAPIDS.cmake) +include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) +include(rapids-cython) + set(CYTHON_FLAGS "--directive binding=True,embedsignature=True,always_allow_keywords=True" CACHE STRING "The directives for Cython compilation.") diff --git a/python/pylibraft/cmake/raft_python_helpers.cmake b/python/pylibraft/cmake/raft_python_helpers.cmake deleted file mode 100644 index 3f83574dfe..0000000000 --- a/python/pylibraft/cmake/raft_python_helpers.cmake +++ /dev/null @@ -1,59 +0,0 @@ -# ============================================================================= -# Copyright (c) 2022, NVIDIA CORPORATION. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except -# in compliance with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software distributed under the License -# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express -# or implied. See the License for the specific language governing permissions and limitations under -# the License. -# ============================================================================= - -#[=======================================================================[.rst: -add_cython_modules ------------------- - -Generate C(++) from Cython and create Python modules. - -.. code-block:: cmake - - add_cython_modules() - -Creates a Cython target for a module, then adds a corresponding Python -extension module. - -``cython_modules`` - The list of modules to build. - -``linked_libraries`` - The list of libraries that need to be linked into all modules. In RAPIDS, - this list usually contains (at minimum) the corresponding C++ libraries. - -``install_base_directory`` - The source directory of the project. This directory is used to compute the - relative install path, which is necessary to propertly support differently - configured installations such as installing in place vs. out of place. - -#]=======================================================================] -function(add_cython_modules cython_modules linked_libraries install_base_directory) - foreach(cython_module ${cython_modules}) - add_cython_target(${cython_module} CXX PY3) - add_library(${cython_module} MODULE ${cython_module}) - python_extension_module(${cython_module}) - - # To avoid libraries being prefixed with "lib". - set_target_properties(${cython_module} PROPERTIES PREFIX "") - foreach(lib ${linked_libraries}) - target_link_libraries(${cython_module} ${lib}) - endforeach() - - # Compute the install directory relative to the source and rely on installs being relative to - # the CMAKE_PREFIX_PATH for e.g. editable installs. - cmake_path(RELATIVE_PATH CMAKE_CURRENT_SOURCE_DIR BASE_DIRECTORY ${install_base_directory} - OUTPUT_VARIABLE install_dst) - install(TARGETS ${cython_module} DESTINATION ${install_dst}) - endforeach(cython_module ${cython_sources}) -endfunction() diff --git a/python/pylibraft/pylibraft/distance/CMakeLists.txt b/python/pylibraft/pylibraft/distance/CMakeLists.txt index bdfb3e5675..8688f74c59 100644 --- a/python/pylibraft/pylibraft/distance/CMakeLists.txt +++ b/python/pylibraft/pylibraft/distance/CMakeLists.txt @@ -18,4 +18,4 @@ set(linked_libraries raft::raft raft::distance) # Build all of the Cython targets # TODO: Depending on how project calls interact we can't rely on variables like CMAKE_PROJECT_NAME, otherwise we could use that to get the _SOURCE_DIR and it wouldn't have to be a parameter. Can we find an alternate approach here? -add_cython_modules("${cython_modules}" "${linked_libraries}" ${raft-python_SOURCE_DIR}) +create_cython_modules("${cython_modules}" "${linked_libraries}" ${raft-python_SOURCE_DIR}) From 7b04525a3af1d90328dec9003addb23dcec70b22 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 27 Apr 2022 14:51:40 -0700 Subject: [PATCH 06/37] Remove skbuild patch and rely on rapids-cmake. --- python/pylibraft/CMakeLists.txt | 1 - python/pylibraft/cmake/CMakeLists.txt | 16 ------- python/pylibraft/cmake/skbuild_patches.cmake | 48 -------------------- 3 files changed, 65 deletions(-) delete mode 100644 python/pylibraft/cmake/CMakeLists.txt delete mode 100644 python/pylibraft/cmake/skbuild_patches.cmake diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 01d81f2548..1b9032d343 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -67,5 +67,4 @@ if(NOT RAFT_FOUND) add_subdirectory(../../cpp raft-cpp) endif() -add_subdirectory(cmake) add_subdirectory(pylibraft/distance) diff --git a/python/pylibraft/cmake/CMakeLists.txt b/python/pylibraft/cmake/CMakeLists.txt deleted file mode 100644 index 184099c83c..0000000000 --- a/python/pylibraft/cmake/CMakeLists.txt +++ /dev/null @@ -1,16 +0,0 @@ -# ============================================================================= -# Copyright (c) 2022, NVIDIA CORPORATION. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except -# in compliance with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software distributed under the License -# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express -# or implied. See the License for the specific language governing permissions and limitations under -# the License. -# ============================================================================= - -include(raft_python_helpers.cmake) -include(skbuild_patches.cmake) diff --git a/python/pylibraft/cmake/skbuild_patches.cmake b/python/pylibraft/cmake/skbuild_patches.cmake deleted file mode 100644 index 9eb0cc8e25..0000000000 --- a/python/pylibraft/cmake/skbuild_patches.cmake +++ /dev/null @@ -1,48 +0,0 @@ -# ============================================================================= -# Copyright (c) 2022, NVIDIA CORPORATION. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except -# in compliance with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software distributed under the License -# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express -# or implied. See the License for the specific language governing permissions and limitations under -# the License. -# ============================================================================= - -#[=======================================================================[.rst: -_set_python_extension_symbol_visibility ---------------------------------------- - -The original version of this function in scikit-build runs a linker script to -modify the visibility of symbols. This version is a patch to avoid overwriting -the visibility of symbols because RAFT specifically overrides some symbol -visibility in order to share certain functional-local static variables. - -#]=======================================================================] -# TODO: Should we guard this based on a scikit-build version? Override this function to avoid -# scikit-build clobbering symbol visibility. -function(_set_python_extension_symbol_visibility _target) - if(PYTHON_VERSION_MAJOR VERSION_GREATER 2) - set(_modinit_prefix "PyInit_") - else() - set(_modinit_prefix "init") - endif() - message("_modinit_prefix:${_modinit_prefix}") - if("${CMAKE_C_COMPILER_ID}" STREQUAL "MSVC") - set_target_properties(${_target} PROPERTIES LINK_FLAGS "/EXPORT:${_modinit_prefix}${_target}") - elseif("${CMAKE_C_COMPILER_ID}" STREQUAL "GNU" AND NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin") - set(_script_path ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/${_target}-version-script.map) - file( - WRITE ${_script_path} - # Note: The change is to this script, which does not indiscriminately mark all non PyInit - # symbols as local. - "{global: ${_modinit_prefix}${_target}; };") - set_property( - TARGET ${_target} - APPEND_STRING - PROPERTY LINK_FLAGS " -Wl,--version-script=\"${_script_path}\"") - endif() -endfunction() From a05ef51df5e589bf72824aace4d8e24c39969457 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 27 Apr 2022 15:07:32 -0700 Subject: [PATCH 07/37] Standardize some more boilerplate and add TODOs. --- cpp/CMakeLists.txt | 1 + python/pylibraft/CMakeLists.txt | 11 +---------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 900561a558..2e7c7cf47c 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -17,6 +17,7 @@ set(RAPIDS_VERSION "22.06") set(RAFT_VERSION "${RAPIDS_VERSION}.00") cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) +# TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 1b9032d343..a4dd2e1e33 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -16,15 +16,12 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) set(raft_version 22.06.00) +# TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(rapids-cython) -set(CYTHON_FLAGS - "--directive binding=True,embedsignature=True,always_allow_keywords=True" - CACHE STRING "The directives for Cython compilation.") - project( raft-python VERSION ${raft_version} @@ -43,12 +40,6 @@ project( option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulting to local files" OFF) -find_package(PythonExtensions REQUIRED) -find_package(Cython REQUIRED) - -# Ignore unused variable warning. -set(ignored_variable "${SKBUILD}") - # If the user requested it we attempt to find RAFT. if(FIND_RAFT_CPP) find_package(RAFT ${raft_version}) From 12fe3ce5fb0904ecd74ee1509cbc2ac8fd817f3a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 11:32:16 -0700 Subject: [PATCH 08/37] Update build to use newest rapids-cython code. --- python/pylibraft/CMakeLists.txt | 3 ++- python/pylibraft/pylibraft/distance/CMakeLists.txt | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index a4dd2e1e33..3df5c7f5de 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -20,7 +20,6 @@ set(raft_version 22.06.00) file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) -include(rapids-cython) project( raft-python @@ -58,4 +57,6 @@ if(NOT RAFT_FOUND) add_subdirectory(../../cpp raft-cpp) endif() +include(rapids-cython) +rapids_cython_init() add_subdirectory(pylibraft/distance) diff --git a/python/pylibraft/pylibraft/distance/CMakeLists.txt b/python/pylibraft/pylibraft/distance/CMakeLists.txt index 8688f74c59..4281a983dd 100644 --- a/python/pylibraft/pylibraft/distance/CMakeLists.txt +++ b/python/pylibraft/pylibraft/distance/CMakeLists.txt @@ -13,9 +13,11 @@ # ============================================================================= # Set the list of Cython files to build -set(cython_modules pairwise_distance) +set(cython_modules pairwise_distance.pyx) set(linked_libraries raft::raft raft::distance) # Build all of the Cython targets -# TODO: Depending on how project calls interact we can't rely on variables like CMAKE_PROJECT_NAME, otherwise we could use that to get the _SOURCE_DIR and it wouldn't have to be a parameter. Can we find an alternate approach here? -create_cython_modules("${cython_modules}" "${linked_libraries}" ${raft-python_SOURCE_DIR}) +rapids_cython_create_modules( + SOURCE_FILES "${cython_modules}" + LINKED_LIBRARIES "${linked_libraries}" + CXX) From 3b5dc4698e1ad29c7eb632719a1db15327c9515a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 15:08:57 -0700 Subject: [PATCH 09/37] Rename project to pylibraft. --- python/pylibraft/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 3df5c7f5de..2211a88678 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -17,12 +17,12 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) set(raft_version 22.06.00) # TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. -file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake +file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/cf54c7d6e215c1188e1b9eb660565bdf42b83b64/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) project( - raft-python + pylibraft VERSION ${raft_version} LANGUAGES # TODO: Building Python extension modules via the python_extension_module requires the C # language to be enabled here. The test project that is built in scikit-build to verify From a1ec5176d38333c971fd0e768791e3117b2717c9 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 15:11:15 -0700 Subject: [PATCH 10/37] Rename cython_modules to cython_sources. --- python/pylibraft/pylibraft/distance/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/pylibraft/pylibraft/distance/CMakeLists.txt b/python/pylibraft/pylibraft/distance/CMakeLists.txt index 4281a983dd..f8a7faf450 100644 --- a/python/pylibraft/pylibraft/distance/CMakeLists.txt +++ b/python/pylibraft/pylibraft/distance/CMakeLists.txt @@ -13,11 +13,11 @@ # ============================================================================= # Set the list of Cython files to build -set(cython_modules pairwise_distance.pyx) +set(cython_sources pairwise_distance.pyx) set(linked_libraries raft::raft raft::distance) # Build all of the Cython targets rapids_cython_create_modules( - SOURCE_FILES "${cython_modules}" + SOURCE_FILES "${cython_sources}" LINKED_LIBRARIES "${linked_libraries}" CXX) From 6dc4c8f35c8a5991099b7e5381a963d55d39179a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 16:06:19 -0700 Subject: [PATCH 11/37] Remove outdated TODO. --- python/pylibraft/setup.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/python/pylibraft/setup.py b/python/pylibraft/setup.py index 495cbd7433..5c37feaf90 100644 --- a/python/pylibraft/setup.py +++ b/python/pylibraft/setup.py @@ -14,16 +14,11 @@ # limitations under the License. # - from setuptools import find_packages from skbuild import setup import versioneer - -# TODO: We were previously forcing pylibraft to build with max optimization -# (-O3) instead of the usual -O2. Should we stick with that? If so, we'll need -# to bump that down into the new CMake files. setup(name='pylibraft', description="RAFT: Reusable Algorithms Functions and other Tools", version=versioneer.get_version(), From 65a2d0472eac6062c2b690a6ac6fcc0d1fc54633 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 16:19:34 -0700 Subject: [PATCH 12/37] Add pyproject.toml and remove superfluous components of setup.py. --- python/pylibraft/pyproject.toml | 24 ++++++++++++++++++++++++ python/pylibraft/setup.py | 3 --- 2 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 python/pylibraft/pyproject.toml diff --git a/python/pylibraft/pyproject.toml b/python/pylibraft/pyproject.toml new file mode 100644 index 0000000000..e3fe544bef --- /dev/null +++ b/python/pylibraft/pyproject.toml @@ -0,0 +1,24 @@ +# Copyright (c) 2022, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[build-system] + +requires = [ + "wheel", + "setuptools", + "cython>=0.29,<0.30", + "scikit-build>=0.13.1", + "cmake>=3.20.1,!=3.23.0", + "ninja", +] diff --git a/python/pylibraft/setup.py b/python/pylibraft/setup.py index 5c37feaf90..4065bacb48 100644 --- a/python/pylibraft/setup.py +++ b/python/pylibraft/setup.py @@ -29,8 +29,6 @@ "Programming Language :: Python :: 3.7" ], author="NVIDIA Corporation", - # TODO: Replace with pyproject.toml. - setup_requires=['cython'], package_data={ # Note: A dict comprehension with an explicit copy is necessary # (rather than something simpler like a dict.fromkeys) because @@ -47,7 +45,6 @@ ) }, packages=find_packages(include=['pylibraft', 'pylibraft.*']), - install_requires=["cython"], license="Apache", cmdclass=versioneer.get_cmdclass(), zip_safe=False From 381df618de9c3775d31777c7ea61164481829ae9 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 16:23:44 -0700 Subject: [PATCH 13/37] Update conda files. --- conda/environments/raft_dev_cuda11.0.yml | 3 +++ conda/environments/raft_dev_cuda11.2.yml | 3 +++ conda/environments/raft_dev_cuda11.4.yml | 3 +++ conda/environments/raft_dev_cuda11.5.yml | 3 +++ conda/recipes/pylibraft/meta.yaml | 2 ++ 5 files changed, 14 insertions(+) diff --git a/conda/environments/raft_dev_cuda11.0.yml b/conda/environments/raft_dev_cuda11.0.yml index 8bcf9abca2..9ec33e5998 100644 --- a/conda/environments/raft_dev_cuda11.0.yml +++ b/conda/environments/raft_dev_cuda11.0.yml @@ -8,6 +8,9 @@ dependencies: - cudatoolkit=11.0 - clang=11.1.0 - clang-tools=11.1.0 +- cython>=0.29,<0.30 +- cmake>=3.20.1,!=3.23.0 +- scikit-build>=0.13.1 - rapids-build-env=22.02.* - rapids-notebook-env=22.02.* - rapids-doc-env=22.02.* diff --git a/conda/environments/raft_dev_cuda11.2.yml b/conda/environments/raft_dev_cuda11.2.yml index e8cf33c8f8..de182252ca 100644 --- a/conda/environments/raft_dev_cuda11.2.yml +++ b/conda/environments/raft_dev_cuda11.2.yml @@ -8,6 +8,9 @@ dependencies: - cudatoolkit=11.2 - clang=11.1.0 - clang-tools=11.1.0 +- cython>=0.29,<0.30 +- cmake>=3.20.1,!=3.23.0 +- scikit-build>=0.13.1 - rapids-build-env=22.02.* - rapids-notebook-env=22.02.* - rapids-doc-env=22.02.* diff --git a/conda/environments/raft_dev_cuda11.4.yml b/conda/environments/raft_dev_cuda11.4.yml index 2d7ec1bfed..a7d71a179a 100644 --- a/conda/environments/raft_dev_cuda11.4.yml +++ b/conda/environments/raft_dev_cuda11.4.yml @@ -8,6 +8,9 @@ dependencies: - cudatoolkit=11.4 - clang=11.1.0 - clang-tools=11.1.0 +- cython>=0.29,<0.30 +- cmake>=3.20.1,!=3.23.0 +- scikit-build>=0.13.1 - rapids-build-env=22.02.* - rapids-notebook-env=22.02.* - rapids-doc-env=22.02.* diff --git a/conda/environments/raft_dev_cuda11.5.yml b/conda/environments/raft_dev_cuda11.5.yml index ad5cb9630d..8c6414dd9f 100644 --- a/conda/environments/raft_dev_cuda11.5.yml +++ b/conda/environments/raft_dev_cuda11.5.yml @@ -9,6 +9,9 @@ dependencies: - cuda-python >=11.5,<12.0 - clang=11.1.0 - clang-tools=11.1.0 +- cython>=0.29,<0.30 +- cmake>=3.20.1,!=3.23.0 +- scikit-build>=0.13.1 - rapids-build-env=22.02.* - rapids-notebook-env=22.02.* - rapids-doc-env=22.02.* diff --git a/conda/recipes/pylibraft/meta.yaml b/conda/recipes/pylibraft/meta.yaml index eaca379c4e..4576e5146f 100644 --- a/conda/recipes/pylibraft/meta.yaml +++ b/conda/recipes/pylibraft/meta.yaml @@ -28,6 +28,8 @@ requirements: - python x.x - setuptools - cython>=0.29,<0.30 + - cmake>=3.20.1,!=3.23.0 + - scikit-build>=0.13.1 - rmm {{ minor_version }} - libraft-headers {{ version }} - libraft-distance {{ version }} From 87f44f4304da39df1fd3668c0c91c4a7354ea81b Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 16:26:18 -0700 Subject: [PATCH 14/37] Update update_version.sh and the name of the version variable in CMakeLists.txt. --- ci/release/update-version.sh | 1 + python/pylibraft/CMakeLists.txt | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ci/release/update-version.sh b/ci/release/update-version.sh index f22e883843..79716fc78c 100755 --- a/ci/release/update-version.sh +++ b/ci/release/update-version.sh @@ -32,6 +32,7 @@ function sed_runner() { } sed_runner 's/'"RAFT VERSION .* LANGUAGES"'/'"RAFT VERSION ${NEXT_FULL_TAG} LANGUAGES"'/g' cpp/CMakeLists.txt +sed_runner 's/'"pylibraft_version .*)"'/'"pylibraft_version ${NEXT_FULL_TAG})"'/g' python/pylibraft/CMakeLists.txt sed_runner 's/'"branch-.*\/RAPIDS.cmake"'/'"branch-${NEXT_SHORT_TAG}\/RAPIDS.cmake"'/g' cpp/CMakeLists.txt for FILE in conda/environments/*.yml; do diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 2211a88678..5c1119a415 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -14,7 +14,7 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) -set(raft_version 22.06.00) +set(pylibraft_version 22.06.00) # TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/cf54c7d6e215c1188e1b9eb660565bdf42b83b64/RAPIDS.cmake @@ -23,7 +23,7 @@ include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) project( pylibraft - VERSION ${raft_version} + VERSION ${pylibraft_version} LANGUAGES # TODO: Building Python extension modules via the python_extension_module requires the C # language to be enabled here. The test project that is built in scikit-build to verify # various linking options for the python library is hardcoded to build with C, so until @@ -41,7 +41,7 @@ option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulti # If the user requested it we attempt to find RAFT. if(FIND_RAFT_CPP) - find_package(RAFT ${raft_version}) + find_package(RAFT ${pylibraft_version}) else() set(RAFT_FOUND OFF) endif() From 1e8dc7e74a6296d3f1167d11391725f97bfbc616 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 16:28:46 -0700 Subject: [PATCH 15/37] Improve formatting of build.sh. --- build.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/build.sh b/build.sh index 568de2956d..07e0718875 100755 --- a/build.sh +++ b/build.sh @@ -99,20 +99,20 @@ fi # Check for valid usage if (( ${NUMARGS} != 0 )); then for a in ${ARGS}; do - if ! (echo " ${VALIDARGS} " | grep -q " ${a} "); then - echo "Invalid option: ${a}" - exit 1 - fi + if ! (echo " ${VALIDARGS} " | grep -q " ${a} "); then + echo "Invalid option: ${a}" + exit 1 + fi done fi # Process flags if hasArg --install; then - INSTALL_TARGET="install" + INSTALL_TARGET="install" fi if hasArg --minimal-deps; then - ENABLE_thrust_DEPENDENCY=OFF + ENABLE_thrust_DEPENDENCY=OFF fi if hasArg -v; then @@ -128,7 +128,7 @@ if hasArg --allgpuarch; then fi if hasArg --compile-libs || (( ${NUMARGS} == 0 )); then - COMPILE_LIBRARIES=ON + COMPILE_LIBRARIES=ON fi if hasArg --compile-nn || hasArg --compile-libs || (( ${NUMARGS} == 0 )); then @@ -167,11 +167,11 @@ if hasArg clean; then CLEAN=1 fi if hasArg uninstall; then - UNINSTALL=1 + UNINSTALL=1 fi if [[ ${CMAKE_TARGET} == "" ]]; then - CMAKE_TARGET="all" + CMAKE_TARGET="all" fi # If clean given, run it prior to any other steps @@ -198,8 +198,8 @@ fi # Pyraft requires ucx + nccl if (( ${NUMARGS} == 0 )) || hasArg pyraft || hasArg docs; then - ENABLE_nccl_DEPENDENCY=ON - ENABLE_ucx_DEPENDENCY=ON + ENABLE_nccl_DEPENDENCY=ON + ENABLE_ucx_DEPENDENCY=ON fi ################################################################################ # Configure for building all C++ targets From d805dd458fd319b02edc1b5ab2ef1b2d709442d9 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 4 May 2022 16:38:47 -0700 Subject: [PATCH 16/37] Update relevant parts of build.sh. --- build.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/build.sh b/build.sh index 07e0718875..e2394a3861 100755 --- a/build.sh +++ b/build.sh @@ -50,7 +50,7 @@ HELP="$0 [ ...] [ ...] LIBRAFT_BUILD_DIR=${LIBRAFT_BUILD_DIR:=${REPODIR}/cpp/build} SPHINX_BUILD_DIR=${REPODIR}/docs PY_RAFT_BUILD_DIR=${REPODIR}/python/raft/build -PY_LIBRAFT_BUILD_DIR=${REPODIR}/python/pylibraft/build +PY_LIBRAFT_BUILD_DIR=${REPODIR}/python/pylibraft/_skbuild BUILD_DIRS="${LIBRAFT_BUILD_DIR} ${PY_RAFT_BUILD_DIR} ${PY_LIBRAFT_BUILD_DIR}" # Set defaults for vars modified by flags to this script @@ -256,10 +256,9 @@ fi if (( ${NUMARGS} == 0 )) || hasArg pylibraft; then cd ${REPODIR}/python/pylibraft + python setup.py build_ext -j${PARALLEL_LEVEL:-1} --inplace -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} -DCMAKE_LIBRARY_PATH=${LIBRAFT_BUILD_DIR} if [[ ${INSTALL_TARGET} != "" ]]; then - python setup.py build_ext -j${PARALLEL_LEVEL:-1} --inplace --library-dir=${LIBRAFT_BUILD_DIR} install --single-version-externally-managed --record=record.txt - else - python setup.py build_ext -j${PARALLEL_LEVEL:-1} --inplace --library-dir=${LIBRAFT_BUILD_DIR} + python setup.py install --single-version-externally-managed --record=record.txt -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} fi fi From 920bade4eb74d16a00863bb8c363dfac4bcd0ede Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 5 May 2022 17:51:26 -0700 Subject: [PATCH 17/37] Update rapids-cmake link now that rapids-cython is merged. --- cpp/CMakeLists.txt | 3 +-- python/pylibraft/CMakeLists.txt | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 2e7c7cf47c..fcdba2da16 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -17,8 +17,7 @@ set(RAPIDS_VERSION "22.06") set(RAFT_VERSION "${RAPIDS_VERSION}.00") cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) -# TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. -file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake +file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(rapids-cmake) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 5c1119a415..7764199f9f 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -16,8 +16,7 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) set(pylibraft_version 22.06.00) -# TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. -file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/cf54c7d6e215c1188e1b9eb660565bdf42b83b64/RAPIDS.cmake +file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) From 517527a18d1b37856ebdd8b3becd853bcb04933f Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 08:12:58 -0700 Subject: [PATCH 18/37] Set the language level. --- python/pylibraft/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 7764199f9f..c3e9000375 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -35,6 +35,9 @@ project( # We can remove CUDA once we upgrade the minimum required CMake version to 3.22. CUDA) +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulting to local files" OFF) From abd5bc061d59abd032112fd6f277bfde95915962 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 10:09:32 -0700 Subject: [PATCH 19/37] Revert "Update rapids-cmake link now that rapids-cython is merged." This reverts commit 920bade4eb74d16a00863bb8c363dfac4bcd0ede. --- cpp/CMakeLists.txt | 3 ++- python/pylibraft/CMakeLists.txt | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index fcdba2da16..2e7c7cf47c 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -17,7 +17,8 @@ set(RAPIDS_VERSION "22.06") set(RAFT_VERSION "${RAPIDS_VERSION}.00") cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) -file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake +# TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. +file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(rapids-cmake) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index c3e9000375..4cf7dfcca3 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -16,7 +16,8 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) set(pylibraft_version 22.06.00) -file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake +# TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. +file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/cf54c7d6e215c1188e1b9eb660565bdf42b83b64/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) From f1c66421d3eaecc7a188bce25ddb00e786c06c8a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 10:20:59 -0700 Subject: [PATCH 20/37] Add support for passing arbitrary CMake arguments through the build script and use that to ask pylibraft to find the C++ RAFT libraries. --- build.sh | 57 +++++++++++++++++++++++++++++++++++-------------- ci/gpu/build.sh | 4 ++-- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/build.sh b/build.sh index e2394a3861..0b22332a75 100755 --- a/build.sh +++ b/build.sh @@ -31,19 +31,20 @@ HELP="$0 [ ...] [ ...] bench - build the benchmarks and is: - -v - verbose build mode - -g - build for debug - --compile-libs - compile shared libraries for all components - --compile-nn - compile shared library for nn component - --compile-dist - compile shared library for distance component - --minimal-deps - disables dependencies like thrust so they can be overridden. - can be useful for a pure header-only install - --allgpuarch - build for all supported GPU architectures - --buildfaiss - build faiss statically into raft - --install - install cmake targets - --nvtx - enable nvtx for profiling support - --show_depr_warn - show cmake deprecation warnings - -h - print this text + -v - verbose build mode + -g - build for debug + --compile-libs - compile shared libraries for all components + --compile-nn - compile shared library for nn component + --compile-dist - compile shared library for distance component + --minimal-deps - disables dependencies like thrust so they can be overridden. + can be useful for a pure header-only install + --allgpuarch - build for all supported GPU architectures + --buildfaiss - build faiss statically into raft + --install - install cmake targets + --nvtx - enable nvtx for profiling support + --show_depr_warn - show cmake deprecation warnings + --cmake-args=\\\"\\\" - pass arbitrary list of CMake configuration options (escape all quotes in argument) + -h - print this text default action (no args) is to build both libraft and pyraft targets " @@ -91,6 +92,28 @@ function hasArg { (( ${NUMARGS} != 0 )) && (echo " ${ARGS} " | grep -q " $1 ") } +function cmakeArgs { + # Check for multiple cmake args options + if [[ $(echo $ARGS | { grep -Eo "\-\-cmake\-args" || true; } | wc -l ) -gt 1 ]]; then + echo "Multiple --cmake-args options were provided, please provide only one: ${ARGS}" + exit 1 + fi + + # Check for cmake args option + if [[ -n $(echo $ARGS | { grep -E "\-\-cmake\-args" || true; } ) ]]; then + # There are possible weird edge cases that may cause this regex filter to output nothing and fail silently + # the true pipe will catch any weird edge cases that may happen and will cause the program to fall back + # on the invalid option error + CMAKE_ARGS=$(echo $ARGS | { grep -Eo "\-\-cmake\-args=\".+\"" || true; }) + if [[ -n ${CMAKE_ARGS} ]]; then + # Remove the full CMAKE_ARGS argument from list of args so that it passes validArgs function + ARGS=${ARGS//$CMAKE_ARGS/} + # Filter the full argument down to just the extra string that will be added to cmake call + CMAKE_ARGS=$(echo $CMAKE_ARGS | grep -Eo "\".+\"" | sed -e 's/^"//' -e 's/"$//') + fi + fi +} + if hasArg -h || hasArg --help; then echo "${HELP}" exit 0 @@ -98,6 +121,7 @@ fi # Check for valid usage if (( ${NUMARGS} != 0 )); then + cmakeArgs for a in ${ARGS}; do if ! (echo " ${VALIDARGS} " | grep -q " ${a} "); then echo "Invalid option: ${a}" @@ -229,7 +253,8 @@ if (( ${NUMARGS} == 0 )) || hasArg libraft || hasArg docs || hasArg tests || has -DRAFT_USE_FAISS_STATIC=${BUILD_STATIC_FAISS} \ -DRAFT_ENABLE_nccl_DEPENDENCY=${ENABLE_nccl_DEPENDENCY} \ -DRAFT_ENABLE_ucx_DEPENDENCY=${ENABLE_ucx_DEPENDENCY} \ - -DRAFT_ENABLE_thrust_DEPENDENCY=${ENABLE_thrust_DEPENDENCY} + -DRAFT_ENABLE_thrust_DEPENDENCY=${ENABLE_thrust_DEPENDENCY} \ + ${CMAKE_ARGS} if [[ ${CMAKE_TARGET} != "" ]]; then echo "-- Compiling targets: ${CMAKE_TARGET}, verbose=${VERBOSE_FLAG}" @@ -256,9 +281,9 @@ fi if (( ${NUMARGS} == 0 )) || hasArg pylibraft; then cd ${REPODIR}/python/pylibraft - python setup.py build_ext -j${PARALLEL_LEVEL:-1} --inplace -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} -DCMAKE_LIBRARY_PATH=${LIBRAFT_BUILD_DIR} + python setup.py build_ext -j${PARALLEL_LEVEL:-1} --inplace -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} -DCMAKE_LIBRARY_PATH=${LIBRAFT_BUILD_DIR} ${CMAKE_ARGS} if [[ ${INSTALL_TARGET} != "" ]]; then - python setup.py install --single-version-externally-managed --record=record.txt -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} + python setup.py install --single-version-externally-managed --record=record.txt -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} ${CMAKE_ARGS} fi fi diff --git a/ci/gpu/build.sh b/ci/gpu/build.sh index 570c847226..7b07278bbd 100644 --- a/ci/gpu/build.sh +++ b/ci/gpu/build.sh @@ -103,9 +103,9 @@ export LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH gpuci_logger "Build C++ and Python targets" # These should link against the existing shared libs if hasArg --skip-tests; then - "$WORKSPACE/build.sh" pyraft pylibraft libraft -v + "$WORKSPACE/build.sh" pyraft pylibraft libraft -v --cmake-args="-DFIND_RAFT_CPP=ON" else - "$WORKSPACE/build.sh" pyraft pylibraft libraft tests bench -v + "$WORKSPACE/build.sh" pyraft pylibraft libraft tests bench -v --cmake-args="-DFIND_RAFT_CPP=ON" fi gpuci_logger "sccache stats" From 5565ec22a1c9d8dfdb09ed1235b6aa16938cb323 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 11:08:12 -0700 Subject: [PATCH 21/37] Find CPP raft when building with conda. --- conda/recipes/pylibraft/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda/recipes/pylibraft/build.sh b/conda/recipes/pylibraft/build.sh index 442428e0ee..09f2117e9b 100644 --- a/conda/recipes/pylibraft/build.sh +++ b/conda/recipes/pylibraft/build.sh @@ -2,4 +2,4 @@ #!/usr/bin/env bash # This assumes the script is executed from the root of the repo directory -./build.sh pylibraft --install +./build.sh pylibraft --install --cmake-args="-DFIND_RAFT_CPP=ON" From d1c62f257548c3ec836b4ecf07743eddb419b0ac Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 11:37:25 -0700 Subject: [PATCH 22/37] Fix quotes. --- ci/gpu/build.sh | 4 ++-- conda/recipes/pylibraft/build.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ci/gpu/build.sh b/ci/gpu/build.sh index 7b07278bbd..cd4030e0b2 100644 --- a/ci/gpu/build.sh +++ b/ci/gpu/build.sh @@ -103,9 +103,9 @@ export LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH gpuci_logger "Build C++ and Python targets" # These should link against the existing shared libs if hasArg --skip-tests; then - "$WORKSPACE/build.sh" pyraft pylibraft libraft -v --cmake-args="-DFIND_RAFT_CPP=ON" + "$WORKSPACE/build.sh" pyraft pylibraft libraft -v --cmake-args=\"-DFIND_RAFT_CPP=ON\" else - "$WORKSPACE/build.sh" pyraft pylibraft libraft tests bench -v --cmake-args="-DFIND_RAFT_CPP=ON" + "$WORKSPACE/build.sh" pyraft pylibraft libraft tests bench -v --cmake-args=\"-DFIND_RAFT_CPP=ON\" fi gpuci_logger "sccache stats" diff --git a/conda/recipes/pylibraft/build.sh b/conda/recipes/pylibraft/build.sh index 09f2117e9b..d3c0770cba 100644 --- a/conda/recipes/pylibraft/build.sh +++ b/conda/recipes/pylibraft/build.sh @@ -2,4 +2,4 @@ #!/usr/bin/env bash # This assumes the script is executed from the root of the repo directory -./build.sh pylibraft --install --cmake-args="-DFIND_RAFT_CPP=ON" +./build.sh pylibraft --install --cmake-args=\"-DFIND_RAFT_CPP=ON\" From 38a99a08e2427b88885b8f7784a9203567f215b8 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 13:22:01 -0700 Subject: [PATCH 23/37] Revert "Revert "Update rapids-cmake link now that rapids-cython is merged."" This reverts commit abd5bc061d59abd032112fd6f277bfde95915962. --- cpp/CMakeLists.txt | 3 +-- python/pylibraft/CMakeLists.txt | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 2e7c7cf47c..fcdba2da16 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -17,8 +17,7 @@ set(RAPIDS_VERSION "22.06") set(RAFT_VERSION "${RAPIDS_VERSION}.00") cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) -# TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. -file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/rapids-cython/RAPIDS.cmake +file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(rapids-cmake) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 4cf7dfcca3..c3e9000375 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -16,8 +16,7 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) set(pylibraft_version 22.06.00) -# TODO: This link needs to be changed back to the mainline branch of rapids-cmake before the PR is merged. -file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/cf54c7d6e215c1188e1b9eb660565bdf42b83b64/RAPIDS.cmake +file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) From ba3a641b70ea8d8f7b0e66d251913b68887c84c3 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 13:22:14 -0700 Subject: [PATCH 24/37] Revert "Set the language level." This reverts commit 517527a18d1b37856ebdd8b3becd853bcb04933f. --- python/pylibraft/CMakeLists.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index c3e9000375..7764199f9f 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -35,9 +35,6 @@ project( # We can remove CUDA once we upgrade the minimum required CMake version to 3.22. CUDA) -set(CMAKE_CXX_STANDARD 17) -set(CMAKE_CXX_STANDARD_REQUIRED ON) - option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulting to local files" OFF) From 40908ff94cd7a376ebee653b1f4075acdf1c09ff Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 15:53:02 -0700 Subject: [PATCH 25/37] Fix finding raft CPP. --- python/pylibraft/CMakeLists.txt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 7764199f9f..ebaa239ec2 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -40,19 +40,18 @@ option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulti # If the user requested it we attempt to find RAFT. if(FIND_RAFT_CPP) - find_package(RAFT ${pylibraft_version}) + find_package(raft ${pylibraft_version} REQUIRED COMPONENTS distance) else() - set(RAFT_FOUND OFF) + set(raft_FOUND OFF) + set(raft_FIND_COMPONENTS distance) endif() -if(NOT RAFT_FOUND) +if(NOT raft_FOUND) set(BUILD_TESTS OFF) set(BUILD_BENCHMARKS OFF) set(BUILD_TESTS OFF) set(RAFT_COMPILE_LIBRARIES ON) set(RAFT_COMPILE_DIST_LIBRARY ON) - #RAFT_COMPILE_NN_LIBRARY OFF - #RAFT_ENABLE_NN_DEPENDENCIES ON add_subdirectory(../../cpp raft-cpp) endif() From 59cb1ac9d297abab958d6ecb01766e409e65575e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 16:34:58 -0700 Subject: [PATCH 26/37] Set options early and use them everywhere. --- cpp/CMakeLists.txt | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index fcdba2da16..6bd87eb120 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -62,8 +62,8 @@ option(DISABLE_OPENMP "Disable OpenMP" OFF) option(NVTX "Enable nvtx markers" OFF) option(RAFT_COMPILE_LIBRARIES "Enable building raft shared library instantiations" ${BUILD_TESTS}) -option(RAFT_COMPILE_NN_LIBRARY "Enable building raft nearest neighbors shared library instantiations" OFF) -option(RAFT_COMPILE_DIST_LIBRARY "Enable building raft distant shared library instantiations" OFF) +option(RAFT_COMPILE_NN_LIBRARY "Enable building raft nearest neighbors shared library instantiations" ${RAFT_COMPILE_LIBRARIES}) +option(RAFT_COMPILE_DIST_LIBRARY "Enable building raft distant shared library instantiations" ${RAFT_COMPILE_LIBRARIES}) option(RAFT_ENABLE_NN_DEPENDENCIES "Search for raft::nn dependencies like faiss" ${RAFT_COMPILE_LIBRARIES}) option(RAFT_ENABLE_mdspan_DEPENDENCY "Enable mdspan dependency" ON) @@ -132,7 +132,20 @@ include(cmake/modules/ConfigureCUDA.cmake) ############################################################################## # - Requirements ------------------------------------------------------------- -if(distance IN_LIST raft_FIND_COMPONENTS OR RAFT_COMPILE_LIBRARIES OR RAFT_COMPILE_DIST_LIBRARY) +if(RAFT_COMPILE_LIBRARIES) + set(RAFT_COMPILE_DIST_LIBRARY ON) + set(RAFT_COMPILE_NN_LIBRARY ON) +endif() + +if(distance IN_LIST raft_FIND_COMPONENTS) + set(RAFT_COMPILE_DIST_LIBRARY ON) +endif() + +if(nn IN_LIST raft_FIND_COMPONENTS) + set(RAFT_COMPILE_NN_LIBRARY ON) +endif() + +if(RAFT_COMPILE_DIST_LIBRARY) set(RAFT_ENABLE_cuco_DEPENDENCY ON) endif() @@ -181,7 +194,7 @@ target_link_libraries(raft INTERFACE target_compile_definitions(raft INTERFACE $<$:NVTX_ENABLED>) target_compile_features(raft INTERFACE cxx_std_17 $) -if(RAFT_COMPILE_LIBRARIES OR RAFT_COMPILE_DIST_LIBRARY OR RAFT_COMPILE_NN_LIBRARY) +if(RAFT_COMPILE_DIST_LIBRARY OR RAFT_COMPILE_NN_LIBRARY) file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/fatbin.ld" [=[ SECTIONS @@ -202,7 +215,7 @@ endif() set_target_properties(raft_distance PROPERTIES EXPORT_NAME distance) -if(RAFT_COMPILE_LIBRARIES OR RAFT_COMPILE_DIST_LIBRARY) +if(RAFT_COMPILE_DIST_LIBRARY) add_library(raft_distance_lib src/distance/pairwise_distance.cu src/distance/specializations/detail/canberra.cu @@ -278,7 +291,7 @@ endif() set_target_properties(raft_nn PROPERTIES EXPORT_NAME nn) -if(RAFT_COMPILE_LIBRARIES OR RAFT_COMPILE_NN_LIBRARY) +if(RAFT_COMPILE_NN_LIBRARY) add_library(raft_nn_lib src/nn/specializations/ball_cover.cu src/nn/specializations/detail/ball_cover_lowdim_pass_one_2d.cu From 47c2e4a33859bc6e254a6ffe6f5fcc1baac3eac9 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 16:35:11 -0700 Subject: [PATCH 27/37] Install raft_distance headers. --- cpp/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 6bd87eb120..4f2287fe10 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -355,6 +355,8 @@ if(TARGET raft_distance_lib) install(TARGETS raft_distance_lib DESTINATION ${lib_dir} EXPORT raft-distance-lib-exports) + install(DIRECTORY include/raft_distance + DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) endif() if(TARGET raft_nn_lib) From 60956db955ebc3ed169a2def2cb4ca7c72c7be3e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 17:20:30 -0700 Subject: [PATCH 28/37] Remove unnecessary extra variables set. --- python/pylibraft/CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index ebaa239ec2..92be880dff 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -49,8 +49,6 @@ endif() if(NOT raft_FOUND) set(BUILD_TESTS OFF) set(BUILD_BENCHMARKS OFF) - set(BUILD_TESTS OFF) - set(RAFT_COMPILE_LIBRARIES ON) set(RAFT_COMPILE_DIST_LIBRARY ON) add_subdirectory(../../cpp raft-cpp) endif() From 0e7a18e21f8c92e742786503bf5ccdd8de9b015d Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 17:26:19 -0700 Subject: [PATCH 29/37] Add rapids cmake download line to update-version.sh. --- ci/release/update-version.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/release/update-version.sh b/ci/release/update-version.sh index 79716fc78c..85c256c61a 100755 --- a/ci/release/update-version.sh +++ b/ci/release/update-version.sh @@ -34,6 +34,7 @@ function sed_runner() { sed_runner 's/'"RAFT VERSION .* LANGUAGES"'/'"RAFT VERSION ${NEXT_FULL_TAG} LANGUAGES"'/g' cpp/CMakeLists.txt sed_runner 's/'"pylibraft_version .*)"'/'"pylibraft_version ${NEXT_FULL_TAG})"'/g' python/pylibraft/CMakeLists.txt sed_runner 's/'"branch-.*\/RAPIDS.cmake"'/'"branch-${NEXT_SHORT_TAG}\/RAPIDS.cmake"'/g' cpp/CMakeLists.txt +sed_runner 's/'"branch-.*\/RAPIDS.cmake"'/'"branch-${NEXT_SHORT_TAG}\/RAPIDS.cmake"'/g' python/pylibraft/CMakeLists.txt for FILE in conda/environments/*.yml; do sed_runner "s/ucx-py=.*/ucx-py=${NEXT_UCX_PY_VERSION}/g" ${FILE}; From 8e75eef69d5bcc23001be4b358bb0b8c5534c5f7 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 22:28:41 -0700 Subject: [PATCH 30/37] Conditionally enable CUDA when building the CPP library. --- python/pylibraft/CMakeLists.txt | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 92be880dff..732bfd6e32 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -28,12 +28,7 @@ project( # various linking options for the python library is hardcoded to build with C, so until # that is fixed we need to keep C. C - CXX - # TODO: The C++ RMM CMake configuration targets cuda_std_17 features, which prior to - # CMake 3.22 will also pull in the corresponding required languages even if this project - # does not actually require those languages. As a result, we need to include CUDA here. - # We can remove CUDA once we upgrade the minimum required CMake version to 3.22. - CUDA) + CXX) option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulting to local files" OFF) @@ -43,10 +38,10 @@ if(FIND_RAFT_CPP) find_package(raft ${pylibraft_version} REQUIRED COMPONENTS distance) else() set(raft_FOUND OFF) - set(raft_FIND_COMPONENTS distance) endif() if(NOT raft_FOUND) + enable_language(CUDA) set(BUILD_TESTS OFF) set(BUILD_BENCHMARKS OFF) set(RAFT_COMPILE_DIST_LIBRARY ON) From 353257149b797bca25b98f27c523bed4daf257e0 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 22:31:06 -0700 Subject: [PATCH 31/37] Minor reformatting. --- python/pylibraft/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 732bfd6e32..00fe1ec1aa 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -50,4 +50,5 @@ endif() include(rapids-cython) rapids_cython_init() + add_subdirectory(pylibraft/distance) From ef2747658b1488d78f8fcc8fb565271676baaf08 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 6 May 2022 22:33:23 -0700 Subject: [PATCH 32/37] Add comment to language enabling. --- python/pylibraft/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 00fe1ec1aa..5ca2c90ee2 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -41,6 +41,9 @@ else() endif() if(NOT raft_FOUND) + # TODO: This will not be necessary once we upgrade to CMake 3.22, which will + # pull in the required languages for the C++ project even if this project + # does not require those languges. enable_language(CUDA) set(BUILD_TESTS OFF) set(BUILD_BENCHMARKS OFF) From fb28a5d401ae128fb60e197b22859229d925644a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Wed, 11 May 2022 19:50:23 -0700 Subject: [PATCH 33/37] Fail to build the Python library if linking against a pre-built C++ library without the distance component. --- cpp/CMakeLists.txt | 8 -------- python/pylibraft/CMakeLists.txt | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index f1c703a5ae..a2b22a0c4e 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -137,14 +137,6 @@ if(RAFT_COMPILE_LIBRARIES) set(RAFT_COMPILE_NN_LIBRARY ON) endif() -if(distance IN_LIST raft_FIND_COMPONENTS) - set(RAFT_COMPILE_DIST_LIBRARY ON) -endif() - -if(nn IN_LIST raft_FIND_COMPONENTS) - set(RAFT_COMPILE_NN_LIBRARY ON) -endif() - if(RAFT_COMPILE_DIST_LIBRARY) set(RAFT_ENABLE_cuco_DEPENDENCY ON) endif() diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 5ca2c90ee2..874ec6462b 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -36,6 +36,14 @@ option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulti # If the user requested it we attempt to find RAFT. if(FIND_RAFT_CPP) find_package(raft ${pylibraft_version} REQUIRED COMPONENTS distance) + # TODO: I find it odd that the raft-config.cmake includes the corresponding + # component lib-target/dependency as OPTIONAL. This leads to the variables + # raft_distance_FOUND and raft_nn_FOUND to always be set to true, and it + # means that the find_package call won't fail if a user requests a + # component that hasn't been installed. Is that an error? + if(NOT TARGET raft::raft_distance_lib) + message(FATAL_ERROR "Building against a preexisting libraft library requires the distance component of that library to have been built!") + endif() else() set(raft_FOUND OFF) endif() From 36c26afc106a7527f5474c6c1fcde0f468e82ace Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Thu, 12 May 2022 16:05:55 -0700 Subject: [PATCH 34/37] Final cleanup, testing against new branch with RUNPATH set to ORIGIN. --- python/pylibraft/CMakeLists.txt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 874ec6462b..fe8aa5240b 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -16,7 +16,7 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) set(pylibraft_version 22.06.00) -file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake +file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/fix/cython_rpath_origin/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) @@ -36,11 +36,6 @@ option(FIND_RAFT_CPP "Search for existing RAFT C++ installations before defaulti # If the user requested it we attempt to find RAFT. if(FIND_RAFT_CPP) find_package(raft ${pylibraft_version} REQUIRED COMPONENTS distance) - # TODO: I find it odd that the raft-config.cmake includes the corresponding - # component lib-target/dependency as OPTIONAL. This leads to the variables - # raft_distance_FOUND and raft_nn_FOUND to always be set to true, and it - # means that the find_package call won't fail if a user requests a - # component that hasn't been installed. Is that an error? if(NOT TARGET raft::raft_distance_lib) message(FATAL_ERROR "Building against a preexisting libraft library requires the distance component of that library to have been built!") endif() @@ -57,6 +52,10 @@ if(NOT raft_FOUND) set(BUILD_BENCHMARKS OFF) set(RAFT_COMPILE_DIST_LIBRARY ON) add_subdirectory(../../cpp raft-cpp) + + # When building the C++ libraries from source we must copy + # libraft_distance.so alongside the pairwise_distance Cython library. + install(TARGETS raft_distance_lib DESTINATION pylibraft/distance) endif() include(rapids-cython) From c42bacd4be1eae9d3b4d2aa827dbcef466cb00b4 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 13 May 2022 12:10:18 -0700 Subject: [PATCH 35/37] Properly initialize architectures when building the C++ library as part of the Python lib. --- python/pylibraft/CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index fe8aa5240b..8b45430d97 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -47,7 +47,13 @@ if(NOT raft_FOUND) # TODO: This will not be necessary once we upgrade to CMake 3.22, which will # pull in the required languages for the C++ project even if this project # does not require those languges. + include(rapids-cuda) + rapids_cuda_init_architectures(pylibraft) enable_language(CUDA) + # Since pylibraft only enables CUDA optionally we need to manually include the file that + # rapids_cuda_init_architectures relies on `project` including. + include("${CMAKE_PROJECT_pylibraft_INCLUDE}") + set(BUILD_TESTS OFF) set(BUILD_BENCHMARKS OFF) set(RAFT_COMPILE_DIST_LIBRARY ON) From 5a286ab0254b6f430f24b497884d946b55f34166 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 13 May 2022 12:17:01 -0700 Subject: [PATCH 36/37] Point back to the main rapids-cmake branch. --- python/pylibraft/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/pylibraft/CMakeLists.txt b/python/pylibraft/CMakeLists.txt index 8b45430d97..030a017087 100644 --- a/python/pylibraft/CMakeLists.txt +++ b/python/pylibraft/CMakeLists.txt @@ -16,7 +16,7 @@ cmake_minimum_required(VERSION 3.20.1 FATAL_ERROR) set(pylibraft_version 22.06.00) -file(DOWNLOAD https://raw.githubusercontent.com/vyasr/rapids-cmake/fix/cython_rpath_origin/RAPIDS.cmake +file(DOWNLOAD https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-22.06/RAPIDS.cmake ${CMAKE_BINARY_DIR}/RAPIDS.cmake) include(${CMAKE_BINARY_DIR}/RAPIDS.cmake) From 94998e718c288b7d93f9bc9898bef28768a90876 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 13 May 2022 14:55:04 -0700 Subject: [PATCH 37/37] Update cpp/CMakeLists.txt --- cpp/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index a2b22a0c4e..0d565896d5 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -137,7 +137,7 @@ if(RAFT_COMPILE_LIBRARIES) set(RAFT_COMPILE_NN_LIBRARY ON) endif() -if(RAFT_COMPILE_DIST_LIBRARY) +if(RAFT_COMPILE_DIST_LIBRARY OR distance IN_LIST raft_FIND_COMPONENTS) set(RAFT_ENABLE_cuco_DEPENDENCY ON) endif()