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

Jump to bottom

Setup pylibcudf package #16299

Merged
merged 56 commits into from
Aug 16, 2024
Merged

Setup pylibcudf package #16299

merged 56 commits into from
Aug 16, 2024

Conversation

lithomas1
Copy link
Contributor

@lithomas1 lithomas1 commented Jul 17, 2024 •
edited
Loading

Description

Migrates cudf._lib.pylibcudf to a new pylibcudf package

Checklist

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

TODO

  • Make existing packages work with the new pylibcudf package @Matt711
    • Make sure cudf/cudf-polars uses the new pylibcudf package correctly
    • Check if anyone else uses pylibcudf (On top of my head, I believe cudf_kafka imports pylibcudf, but we should check everything)
    • Add versioning to pylibcudf package (e.g. __version__)
  • Add CI/scripts to build the new pylibcudf package @lithomas1
  • Make conda packages for pylibcudf @Matt711

@lithomas1 lithomas1 added feature request New feature or request non-breaking Non-breaking change labels Jul 17, 2024
@github-actions github-actions bot added Python Affects Python cuDF API. CMake CMake build issue pylibcudf Issues specific to the pylibcudf package labels Jul 17, 2024
@Matt711 Matt711 changed the base branch from branch-24.08 to branch-24.10 July 25, 2024 16:20
@github-actions github-actions bot added libcudf Affects libcudf (C++/CUDA) code. Java Affects Java cuDF API. cudf.pandas Issues specific to cudf.pandas cudf.polars Issues specific to cudf.polars labels Jul 31, 2024
@github-actions github-actions bot removed libcudf Affects libcudf (C++/CUDA) code. Java Affects Java cuDF API. cudf.pandas Issues specific to cudf.pandas cudf.polars Issues specific to cudf.polars labels Jul 31, 2024
vyasr
vyasr requested changes Aug 15, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

Overall looks great! A few small things I noticed.

.github/workflows/pr.yaml Show resolved Hide resolved
conda/environments/all_cuda-118_arch-x86_64.yaml Outdated Show resolved Hide resolved
python/cudf/cudf/_lib/aggregation.pyx Outdated Show resolved Hide resolved
python/pylibcudf/cmake/Modules/LinkPyarrowHeaders.cmake Outdated Show resolved Hide resolved
python/pylibcudf/pylibcudf/__init__.py Show resolved Hide resolved
python/pylibcudf/pylibcudf/exception_handler.pxd Outdated Show resolved Hide resolved
python/pylibcudf/pylibcudf/libcudf/exception_handler.pxd Outdated Show resolved Hide resolved
vyasr
vyasr approved these changes Aug 16, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, I won the race 😂 #16548 merged first, so now there are conflicts here that you'll need to resolve. Once you handle that, this PR is good to go in my view. You're also blocked until #16576 merges.

jameslamb
jameslamb approved these changes Aug 16, 2024
View reviewed changes
Copy link
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

There are still some pip install instances in scripts that I think should be combined, but let's not hold up this PR any longer for those. I can address them in #16575 after this is merged.

I have no other comments, thanks!

vyasr
vyasr reviewed Aug 16, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

Download logic needs a small fix

ci/test_wheel_cudf.sh Outdated Show resolved Hide resolved
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Aug 16, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@vyasr
Copy link
Contributor

vyasr commented Aug 16, 2024

/ok to test

This was referenced Aug 16, 2024
Merged
Merged
vyasr
vyasr reviewed Aug 16, 2024
View reviewed changes
ci/test_wheel_cudf.sh Outdated Show resolved Hide resolved
@vyasr
Copy link
Contributor

vyasr commented Aug 16, 2024

/ok to test

@vyasr
Copy link
Contributor

vyasr commented Aug 16, 2024

All right, we have tests passing here so I'm going to go ahead and merge the devcontainers PR and rerun the devcontainer tests.

@vyasr
Copy link
Contributor

vyasr commented Aug 16, 2024

/merge

@rapids-bot rapids-bot bot merged commit bc8ca9b into rapidsai:branch-24.10 Aug 16, 2024
83 checks passed
@lithomas1 lithomas1 deleted the setup-pylibcudf-package branch August 16, 2024 20:53
This was referenced Aug 16, 2024
Merged
Merged
Closed
rapids-bot bot pushed a commit that referenced this pull request Aug 17, 2024
@vyasr
Add build job for pylibcudf (#16587)
b63ba70 
This was missed in #16299 and is necessary to get builds published.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: #16587
@jameslamb jameslamb mentioned this pull request Aug 19, 2024
Merged
3 tasks
rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this pull request Aug 19, 2024
@vyasr
Update cuspatial to use pylibcudf (#1440)
c60bd4d 
This is necessary for compatibility with cudf after rapidsai/cudf#16299.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Bradley Dice (https://github.com/bdice)

URL: #1440
rapids-bot bot pushed a commit that referenced this pull request Aug 19, 2024
@jameslamb
standardize and consolidate wheel installations in testing scripts (#…
6ccc2c2 
…16575)

I noticed some common changes to wheel-testing scripts in the PRs splitting off `pylibcudf` (#16299) and `libcudf` (#15483).

* consolidating multiple `pip install`'s into 1
  - *(this is safer, as it removes the risk of `pip` replacing a previously-installed CI package with another one from a remote package repository)*
* standardizing the approach used for "install some wheels built earlier in this same CI run"

These can go onto `branch-24.10` right now, so proposing them in a separate PR so that `cudf` CI can benefit from them without having to wait on those large PRs.

Authors:
  - James Lamb (https://github.com/jameslamb)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Bradley Dice (https://github.com/bdice)

URL: #16575
rapids-bot bot pushed a commit that referenced this pull request Aug 19, 2024
@jameslamb
make more use of YAML anchors in dependencies.yaml (#16597)
f2d13c9 
Contributes to rapidsai/build-planning#33

Follow-up to #16299

This proposes some simplifications to `dependencies.yaml`. It's not intended to change any behavior.

* more use of YAML anchors for requirements that are intended to be identical to each other
* eliminating the `pylibcudf_build_dep` dependency group that was introduced in #16299, in favor of just tracking the `pylibcudf` build dependency alongside `cudf`'s `rmm` build dependency in the existing `build_python_cudf` group
  - *(sorry I'd missed that in the review on #16299)*

I found myself starting to make similar changes in the PR breaking up these packages into more (splitting out a `libcudf` in #15483) and thought they'd be better as a standalone PR.

Authors:
  - James Lamb (https://github.com/jameslamb)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: #16597
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vyasr vyasr vyasr approved these changes

@Matt711 Matt711 Matt711 left review comments

@jameslamb jameslamb jameslamb approved these changes

@galipremsagar galipremsagar Awaiting requested review from galipremsagar galipremsagar is a code owner automatically assigned from rapidsai/cudf-python-codeowners

Assignees

@Matt711 Matt711

@lithomas1 lithomas1

Labels
CMake CMake build issue cudf.pandas Issues specific to cudf.pandas cudf.polars Issues specific to cudf.polars feature request New feature or request non-breaking Non-breaking change pylibcudf Issues specific to the pylibcudf package Python Affects Python cuDF API.
Projects
cuDF/Dask/Numba/UCX
Status: Done
pylibcudf
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lithomas1 @Matt711 @vyasr @jameslamb