-
Notifications
You must be signed in to change notification settings - Fork 917
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
Migrate round to pylibcudf #15863
Merged
Merged
Migrate round to pylibcudf #15863
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
9411162
Migrate round to pylibcudf
lithomas1 878f245
Add docs
lithomas1 ff6453a
Merge branch 'branch-24.08' into pylibcudf-round
lithomas1 ddae621
add round to __all__
lithomas1 4aedbc5
Update round.pxd
lithomas1 ac5caf8
Update python/cudf/cudf/_lib/pylibcudf/round.pyx
lithomas1 6a67133
update after feedback
lithomas1 38c7d4b
steal tests from Lawrence's PR
lithomas1 fcb60e9
fix docs
lithomas1 46aa296
remove duplicate import
lithomas1 26fc147
add type to enum
lithomas1 96312b8
Merge branch 'branch-24.08' of github.com:rapidsai/cudf into pylibcud…
lithomas1 4e530cf
swap assert order
lithomas1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
===== | ||
round | ||
===== | ||
|
||
.. automodule:: cudf._lib.pylibcudf.round | ||
:members: |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# Copyright (c) 2024, NVIDIA CORPORATION. | ||
from libc.stdint cimport int32_t | ||
|
||
from cudf._lib.pylibcudf.libcudf.round cimport rounding_method | ||
|
||
from .column cimport Column | ||
|
||
|
||
cpdef Column round( | ||
Column source, | ||
int32_t decimal_places = *, | ||
rounding_method round_method = * | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
# Copyright (c) 2024, NVIDIA CORPORATION. | ||
from libc.stdint cimport int32_t | ||
from libcpp.memory cimport unique_ptr | ||
from libcpp.utility cimport move | ||
|
||
from cudf._lib.pylibcudf.libcudf.round cimport ( | ||
round as cpp_round, | ||
rounding_method, | ||
) | ||
|
||
from cudf._lib.pylibcudf.libcudf.round import \ | ||
rounding_method as RoundingMethod # no-cython-lint | ||
|
||
from cudf._lib.pylibcudf.libcudf.column.column cimport column | ||
|
||
from .column cimport Column | ||
|
||
|
||
cpdef Column round( | ||
Column source, | ||
int32_t decimal_places = 0, | ||
rounding_method round_method = rounding_method.HALF_UP | ||
): | ||
"""Rounds all the values in a column to the specified number of decimal places. | ||
|
||
lithomas1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
For details, see :cpp:func:`round`. | ||
|
||
Parameters | ||
---------- | ||
source : Column | ||
The Column for which to round values. | ||
decimal_places: int32_t, optional | ||
The number of decimal places to round to (default 0) | ||
round_method: rounding_method, optional | ||
The method by which to round each value. | ||
Can be one of { RoundingMethod.HALF_UP, RoundingMethod.HALF_EVEN } | ||
(default rounding_method.HALF_UP) | ||
|
||
Returns | ||
------- | ||
pylibcudf.Column | ||
A Column with values rounded | ||
""" | ||
cdef unique_ptr[column] c_result | ||
with nogil: | ||
c_result = move( | ||
cpp_round( | ||
source.view(), | ||
decimal_places, | ||
round_method | ||
) | ||
) | ||
|
||
return Column.from_libcudf(move(c_result)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
# Copyright (c) 2024, NVIDIA CORPORATION. | ||
|
||
import pyarrow as pa | ||
import pytest | ||
from utils import assert_column_eq | ||
|
||
import cudf._lib.pylibcudf as plc | ||
|
||
|
||
@pytest.fixture(params=[False, True]) | ||
def nullable(request): | ||
return request.param | ||
|
||
|
||
@pytest.fixture(params=["float32", "float64"]) | ||
def column(request, nullable): | ||
values = [2.5, 2.49, 1.6, 8, -1.5, -1.7, -0.5, 0.5] | ||
typ = {"float32": pa.float32(), "float64": pa.float64()}[request.param] | ||
if nullable: | ||
values[2] = None | ||
return plc.interop.from_arrow(pa.array(values, type=typ)) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"round_mode", ["half_towards_infinity", "half_to_even"] | ||
) | ||
@pytest.mark.parametrize("decimals", [0, 1, 2, 5]) | ||
def test_round(column, round_mode, decimals): | ||
method = { | ||
"half_towards_infinity": plc.round.RoundingMethod.HALF_UP, | ||
"half_to_even": plc.round.RoundingMethod.HALF_EVEN, | ||
}[round_mode] | ||
got = plc.round.round(column, decimals, method) | ||
expect = pa.compute.round( | ||
plc.interop.to_arrow(column), decimals, round_mode | ||
) | ||
|
||
assert_column_eq(expect, got) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the strings after the enum values mean anything (e.g. "cudf::rounding_method::HALF_UP") or are they purely cosmetic?
(asking since I don't want to unintentionally change behavior by changing this here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strings are meaningful, but removing them is the right answer. The strings are a way to alias the names of Cython objects so that they appear differently in generated C code. IOW when you write
cdef rounding_method x = HALF_UP
in Cython code, that will translate to C code that saysenum rounding_method X = cudf::rounding_method::HALF_UP
. The reason that we previously need this in enums in our Cython is because prior to version 3.0 Cython did not support C++ scoped enumerations (enum class
), but all of our C++ enums are defined that way, so the alias was the only way to trick Cython into generating the code that we needed (with raw C enums you don't need to docudf::rounding_method::HALF_UP
, you can just docudf::HALF_UP
because the enum doesn't create it's own namespace). By switching to using anenum class
here we remove the need for that.