cuSpatial: Build CUDA 12 packages #1044

isVoid · 2023-04-07T00:30:07Z

Description

This PR builds libcuspatial and cuspatial conda packages using CUDA 12. Closes #925.

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

bdice

Good start. Let’s minimize the dependencies. I don’t think cuSpatial requires all the same things as libcudf. I will revisit the math libraries question in a bit.

ci/build_cpp.sh

conda/recipes/libcuspatial/conda_build_config.yaml

conda/recipes/libcuspatial/meta.yaml

ajschmidt8 · 2023-04-07T17:31:10Z

Please use GitHub's Draft PR feature in the future. Draft PRs have the benefit of preventing notifications to codeowners until PRs are marked Ready for Review, which helps cut down on excessive notifications for PRs that are still being worked on. CI will still run on Draft PRs.

Some useful information about Draft PRs:

ajschmidt8 · 2023-04-07T17:31:18Z

Marking PR as draft.

bdice · 2023-04-07T19:15:16Z

I will revisit the math libraries question in a bit.

The math library dependency I was thinking of was cusparse, which was removed in this PR: #959. No math libraries are needed here, which simplifies things. 😄

conda/recipes/libcuspatial/meta.yaml

This reverts commit 362535d.

This reverts commit 4be8e48.

…into conda-cuda-12

…is finished.

bdice · 2023-06-29T14:05:47Z

CI is passing so I'm going to open this draft for review. I'll also start a review. 👍

bdice

I pushed a small change to leverage run_exports by putting -dev packages in the host section, so that they populate the run section implicitly. We adopted this approach in cudf because it plays better with the conda overlinking checker. Otherwise this looks good to me! I'd like a review from one other CUDA 12 expert before approving.

vyasr

This is fairly close, but a couple of things need resolving and perhaps a bit of discussion.

conda/recipes/cuspatial/meta.yaml

cpp/cmake/thirdparty/get_libcudacxx.cmake

cpp/cmake/thirdparty/get_thrust.cmake

dependencies.yaml

jakirkham

Thanks Michael and everyone reviewing! 🙏

Had a couple suggestions below

conda/recipes/libcuspatial/meta.yaml

jakirkham · 2023-06-29T20:34:48Z

conda/recipes/libcuspatial/meta.yaml

      ignore_run_exports_from:
-        - {{ compiler('cuda') }}
+        {% if cuda_major == "11" %}
+        - {{ compiler('cuda11') }}
+        {% endif %}


Noticed the run_exports of these compilers are ignored, but the compilers are not listed in build. Do we intend to have the compilers or is this section unneeded?

Probably unneeded. Removed and trying if CI passes.

conda/recipes/libcuspatial/meta.yaml

jakirkham · 2023-06-30T05:02:04Z

conda/recipes/libcuspatial/meta.yaml

-        - {{ compiler('cuda') }}
+        {% if cuda_major == "11" %}
+        - {{ compiler('cuda11') }}
+        {% endif %}
    requirements:
      build:


Should we have the compilers here?

Suggested change

build:

build:

- {{ compiler('c') }}

- {{ compiler('cxx') }}

{% if cuda_major == "11" %}

- {{ compiler('cuda11') }} ={{ cuda_version }}

{% else %}

- {{ compiler('cuda') }}

{% endif %}

- sysroot_{{ target_platform }} {{ sysroot_version }}

No, we have already finished compilation by the time we're packaging libcuspatial and libcuspatial-tests. Compilers are needed in the top-level build section for use by build.sh, but the individual package outputs only install (copy) files to the conda environment and do not perform compilation.

