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

[BUG] copying::shift doesn't handle abs(offset) > column.size() #10314

Closed
isVoid opened this issue Feb 17, 2022 · 1 comment · Fixed by #10414
Closed

[BUG] copying::shift doesn't handle abs(offset) > column.size() #10314

isVoid opened this issue Feb 17, 2022 · 1 comment · Fixed by #10414
Assignees
Labels
bug Something isn't working libcudf Affects libcudf (C++/CUDA) code.

Comments

@isVoid
Copy link
Contributor

isVoid commented Feb 17, 2022

Describe the bug
When offset is greater than the size of the column, libcudf will fail with not user-friendly error messages.

Steps/Code to reproduce bug

In [7]: import cudf

In [8]: s = cudf.DataFrame({"a": [1, 2, 3, 4]})

In [10]: s.shift(5)
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
Input In [10], in <module>
----> 1 s.shift(5)

...
File ~/cudf/python/cudf/cudf/core/frame.py:1610, in <genexpr>(.0)
   1606 if freq is not None:
   1607     raise ValueError("The freq argument is not yet supported.")
   1609 data_columns = (
-> 1610     col.shift(periods, fill_value) for col in self._columns
   1611 )
   1612 return self.__class__._from_data(
   1613     zip(self._column_names, data_columns), self._index
   1614 )

File ~/cudf/python/cudf/cudf/core/column/column.py:338, in ColumnBase.shift(self, offset, fill_value)
    337 def shift(self, offset: int, fill_value: ScalarLike) -> ColumnBase:
--> 338     return libcudf.copying.shift(self, offset, fill_value)

File ~/cudf/python/cudf/cudf/_lib/copying.pyx:631, in cudf._lib.copying.shift()

RuntimeError: parallel_for failed: cudaErrorInvalidConfiguration: invalid configuration **argument**

Expected behavior
Propose to match segmented_shift behavior in returning an all-null column if fill_value is unspecified or return a column filled with fill_value if otherwise.

Environment overview (please complete the following information)

  • Environment location: [compose]
  • Method of cuDF install: [compose]
@isVoid isVoid added bug Something isn't working Needs Triage Need team to review and classify libcudf Affects libcudf (C++/CUDA) code. labels Feb 17, 2022
@rapids-bot rapids-bot bot closed this as completed in f5ec4b2 Feb 17, 2022
@isVoid
Copy link
Contributor Author

isVoid commented Feb 17, 2022

Also unexpectedly closed by comments of #9805 (comment)

@isVoid isVoid reopened this Feb 17, 2022
@davidwendt davidwendt self-assigned this Mar 11, 2022
rapids-bot bot pushed a commit that referenced this issue Mar 17, 2022
Closes #10314 

Fixes logic to handle `abs(offset) > input.size()` when passed to `cudf::shift`.  As mentioned in #10314 this was causing an unexpected exception:
```
C++ exception with description "parallel_for failed: cudaErrorInvalidConfiguration: invalid configuration argument" thrown in the test body.
```
The behavior now fills the entire output column with the input scalar value. If the scalar is null, then the column is filled with null entries. The logic added here did not require changing or adding any new kernel functions. Additional gtests were added to `shift_tests.cpp` as well.

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - MithunR (https://github.com/mythrocks)
  - Yunsong Wang (https://github.com/PointKernel)

URL: #10414
@bdice bdice removed the Needs Triage Need team to review and classify label Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants