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.

Sign up for GitHub

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

Re-enable cucim and xgboost in CUDA 12 rapids builds. #669

Merged
merged 17 commits into from
Jul 27, 2023
Merged

Re-enable cucim and xgboost in CUDA 12 rapids builds. #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7da6367
Re-enable cucim and xgboost in CUDA 12 rapids builds.
bdice Jul 25, 2023
921f35e
Update xgboost pinnings.
bdice Jul 26, 2023
f4660ee
Merge remote-tracking branch 'upstream/branch-23.08' into enable-cuda…
bdice Jul 26, 2023
9417308
Enable testing of conda packages.
bdice Jul 26, 2023
d1aab9c
Fix cuda_version to cuda-version.
bdice Jul 26, 2023
8b16f6b
Update conda/recipes/rapids-xgboost/meta.yaml
bdice Jul 26, 2023
e9a1ff8
Update conda/recipes/rapids-xgboost/meta.yaml
bdice Jul 26, 2023
8e32ca7
Skip tests for rapids.
bdice Jul 26, 2023
9a1ab9d
Merge branch 'enable-cuda-12-cucim-xgboost' of github.com:bdice/integ…
bdice Jul 26, 2023
1b21545
Enable --use-local.
bdice Jul 26, 2023
e7ebe5d
Use CONDA_OVERRIDE_CUDA.
bdice Jul 26, 2023
328f27f
Merge branch 'enable-cuda-12-cucim-xgboost' of github.com:bdice/integ…
bdice Jul 26, 2023
264fa10
Update ci/build_python.sh
bdice Jul 26, 2023
6b0f77b
Update conda/recipes/rapids/meta.yaml
bdice Jul 26, 2023
2d55e20
Update conda/recipes/rapids-xgboost/meta.yaml
bdice Jul 26, 2023
a2bcb83
Stop testing imports but ensure an environment is created
vyasr Jul 26, 2023
3743b2b
Update conda/recipes/rapids-xgboost/meta.yaml
bdice Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ci/build_python.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,21 @@ set -euo pipefail
source rapids-env-update

CONDA_CONFIG_FILE="conda/recipes/versions.yaml"
export CONDA_OVERRIDE_CUDA="${RAPIDS_CUDA_VERSION}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably consider the implications here. Today, I think rapids is installable even on CPU-only machines. The new rapids-xgboost package design requires __cuda to install. This is important to support for cases like HPC systems with CPU login nodes and GPU worker nodes that use the same environment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah they can also install by setting CONDA_OVERRIDE_CUDA to some value

In any event, this is coming from libxgboost. So we could move this just to the rapids-xgboost if we prefer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new rapids-xgboost package design

What is the relevant new change? Is it in libxgboost or in something about how rapids-xgboost is packaged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Well, maybe this requirement already existed. I'm not sure.

The question is whether __cuda should be a hard requirement for installation, which is coming from xgboost-related packages. I'm not sure if it was that way for the old xgboost packages we shipped in 23.06 or not. Regardless, it feels funny that no other RAPIDS package has this requirement besides xgboost.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping this in PR ( #673 ), which pulls in the new xgboost packages


rapids-print-env

rapids-logger "Build rapids-xgboost"

bdice marked this conversation as resolved.
Show resolved Hide resolved
rapids-mamba-retry mambabuild \
--no-test \
--use-local \
--variant-config-files "${CONDA_CONFIG_FILE}" \
bdice marked this conversation as resolved.
Show resolved Hide resolved
conda/recipes/rapids-xgboost

rapids-logger "Build rapids"

rapids-mamba-retry mambabuild \
--no-test \
--use-local \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just want to mention that for other repos, we use --channel "${RAPIDS_CONDA_BLD_OUTPUT_DIR}": https://github.com/rapidsai/cudf/blob/abb59c83128f956c7edcb4d7744cb0faecf0026c/ci/build_python.sh#L18-L39

RAPIDS_CONDA_BLD_OUTPUT_DIR is set in our CI images: https://github.com/rapidsai/ci-imgs/blob/75cad918c44c6e00480001b24cb764e1b43fa0a5/Dockerfile#L108-L113

It looks like --use-local works, I'm just pointing this out in case we want consistency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah --use-local will check the same things. Please see this list of paths that Conda checks when --use-local is set

--variant-config-files "${CONDA_CONFIG_FILE}" \
conda/recipes/rapids

Expand Down
10 changes: 8 additions & 2 deletions conda/recipes/rapids-xgboost/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,19 @@ requirements:
- python
- cuda-version ={{ cuda_version }}
run:
- {{ pin_compatible('cuda_version', max_pin='x', min_pin='x') }}
- {{ pin_compatible('cuda-version', max_pin='x', min_pin='x') }}
{% if cuda_major == "11" %}
- cudatoolkit
{% endif %}
- nccl {{ nccl_version }}
- python
- xgboost {{ xgboost_version }}{{ major_minor_version }}
- xgboost {{ xgboost_version }} rapidsai_cuda*

bdice marked this conversation as resolved.
Show resolved Hide resolved
test:
requires:
- cuda-version ={{ cuda_version }}
bdice marked this conversation as resolved.
Show resolved Hide resolved
commands:
- exit 0

about:
home: https://rapids.ai/
Expand Down
30 changes: 5 additions & 25 deletions conda/recipes/rapids/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,12 @@ requirements:
- cudf ={{ major_minor_version }}.*
- cugraph ={{ major_minor_version }}.*
- cuml ={{ major_minor_version }}.*
{% if cuda_major == "11" %}
# Temporarily disabled on CUDA 12 until
# https://github.com/rapidsai/cucim/issues/513 is complete
- cucim ={{ major_minor_version }}.*
{% endif %}
- cuspatial ={{ major_minor_version }}.*
- custreamz ={{ major_minor_version }}.*
- cuxfilter ={{ major_minor_version }}.*
- dask-cuda ={{ major_minor_version }}.*
{% if cuda_major == "11" %}
# Temporarily disabled on CUDA 12 until
# https://github.com/rapidsai/xgboost-feedstock/issues/4 is complete
- rapids-xgboost ={{ major_minor_version }}.*
{% endif %}
- rmm ={{ major_minor_version }}.*
- pylibcugraph ={{ major_minor_version }}.*
- libcugraph_etl ={{ major_minor_version }}.*
Expand All @@ -69,23 +61,11 @@ requirements:
- conda-forge::ucx-proc=*=gpu
- conda-forge::ucx {{ ucx_version }}

test: # [linux64]
imports: # [linux64]
- cucim # [linux64]
- cudf # [linux64]
- cudf_kafka # [linux64]
- cugraph # [linux64]
- cuml # [linux64]
{% if cuda_major == "11" %}
- cusignal # [linux64]
{% endif %}
- cuspatial # [linux64]
- custreamz # [linux64]
- cuxfilter # [linux64]
- dask_cuda # [linux64]
- dask_cudf # [linux64]
- pylibcugraph # [linux64]
- rmm # [linux64]
Comment on lines -73 to -88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could run these through pkgutil.find_loader in a run_test.py script in the recipe

This would let us test for their existence without needing to import them (and thus not need a GPU to test)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary for this PR, maybe file an issue or PR with this proposal later on. I feel comfortable with the current level of testing, which is higher than what we had before.

test:
bdice marked this conversation as resolved.
Show resolved Hide resolved
requires:
- cuda-version ={{ cuda_version }}
commands:
- exit 0

about:
home: https://rapids.ai/
Expand Down
2 changes: 1 addition & 1 deletion conda/recipes/versions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Versions for `rapids-xgboost` meta-pkg
xgboost_version:
# Minor version is appended in meta.yaml
- '=1.7.5dev.rapidsai'
- '=1.7.4'
bdice marked this conversation as resolved.
Show resolved Hide resolved

cuda11_cuda_python_version:
- '>=11.7.1,<12.0a'
Expand Down