Skip to content

Commit

Permalink
make pylibcudf depend on libcudf, use '_libcudf' instead of '_cpp'
Browse files Browse the repository at this point in the history
  • Loading branch information
jameslamb committed Aug 19, 2024
1 parent 78af8e9 commit b33f421
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 76 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ jobs:
branch: ${{ inputs.branch }}
sha: ${{ inputs.sha }}
date: ${{ inputs.date }}
script: ci/build_wheel_cpp.sh
script: ci/build_wheel_libcudf.sh
wheel-publish-pylibcudf:
needs: wheel-build-libcudf
secrets: inherit
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ jobs:
arch: "amd64"
container_image: "rapidsai/ci-conda:latest"
run_script: "ci/build_docs.sh"
wheel-build-cpp:
wheel-build-libcudf:
needs: checks
secrets: inherit
uses: rapidsai/shared-workflows/.github/workflows/[email protected]
with:
matrix_filter: group_by([.ARCH, (.CUDA_VER|split(".")|map(tonumber)|.[0])]) | map(max_by(.PY_VER|split(".")|map(tonumber)))
build_type: pull-request
script: "ci/build_wheel_cpp.sh"
script: "ci/build_wheel_libcudf.sh"
wheel-build-pylibcudf:
needs: checks
secrets: inherit
Expand All @@ -138,7 +138,7 @@ jobs:
build_type: pull-request
script: "ci/build_wheel_pylibcudf.sh"
wheel-build-cudf:
needs: [checks, wheel-build-cpp]
needs: [checks, wheel-build-libcudf]
secrets: inherit
uses: rapidsai/shared-workflows/.github/workflows/[email protected]
with:
Expand Down
File renamed without changes.
15 changes: 15 additions & 0 deletions ci/build_wheel_libcudf.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/bash
# Copyright (c) 2023-2024, NVIDIA CORPORATION.

set -euo pipefail

package_dir="python/libcudf"

./ci/build_wheel.sh ${package_dir}

RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"

mkdir -p ${package_dir}/final_dist
python -m auditwheel repair --exclude libarrow.so.1601 -w ${package_dir}/final_dist ${package_dir}/dist/*

RAPIDS_PY_WHEEL_NAME="libcudf_${RAPIDS_PY_CUDA_SUFFIX}" rapids-upload-wheels-to-s3 cpp ${package_dir}/final_dist
16 changes: 11 additions & 5 deletions dependencies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,14 @@ files:
includes:
- test_python_common
- test_python_cudf
py_rapids_build_pylibcudf:
py_build_pylibcudf:
output: pyproject
pyproject_dir: python/pylibcudf
extras:
table: build-system
includes:
- rapids_build_skbuild
py_build_pylibcudf:
py_rapids_build_pylibcudf:
output: pyproject
pyproject_dir: python/pylibcudf
extras:
Expand Down Expand Up @@ -392,6 +392,7 @@ dependencies:
common:
- output_types: conda
packages:
- &libcudf_unsuffixed libcudf==24.10.*,>=0.0.0a0
- &rmm_unsuffixed rmm==24.10.*,>=0.0.0a0
- output_types: requirements
packages:
Expand All @@ -406,13 +407,15 @@ dependencies:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- &libcudf_cu12 libcudf-cu12==24.10.*,>=0.0.0a0
- rmm-cu12==24.10.*,>=0.0.0a0
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- &libcudf_cu11 libcudf-cu11==24.10.*,>=0.0.0a0
- rmm-cu11==24.10.*,>=0.0.0a0
- {matrix: null, packages: [*rmm_unsuffixed]}
- {matrix: null, packages: [*libcudf_unsuffixed, *rmm_unsuffixed]}
build_python_cudf:
common:
- output_types: conda
Expand Down Expand Up @@ -492,7 +495,7 @@ dependencies:
common:
- output_types: [conda]
packages:
- &libcudf_unsuffixed libcudf==24.10.*,>=0.0.0a0
- *libcudf_unsuffixed
specific:
- output_types: [pyproject]
matrices:
Expand Down Expand Up @@ -696,6 +699,7 @@ dependencies:
- typing_extensions>=4.0.0
- output_types: conda
packages:
- *libcudf_unsuffixed
- *rmm_unsuffixed
- output_types: requirements
packages:
Expand All @@ -719,13 +723,15 @@ dependencies:
cuda: "12.*"
cuda_suffixed: "true"
packages:
- *libcudf_cu12
- rmm-cu12==24.10.*,>=0.0.0a0
- matrix:
cuda: "11.*"
cuda_suffixed: "true"
packages:
- *libcudf_cu11
- rmm-cu11==24.10.*,>=0.0.0a0
- {matrix: null, packages: [*rmm_unsuffixed]}
- {matrix: null, packages: [*libcudf_unsuffixed, *rmm_unsuffixed]}
run_cudf:
common:
- output_types: [conda, requirements, pyproject]
Expand Down
2 changes: 1 addition & 1 deletion python/cudf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ endif()
# are installed to. Since there are multiple subpackages of cudf._lib that require access to
# libcudf, we place the library and all its dependent artifacts in the cudf directory as a single
# source of truth.
include(cmake/Modules/WheelHelpers.cmake)
include(../pylibcudf/cmake/Modules/WheelHelpers.cmake)

# TODO: This install is currently overzealous. We should only install the libraries that are
# downloaded by CPM during the build, not libraries that were found on the system. However, in
Expand Down
9 changes: 5 additions & 4 deletions python/libcudf/libcudf/load.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@ def load_library():
except OSError:
# If neither of these directories contain the library, we assume we are in an
# environment where the C++ library is already installed somewhere else and the
# CMake build of the libcudf Python package was a no-op. Note that this approach
# won't work for real editable installs of the libcudf package, but that's not a
# use case I think we need to support. scikit-build-core has limited support for
# importlib.resources so there isn't a clean way to support that case yet.
# CMake build of the libcudf Python package was a no-op.
#
# Note that this approach won't work for real editable installs of the libcudf package.
# scikit-build-core has limited support for importlib.resources so there isn't a clean
# way to support that case yet.
for lib_dir in ("lib", "lib64"):
if os.path.isfile(
lib := os.path.join(
Expand Down
68 changes: 6 additions & 62 deletions python/pylibcudf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,72 +24,16 @@ project(
LANGUAGES CXX CUDA
)

option(FIND_CUDF_CPP "Search for existing CUDF C++ installations before defaulting to local files"
OFF
)
option(USE_LIBARROW_FROM_PYARROW "Only use the libarrow contained in pyarrow" OFF)
mark_as_advanced(USE_LIBARROW_FROM_PYARROW)

# Find Python early so that later commands can use it
find_package(Python 3.9 REQUIRED COMPONENTS Interpreter)

# If the user requested it we attempt to find CUDF.
if(FIND_CUDF_CPP)
include(rapids-cpm)
include(rapids-export)
include(rapids-find)
rapids_cpm_init()

if(USE_LIBARROW_FROM_PYARROW)
# We need to find arrow before libcudf since libcudf requires it but doesn't bundle arrow
# libraries. These variables have no effect because we are always searching for arrow via
# pyarrow, but they must be set as they are required arguments to the function in
# get_arrow.cmake.
set(CUDF_USE_ARROW_STATIC OFF)
set(CUDF_ENABLE_ARROW_S3 OFF)
set(CUDF_ENABLE_ARROW_ORC OFF)
set(CUDF_ENABLE_ARROW_PYTHON OFF)
set(CUDF_ENABLE_ARROW_PARQUET OFF)
include(../../cpp/cmake/thirdparty/get_arrow.cmake)
endif()

find_package(cudf "${RAPIDS_VERSION}" REQUIRED)
find_package(cudf "${RAPIDS_VERSION}" REQUIRED)

# an installed version of libcudf doesn't provide the dlpack headers so we need to download dlpack
# for the interop.pyx
include(../../cpp/cmake/thirdparty/get_dlpack.cmake)
else()
set(cudf_FOUND OFF)
endif()
# an installed version of libcudf doesn't provide the dlpack headers so we need to download dlpack
# for the interop.pyx
include(rapids-cpm)
rapids_cpm_init()
include(../../cpp/cmake/thirdparty/get_dlpack.cmake)

include(rapids-cython-core)

if(NOT cudf_FOUND)
set(BUILD_TESTS OFF)
set(BUILD_BENCHMARKS OFF)
set(CUDF_BUILD_TESTUTIL OFF)
set(CUDF_BUILD_STREAMS_TEST_UTIL OFF)
set(CUDA_STATIC_RUNTIME ON)

add_subdirectory(../../cpp cudf-cpp EXCLUDE_FROM_ALL)

# libcudf targets are excluded by default above via EXCLUDE_FROM_ALL to remove extraneous
# components like headers from libcudacxx, but we do need the libraries. However, we want to
# control where they are installed to. Since there are multiple subpackages of pylibcudf that
# require access to libcudf, we place the library and all its dependent artifacts in the cudf
# directory as a single source of truth and modify the other rpaths appropriately.
set(cython_lib_dir pylibcudf)
include(cmake/Modules/WheelHelpers.cmake)
# TODO: This install is currently overzealous. We should only install the libraries that are
# downloaded by CPM during the build, not libraries that were found on the system. However, in
# practice right this would only be a problem is if libcudf was not found but some of the
# dependencies were, and we have no real use cases where that happens.
install_aliased_imported_targets(
TARGETS cudf arrow_shared nvcomp::nvcomp nvcomp::nvcomp_gdeflate nvcomp::nvcomp_bitcomp
DESTINATION ${cython_lib_dir}
)
endif()

rapids_cython_init()

include(cmake/Modules/LinkPyarrowHeaders.cmake)
Expand Down
10 changes: 10 additions & 0 deletions python/pylibcudf/pylibcudf/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Copyright (c) 2023-2024, NVIDIA CORPORATION.

# If libcudf was installed as a wheel, we must request it to load the library symbols.
# Otherwise, we assume that the library was installed in a system path that ld can find.
try:
import libcudf
except ModuleNotFoundError:
pass
else:
libcudf.load_library()
del libcudf

from . import (
aggregation,
binaryop,
Expand Down
2 changes: 2 additions & 0 deletions python/pylibcudf/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ license = { text = "Apache 2.0" }
requires-python = ">=3.9"
dependencies = [
"cuda-python>=11.7.1,<12.0a0",
"libcudf==24.10.*,>=0.0.0a0",
"nvtx>=0.2.1",
"packaging",
"pyarrow>=16.1.0,<16.2.0a0",
Expand Down Expand Up @@ -101,6 +102,7 @@ matrix-entry = "cuda_suffixed=true"
requires = [
"cmake>=3.26.4,!=3.30.0",
"cython>=3.0.3",
"libcudf==24.10.*,>=0.0.0a0",
"ninja",
"numpy==1.23.*",
"pyarrow==16.1.0.*",
Expand Down

0 comments on commit b33f421

Please sign in to comment.