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

Migrate expressions to pylibcudf #16056

Merged
merged 11 commits into from
Jul 16, 2024
Merged

Migrate expressions to pylibcudf #16056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c256f1e
Migrate expressions to pylibcudf
lithomas1 Jun 18, 2024
a57b132
fix typo in docs
lithomas1 Jun 18, 2024
5bc6917
Merge branch 'branch-24.08' of github.com:rapidsai/cudf into pylibcud…
lithomas1 Jun 24, 2024
7c0d72d
add to init file
lithomas1 Jun 24, 2024
04213d1
Merge branch 'branch-24.08' of github.com:rapidsai/cudf into pylibcud…
lithomas1 Jul 3, 2024
2bc2fb8
Merge branch 'branch-24.08' of github.com:rapidsai/cudf into pylibcud…
lithomas1 Jul 12, 2024
0210fd2
sync parquet changes
lithomas1 Jul 15, 2024
6f3ae30
Merge branch 'branch-24.08' of github.com:rapidsai/cudf into pylibcud…
lithomas1 Jul 16, 2024
649454c
take in scalars instead
lithomas1 Jul 16, 2024
a6f305b
Apply suggestions from code review
lithomas1 Jul 16, 2024
2590f6e
Merge branch 'branch-24.08' into pylibcudf-expr
lithomas1 Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/cudf/source/user_guide/api_docs/pylibcudf/datetime.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
=======
copying
=======
========
datetime
========

.. automodule:: cudf._lib.pylibcudf.datetime
:members:
6 changes: 6 additions & 0 deletions docs/cudf/source/user_guide/api_docs/pylibcudf/expressions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
===========
expressions
===========

.. automodule:: cudf._lib.pylibcudf.expressions
:members:
1 change: 1 addition & 0 deletions docs/cudf/source/user_guide/api_docs/pylibcudf/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ This page provides API documentation for pylibcudf.
concatenate
copying
datetime
expressions
filling
gpumemoryview
groupby
Expand Down
1 change: 0 additions & 1 deletion python/cudf/cudf/_lib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ set(cython_sources
copying.pyx
csv.pyx
datetime.pyx
expressions.pyx
filling.pyx
groupby.pyx
hash.pyx
Expand Down
3 changes: 1 addition & 2 deletions python/cudf/cudf/_lib/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2020-2023, NVIDIA CORPORATION.
# Copyright (c) 2020-2024, NVIDIA CORPORATION.
import numpy as np

from . import (
Expand All @@ -8,7 +8,6 @@
copying,
csv,
datetime,
expressions,
filling,
groupby,
hash,
Expand Down
156 changes: 0 additions & 156 deletions python/cudf/cudf/_lib/expressions.pyx
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion python/cudf/cudf/_lib/parquet.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ cimport cudf._lib.pylibcudf.libcudf.io.data_sink as cudf_io_data_sink
cimport cudf._lib.pylibcudf.libcudf.io.types as cudf_io_types
cimport cudf._lib.pylibcudf.libcudf.types as cudf_types
from cudf._lib.column cimport Column
from cudf._lib.expressions cimport Expression
from cudf._lib.io.utils cimport (
make_sinks_info,
make_source_info,
update_struct_field_names,
)
from cudf._lib.pylibcudf.expressions cimport Expression
from cudf._lib.pylibcudf.io.datasource cimport NativeFileDatasource
from cudf._lib.pylibcudf.libcudf.expressions cimport expression
from cudf._lib.pylibcudf.libcudf.io.parquet cimport (
Expand Down
1 change: 1 addition & 0 deletions python/cudf/cudf/_lib/pylibcudf/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ set(cython_sources
concatenate.pyx
copying.pyx
datetime.pyx
expressions.pyx
filling.pyx
gpumemoryview.pyx
groupby.pyx
Expand Down
1 change: 1 addition & 0 deletions python/cudf/cudf/_lib/pylibcudf/__init__.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ from . cimport (
concatenate,
copying,
datetime,
expressions,
filling,
groupby,
join,
Expand Down
1 change: 1 addition & 0 deletions python/cudf/cudf/_lib/pylibcudf/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
concatenate,
copying,
datetime,
expressions,
filling,
groupby,
interop,
Expand Down
29 changes: 12 additions & 17 deletions python/cudf/cudf/_lib/expressions.pxd → .../cudf/cudf/_lib/pylibcudf/expressions.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,31 @@
# Copyright (c) 2022-2024, NVIDIA CORPORATION.

from libc.stdint cimport int32_t, int64_t
# Copyright (c) 2024, NVIDIA CORPORATION.
from libcpp.memory cimport unique_ptr
from libcpp.string cimport string

from cudf._lib.pylibcudf.libcudf.expressions cimport (
column_reference,
ast_operator,
expression,
literal,
operation,
)
from cudf._lib.pylibcudf.libcudf.scalar.scalar cimport (
numeric_scalar,
scalar,
string_scalar,
timestamp_scalar,
table_reference,
)

from .scalar cimport Scalar


cdef class Expression:
cdef unique_ptr[expression] c_obj


cdef class Literal(Expression):
cdef unique_ptr[scalar] c_scalar

# Hold on to input scalar so it doesn't get gc'ed
cdef Scalar scalar

cdef class ColumnReference(Expression):
pass


cdef class Operation(Expression):
pass
# Hold on to the input expressions so
# they don't get gc'ed
cdef Expression right
cdef Expression left
Copy link
Contributor Author

@lithomas1 lithomas1 Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There was a use-after-free here (in the old cudf/_lib/expressions.pyx) previously, but it didn't get caught I think because the inputs to Operations weren't gc'ed yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, if the C++ side takes a reference, we need to hold on to the object on the Python side, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup we do, that's a good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

The previous approach was "person constructing the expression must keep everything alive".


cdef class ColumnNameReference(Expression):
pass
Loading
Loading