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

Use pylibcudf contiguous split APIs in cudf python #17246

Merged
merged 17 commits into from
Nov 16, 2024
Merged

Use pylibcudf contiguous split APIs in cudf python #17246

merged 17 commits into from
Nov 16, 2024

Conversation

Matt711
Copy link
Contributor

@Matt711 Matt711 commented Nov 5, 2024 •
edited
Loading

Description

Apart of #15162

Checklist

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

@Matt711 Matt711 added feature request New feature or request non-breaking Non-breaking change labels Nov 5, 2024
@Matt711 Matt711 self-assigned this Nov 5, 2024
@github-actions github-actions bot added the Python Affects Python cuDF API. label Nov 5, 2024
@Matt711 Matt711 mentioned this pull request Nov 5, 2024
Closed
3 tasks
@github-actions github-actions bot added the pylibcudf Issues specific to the pylibcudf package label Nov 7, 2024
@Matt711 Matt711 changed the title Migrate contiguous split APIs to pylibcudf Use pylibcudf contiguous split APIs in cudf python Nov 13, 2024
@Matt711 Matt711 marked this pull request as ready for review November 13, 2024 01:43
@Matt711 Matt711 requested a review from a team as a code owner November 13, 2024 01:43
@Matt711 Matt711 requested review from wence- and vyasr November 13, 2024 01:43
wence-
wence- requested changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

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

I think we can simplify this a lot by using the fact that pylibcudf's PackedColumns has a release method that provides a memoryview for the metadata and a gpumemoryview for the data.

python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
python/pylibcudf/pylibcudf/contiguous_split.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
@Matt711 Matt711 requested a review from wence- November 14, 2024 22:20
Matt711
Matt711 commented Nov 14, 2024
View reviewed changes
Copy link
Contributor Author

@Matt711 Matt711 left a comment

Choose a reason for hiding this comment

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

simplify

python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
wence-
wence- approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

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

Small suggestion to avoid needing setstate/getstate on HostBuffer, otherwise looks good, thanks.

python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
python/pylibcudf/pylibcudf/contiguous_split.pxd Outdated Show resolved Hide resolved
python/pylibcudf/pylibcudf/contiguous_split.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/_lib/copying.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/_lib/copying.pyx Show resolved Hide resolved
python/pylibcudf/pylibcudf/contiguous_split.pyx Outdated Show resolved Hide resolved
@Matt711
Copy link
Contributor Author

Matt711 commented Nov 15, 2024

/merge

@rapids-bot rapids-bot bot merged commit 9cc9071 into rapidsai:branch-24.12 Nov 16, 2024
125 checks passed
@mroeschke mroeschke mentioned this pull request Dec 7, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wence- wence- wence- approved these changes

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

Assignees

@Matt711 Matt711

Labels
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 Python
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Matt711 @wence-