The only argument I could see for adding compilers would be to influence the host/run environments via run_exports but I think that is not necessary here (we haven't traditionally done this).

conda/recipes/libcuspatial/meta.yaml

conda/recipes/cuspatial/meta.yaml

jakirkham · 2023-06-30T05:10:11Z

conda/recipes/libcuspatial/meta.yaml

-        - {{ compiler('cuda') }}
+        {% if cuda_major == "11" %}
+        - {{ compiler('cuda11') }}
+        {% endif %}
    requirements:
      build:
        - cmake {{ cmake_version }}


Do we need ninja here too (as it is used elsewhere)?

Suggested change

- cmake {{ cmake_version }}

- cmake {{ cmake_version }}

- ninja

No, we do not need ninja here. Ninja should be in the top-level build but not the subpackages' build sections.

…efined

bdice

All looks good to me. Thanks @isVoid!

vyasr

LGTM

harrism · 2023-07-06T06:11:12Z

/merge

jakirkham · 2023-07-06T21:32:21Z

Discovered the cuspatial (Python) packages were not uploading for CUDA 12. This was due to the fact that the build/string of cuspatial needs the CUDA major version as well (similar to what is done for libcuspatial). Adding this in PR ( #1211 )

initial porting of rapidsai/cudf/12922

d6abd8d

isVoid requested a review from a team as a code owner April 7, 2023 00:30

github-actions bot added ci conda Related to conda and conda configuration labels Apr 7, 2023

isVoid self-assigned this Apr 7, 2023

jakirkham mentioned this pull request Apr 7, 2023

cuSpatial: CUDA 12 Conda Packages #925

Closed

bdice reviewed Apr 7, 2023

View reviewed changes

ci/build_cpp.sh Outdated Show resolved Hide resolved

conda/recipes/libcuspatial/conda_build_config.yaml Outdated Show resolved Hide resolved

conda/recipes/libcuspatial/meta.yaml Outdated Show resolved Hide resolved

bdice added the 5 - DO NOT MERGE Hold off on merging; see PR for details label Apr 7, 2023

isVoid changed the title ~~Build CUDA 12 packages of libcuspatial~~ cuSpatial: Build CUDA 12 packages Apr 7, 2023

ajschmidt8 marked this pull request as draft April 7, 2023 17:31

isVoid added 6 commits April 7, 2023 10:37

add libcudf channel

faaec0c

remove most cuda dependencies

27a4ae9

add libcupti

011f1b5

add cudart

9e22857

add cudart-dev

6b8e198

add cudart-static

b17dc52

isVoid and others added 2 commits April 7, 2023 12:16

add cuda-driver

26faa51

Use libcudf with CUDA major build string.

01b5c6b

bdice reviewed Apr 7, 2023

View reviewed changes

conda/recipes/libcuspatial/meta.yaml Outdated Show resolved Hide resolved

conda/recipes/libcuspatial/meta.yaml Outdated Show resolved Hide resolved

isVoid added 5 commits April 7, 2023 12:29

add cuda_driver_version

b878052

add cuda12 compiler for python build

362535d

add major_version

4be8e48

Revert "add cuda12 compiler for python build"

93db8af

This reverts commit 362535d.

Revert "add major_version"

7a34966

This reverts commit 4be8e48.

isVoid changed the base branch from branch-23.06 to branch-23.08 June 12, 2023 15:14

isVoid added 2 commits June 23, 2023 17:50

Merge branch 'branch-23.08' of https://github.com/rapidsai/cuspatial …

ded69e2

…into conda-cuda-12

Remove temporary pull artifacts now that rmm and cudf cuda12 bringup …

7e0e324

…is finished.

isVoid added 2 commits June 28, 2023 20:37

Revert, add ctk to cuda11 host environment

baee048

Change preset cuda_compiler name

5b6372d

bdice marked this pull request as ready for review June 29, 2023 14:05

bdice requested a review from a team as a code owner June 29, 2023 14:05

bdice requested a review from vyasr June 29, 2023 14:05

bdice removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Jun 29, 2023

Fix host/run sections using run_exports.

cbd9ac1

bdice reviewed Jun 29, 2023

View reviewed changes

vyasr requested changes Jun 29, 2023

View reviewed changes

bdice and others added 3 commits June 29, 2023 14:36

Only ignore CUDA compiler run_exports for CUDA 11.

d2a20f9

Attempt to remove custom patching for libcudacxx and thrust

2e7120a

update python dependencies with cuda12 pip packages

1688b39

jakirkham reviewed Jun 30, 2023

View reviewed changes

removing custom fetching for libcudacxx and thrust

1264b1f

github-actions bot removed the libcuspatial Relates to the cuSpatial C++ library label Jun 30, 2023

isVoid added 4 commits June 30, 2023 11:39

remove ctk where compiler already defines

0e55484

remove compiler run export ignore where compiler dependency was not d…

7e8363b

…efined

Remove ninja requirement in build

bff32db

Remove unused cuda_spec

cb3fb6f

isVoid requested a review from vyasr June 30, 2023 21:45

Remove run_export ignore for libcuspatial-test

b2a5d4a

bdice approved these changes Jun 30, 2023

View reviewed changes

vyasr approved these changes Jul 1, 2023

View reviewed changes

raydouglass approved these changes Jul 5, 2023

View reviewed changes

jakirkham approved these changes Jul 5, 2023

View reviewed changes

thomcom approved these changes Jul 5, 2023

View reviewed changes

rapids-bot bot merged commit feff9eb into rapidsai:branch-23.08 Jul 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cuSpatial: Build CUDA 12 packages #1044

cuSpatial: Build CUDA 12 packages #1044

isVoid commented Apr 7, 2023 •

edited by bdice

Loading

bdice left a comment

ajschmidt8 commented Apr 7, 2023

ajschmidt8 commented Apr 7, 2023

bdice commented Apr 7, 2023

bdice commented Jun 29, 2023

bdice left a comment

vyasr left a comment

jakirkham left a comment

jakirkham Jun 29, 2023

isVoid Jun 30, 2023

jakirkham Jun 30, 2023

bdice Jun 30, 2023 •

edited

Loading

jakirkham Jun 30, 2023

bdice Jun 30, 2023

bdice left a comment

vyasr left a comment

harrism commented Jul 6, 2023

jakirkham commented Jul 6, 2023

-      build:
+      build:
+        - {{ compiler('c') }}
+        - {{ compiler('cxx') }}
+        {% if cuda_major == "11" %}
+        - {{ compiler('cuda11') }} ={{ cuda_version }}
+        {% else %}
+        - {{ compiler('cuda') }}
+        {% endif %}
+        - sysroot_{{ target_platform }} {{ sysroot_version }}

	- cmake {{ cmake_version }}
	- cmake {{ cmake_version }}
	- ninja

cuSpatial: Build CUDA 12 packages #1044

cuSpatial: Build CUDA 12 packages #1044

Conversation

isVoid commented Apr 7, 2023 • edited by bdice Loading

Description

Checklist

bdice left a comment

Choose a reason for hiding this comment

ajschmidt8 commented Apr 7, 2023

ajschmidt8 commented Apr 7, 2023

bdice commented Apr 7, 2023

bdice commented Jun 29, 2023

bdice left a comment

Choose a reason for hiding this comment

vyasr left a comment

Choose a reason for hiding this comment

jakirkham left a comment

Choose a reason for hiding this comment

jakirkham Jun 29, 2023

Choose a reason for hiding this comment

isVoid Jun 30, 2023

Choose a reason for hiding this comment

jakirkham Jun 30, 2023

Choose a reason for hiding this comment

bdice Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

jakirkham Jun 30, 2023

Choose a reason for hiding this comment

bdice Jun 30, 2023

Choose a reason for hiding this comment

bdice left a comment

Choose a reason for hiding this comment

vyasr left a comment

Choose a reason for hiding this comment

harrism commented Jul 6, 2023

jakirkham commented Jul 6, 2023

isVoid commented Apr 7, 2023 •

edited by bdice

Loading

bdice Jun 30, 2023 •

edited

Loading