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 initial devcontainers #155

Draft
wants to merge 4 commits into
base: branch-0.36
Choose a base branch
from

Conversation

msarahan
Copy link
Contributor

@msarahan msarahan commented Jan 9, 2024

This is working towards building wheels, but it has benefits for making development more unified with other RAPIDS projects.

At time of writing, this PR is not enough to get wheels working. In particular, there are some annoying issues to resolve:

  • dependencies.yaml does not account for CUDA-version-specific version names in package names, like rmm-cu12. That means the package names just don't resolve for wheel builds. This PR attempts to add that using the matrix mechanism.
  • rapids-dependency-file-generator doesn't allow the matrix mechanism for pyproject.toml outputs. Paul pointed me to feat: support matrices and stdout for all output types dependency-file-generator#48 as the feature that is needed to resolve this.

As a hacky workaround just to try to get a wheel built, I changed pyproject.toml manually for a cuda 12 build. This is part of the "DO NOT MERGE" commit. This change would affect or break the conda build, so it should not go in. This file needs to be generated during build, not committed with changes.

Even with the pyproject.toml changes, we have a cmake build error:

      CMake Error in ucxx/_lib/CMakeLists.txt:
        Imported target "ucxx::ucxx" includes non-existent path
      
          "/home/coder/ucxx/cpp/build/release/include"
      
        in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:
 
        * The path was deleted, renamed, or moved to another location.
        * An install or uninstall procedure did not complete successfully.
        * The installation package was faulty and references files it does not
        provide.

Paul says "Looks like it's added a path to the build dir in its public interface." I don't know what that means, but I think the next step is debugging in CMakeLists.txt.

.devcontainer/README.md Outdated Show resolved Hide resolved
.devcontainer/README.md Outdated Show resolved Hide resolved
.devcontainer/README.md Outdated Show resolved Hide resolved
.devcontainer/README.md Outdated Show resolved Hide resolved
msarahan and others added 2 commits January 10, 2024 10:58
@@ -1,7 +1,7 @@
# Dependency list for https://github.com/rapidsai/dependency-file-generator
files:
all:
output: conda
output: [conda, pyproject]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
output: [conda, pyproject]
output: conda

@@ -70,7 +70,7 @@ dependencies:
- fmt>=10.1.1,<11
- &gmock gmock>=1.13.0
- &gtest gtest>=1.13.0
- librmm==24.2.*
- librmm==24.02.*
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- librmm==24.02.*
- librmm==24.2.*

Comment on lines +96 to +97
# we need --pre for rmm 24.2, as all current versions are pre-release (alpha)
- --pre
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to handle this with something like cudf's use of alpha_spec in a script: https://github.com/rapidsai/cudf/blob/6abef4a4746f1f9917711f372726023efdc21e85/ci/build_wheel.sh#L28-L35

@@ -160,7 +182,7 @@ dependencies:
common:
- output_types: [conda, requirements, pyproject]
packages:
- numpy>=1.21
- numpy>=1.21,<1.25
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's diagnose the root cause for this upper bound before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants