Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cuspatial devcontainers #960

Merged
merged 26 commits into from
Mar 17, 2023

Conversation

trxcllnt
Copy link
Contributor

@trxcllnt trxcllnt commented Feb 28, 2023

Description

This PR adds some devcontainers to help simplify building the cuspatial C++ and Python libraries.

The devcontainers can be launched in one of two ways:

  1. By clicking the "Reopen in Container" button that VSCode shows when opening the repo (or by using the "Rebuild and Reopen in Container" command from the command palette):
    image
  2. By using the .devcontainer/launch.sh script, which launches new VSCode windows and instances of the devcontainer.

launch.sh takes two arguments, a mode and a package manager.

The mode argument determines how the devcontainer interacts with the files on the host:

  • isolated means the devcontainer doesn't mount the cuspatial source from the host, and is unique to this container instance. Use this mode if you want to launch two separate devcontainers that can each be checked out to two independent branches of cuspatial.
    • Be sure to push any commits you want to persist. Once this container is removed, any unpushed changes will be lost!
  • single means the devcontainer will mount the cuspatial source from the host, but install RMM and cuDF via the package manager. This is the default option.
  • unified means the devcontainer will mount rmm, cudf, and cuspatial sources from the host (and assumes RMM and cuDF are siblings to the cuspatial dir). In this mode, RMM and cuDF will not be installed, but the devcontainer will install the dependencies required to build all three.

The package manager argument can be either conda, or pip. This determines whether the devcontainer uses conda or pip to install the dependencies (the default is conda). pip is experimental/not working for normal dev, and is currently meant to aid in pip packaging work.

Examples:

# Launch a devcontainer that only mounts cuspatial and installs dependencies via conda
$ .devcontainer/launch.sh

# Launch a devcontainer that is isolated from changes on the host and installs dependencies via conda
$ .devcontainer/launch.sh isolated

# Launch a devcontainer that mounts rmm, cudf, and cuspatial from the host and installs dependencies via conda
$ .devcontainer/launch.sh unified conda

# Launch a devcontainer that is isolated from changes on the host and installs dependencies via pip
$ .devcontainer/launch.sh isolated pip

Inside the devcontainer, you can use tab-completion to explore the available commands.

# Clone one of the RAPIDS repositories into the container
$ clone-<lib_name>

# Configure the C++ CMake of <lib_name> (command accepts additional CMake arguments)
$ configure-<lib_name>-cpp

# Optionally configure the C++ CMake of <lib_name> and build the C++ (command accepts additional CMake arguments)
$ build-<lib_name>-cpp

# Optionally build the <lib_name> C++, then build the Python library (command accepts additional CMake arguments)
$ build-<lib_name>-python

You can, of course, use CMake, ninja, pip install -e . etc. directly too. The above commands are only meant as aids/shortcuts! They are simple shell scripts, and you can inspect their contents via e.g. code $(which configure-cuspatial-cpp).

All the code repos are in the $HOME dir, so cleaning is straightforward:

# Clean cuspatial C++
$ rm -rf ~/cuspatial/cpp/build

# Clean cuspatial Python
$ rm -rf ~/cuspatial/python/cuspatial/_skbuild

Checklist

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

@trxcllnt trxcllnt requested review from a team as code owners February 28, 2023 07:06
@trxcllnt trxcllnt requested review from harrism, zhangjianting and isVoid and removed request for zhangjianting February 28, 2023 07:06
@github-actions github-actions bot added cmake Related to CMake code or build configuration Python Related to Python code labels Feb 28, 2023
@trxcllnt trxcllnt added 3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 28, 2023
Copy link
Contributor

@isVoid isVoid left a comment

Choose a reason for hiding this comment

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

I worked with @trxcllnt and verified the 3 modes (single, unified and isolated) all work as expected. We noticed some issue with clangd not being able to find libcudf. Since it's rather minor, I'm approving this so that we can start enjoy the benefit of devcontainers!

@harrism
Copy link
Member

harrism commented Feb 28, 2023

This is very exciting. I would like to see documentation checked in with this PR. The good news is I think the description above would make a perfect readme / documentation for using the devcontainer. Can you add it to the PR, @trxcllnt ?

@github-actions github-actions bot added the libcuspatial Relates to the cuSpatial C++ library label Feb 28, 2023
ajschmidt8 added a commit to ajschmidt8/cuspatial that referenced this pull request Mar 1, 2023
This PR updates the `.gitignore` file to make the following changes:

- Removes `ops-codeowners` from being responsible for Dockerfiles. We don't need to be alerted about changes to Dockerfiles
- Ensures that we only get alerted about `conda/` directory changes from the root `conda/` directory (and not sub `conda/` directories like those added in rapidsai#960).

[skip ci]
ajschmidt8 added a commit that referenced this pull request Mar 1, 2023
This PR updates the `.gitignore` file to make the following changes:

- Removes `ops-codeowners` from being responsible for Dockerfiles. We don't need to be alerted about changes to Dockerfiles
- Ensures that we only get alerted about `conda/` directory changes from the root `conda/` directory (and not sub `conda/` directories like those added in #960).

I skipped CI on this PR since these changes aren't tested by CI.

This PR can be admin-merged pending approval.

Authors:
   - AJ Schmidt (https://github.com/ajschmidt8)

Approvers:
   - Mark Harris (https://github.com/harrism)
   - Ray Douglass (https://github.com/raydouglass)

[skip ci]
@github-actions github-actions bot added the ci label Mar 1, 2023
@trxcllnt
Copy link
Contributor Author

trxcllnt commented Mar 1, 2023

I have no idea what these test failures mean, esp. since this PR doesn't change any source code. It looks like something is corrupting the runner?

@ajschmidt8
Copy link
Member

I have no idea what these test failures mean, esp. since this PR doesn't change any source code. It looks like something is corrupting the runner?

@trxcllnt, for reference, we fixed a connectivity issue with our GPU-enabled self-hosted runners last week that was contributing to random timeouts and segfaults.

It's possible that the failures you linked were related to those connectivity issues.

.devcontainer/Dockerfile Show resolved Hide resolved
ci/release/update-version.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@thomcom thomcom left a comment

Choose a reason for hiding this comment

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

Working well over here.

@trxcllnt trxcllnt requested a review from a team as a code owner March 15, 2023 17:43
@github-actions github-actions bot added conda Related to conda and conda configuration Java Related to Java code and removed conda Related to conda and conda configuration labels Mar 15, 2023
@trxcllnt
Copy link
Contributor Author

@thomcom rebased

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this!

@trxcllnt trxcllnt requested a review from thomcom March 16, 2023 01:37
@isVoid isVoid requested a review from ajschmidt8 March 16, 2023 19:16
@rapidsai rapidsai deleted a comment from trxcllnt Mar 17, 2023
@thomcom
Copy link
Contributor

thomcom commented Mar 17, 2023

/merge

@thomcom thomcom requested review from ajschmidt8 and removed request for ajschmidt8 March 17, 2023 15:36
@trxcllnt trxcllnt removed the request for review from ajschmidt8 March 17, 2023 16:40
@trxcllnt
Copy link
Contributor Author

@gpucibot merge

@rapids-bot
Copy link

rapids-bot bot commented Mar 17, 2023

@trxcllnt, the @gpucibot merge command has been replaced with /merge.

Please re-comment this PR with /merge and use this new command in the future.

@trxcllnt
Copy link
Contributor Author

/merge

@thomcom
Copy link
Contributor

thomcom commented Mar 17, 2023

?

@rapids-bot rapids-bot bot merged commit 40897ef into rapidsai:branch-23.04 Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team ci cmake Related to CMake code or build configuration improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change Python Related to Python code
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

5 participants