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] Upcast (but never downcast) the result of a binary operation between decimals (Python) #10282

Closed
shwina opened this issue Feb 14, 2022 · 1 comment · Fixed by #10299
Closed
Assignees
Labels
bug Something isn't working Python Affects Python cuDF API.

Comments

@shwina
Copy link
Contributor

shwina commented Feb 14, 2022

Currently, a binary operation between two decimal types can result in a downcast if the resulting precision can be accommodated by a smaller decimal type:

In [20]: s = cudf.Series([0, 1, 2], dtype=cudf.Decimal64Dtype(scale=2, precision=5))

In [21]: s
Out[21]:
0    0.00
1    1.00
2    2.00
dtype: decimal64

In [22]: s + s
Out[22]:
0    0.00
1    2.00
2    4.00
dtype: decimal32

This downcast may be inefficient, or at worst, dangerous, depending on downstream operations.

We should never downcast, but upcast if necessary, according to the rules summarized here.

@shwina shwina added bug Something isn't working Python Affects Python cuDF API. labels Feb 14, 2022
@galipremsagar galipremsagar self-assigned this Feb 14, 2022
@galipremsagar
Copy link
Contributor

@shwina Assigned it to me, I'll open a PR for these changes.

rapids-bot bot pushed a commit that referenced this issue Feb 23, 2022
Fixes: #10282 

This PR removes decimal type narrowing and also updates the tests accordingly.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

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

URL: #10299
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants