rapidsai / cudf

cuDF - GPU DataFrame Library
https://docs.rapids.ai/api/cudf/stable/
Apache License 2.0
8.23k stars 883 forks source link

[BUG] Converting from numeric types to fixed point types does not handle underflow/overflow #15884

Open ttnghia opened 3 months ago

ttnghia commented 3 months ago

When converting floating point types to fixed point types, the casting operations are just blindly scaling the input values and then casting to integer without checking whether the scaled values are too big to be stored in the target integer types.

Probably a more proper way to do casting here is to check for underflow/overflow and nullify the output accordingly.

pmattione-nvidia commented 3 months ago

what if we do fixed-point -> float, and we exceed e.g. DBL_MAX. Do we want null here too, or return a double with value infinity?

davidwendt commented 3 months ago

Creating nulls for overflow is not something we do in general for other operations (e.g. casting float to integer). Usually nulls identify unvalid input rows and are not used to communicate any operation exceptions in this way. Just wanted to point this out. Perhaps an exception is needed for fixed-point.

ttnghia commented 3 months ago

I agree with that. Yes, nullifying the output rows may not be a generic solution to fix all cases. A more comprehensive solution is probably to return a boolean value after nullifying the output rows indicating whether such replacements have happened.

FYI, in Spark, we either choose to nullify the output rows or throw an exception depending on what the user wants (through a config value).

pmattione-nvidia commented 3 months ago

When we say exception here, do we mean throw a normal C++ exception? Can we do that from kernel code? Do we even want to?

For float -> fixed-point, the conversion could return a std::optional, which is std::nullopt_t if the value overflowed. Upstream code (e.g. the kernel) can then decide what to do with this (Null or exception).

ttnghia commented 3 months ago

What I do in such cases is nullify the output row, and also set a boolean flag (which will be returned) indicating that such overflow occurred during conversion.