Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[BUG] test_cast_decimal_to_decimal[to:DecimalType(1,-1)-from:Decimal(5,-3)] fails with DATAGEN_SEED=1702439569 #10050

Closed
NVnavkumar opened this issue Dec 14, 2023 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@NVnavkumar
Copy link
Collaborator

NVnavkumar commented Dec 14, 2023

Describe the bug

FAILED ../../src/main/python/cast_test.py::test_cast_decimal_to_decimal[to:DecimalType(1,-1)-from:Decimal(5,-3)][DATAGEN_SEED=1702439569] - AssertionError: GPU and CPU are not both null at [994, 'a']

This was originally detected in the scala 2.13 pre-merge check against Spark 3.3.0, but I was also able to reproduce against Spark 3.1.1 with Scala 2.12 using the same seed value.

@NVnavkumar
Copy link
Collaborator Author

NVnavkumar commented Dec 14, 2023

Failure also showed up in Databricks 11.3 nightly IT run, so very likely this affects all shims

@andygrove
Copy link
Contributor

Input is -1E+3. CPU produces None and GPU produces Decimal('-1.00E+3').

-Row(a=None, a=Decimal('-1E+3'))
+Row(a=Decimal('-1.00E+3'), a=Decimal('-1E+3'))

@mattahrens mattahrens removed the ? - Needs Triage Need team to review and classify label Dec 18, 2023
@razajafri
Copy link
Collaborator

We are returning what cudf is returning

scala> val vec = ColumnVector.fromDecimals(new java.math.BigDecimal("-1E+3"))
scala> val casted = vec.castTo(DType.create(DType.DTypeEnum.DECIMAL32, 1))
casted: ai.rapids.cudf.ColumnVector = ColumnVector{rows=1, type=DECIMAL32 scale:1, nullCount=Optional.empty, offHeap=(ID: 88 7f36afa50c60)}
scala> println(casted.copyToHost.getBigDecimal(0))
-1.00E+3

This makes it obvious that there is a part of code where we are determining if the value is out of bounds, that part is broken. I believe that is the checkNFixDecimalBounds method here
Spark thinks that we are overflowing but cudf thinks we don’t I would look at this part of the code and just copy it on our side to make it compatible.

@razajafri razajafri added the ? - Needs Triage Need team to review and classify label Dec 20, 2023
@andygrove andygrove removed their assignment Dec 20, 2023
@razajafri razajafri self-assigned this Dec 20, 2023
@razajafri razajafri removed the ? - Needs Triage Need team to review and classify label Dec 20, 2023
@razajafri
Copy link
Collaborator

razajafri commented Dec 20, 2023

I know what's happening. The new precision required for correct representation (3) is greater than the target precision (1).
Cudf doesn't have the concept of precision so this is not a problem there but Spark doesn't allow exceeding the precision

I will put up a PR for this

@razajafri
Copy link
Collaborator

razajafri commented Jan 10, 2024

Will be fixed by rapidsai/cudf#14731

@razajafri
Copy link
Collaborator

Will test tomorrow then close

@razajafri
Copy link
Collaborator

Will have to hold off testing this until jni nightly gets kicked off again as it doesn't have the changes from rapidsai/cudf#14731

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants