-
Notifications
You must be signed in to change notification settings - Fork 242
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
Fallback to CPU for mod only for DB11.3 [Databricks] #7609
Fallback to CPU for mod only for DB11.3 [Databricks] #7609
Conversation
Signed-off-by: Raza Jafri <[email protected]>
@@ -383,6 +383,7 @@ def test_mod_pmod_by_zero_not_ansi(data_gen): | |||
@pytest.mark.parametrize('rhs', [byte_gen, short_gen, int_gen, long_gen, DecimalGen(6, 3), | |||
DecimalGen(10, -2), DecimalGen(15, 3), DecimalGen(30, 12), DecimalGen(3, -3), | |||
DecimalGen(27, 7), DecimalGen(20, -3)], ids=idfn) | |||
@pytest.mark.skipif(is_databricks113_or_later(), reason='https://github.com/NVIDIA/spark-rapids/issues/7595') |
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.
It would be great if we could at least have a test that verifies that we fall back to the CPU on decimal values for this version of spark.
Even better if we check that it still works for other mixed data types, but I think that is minor if we have mod in general covered for the other non-mixed types.
@@ -383,6 +383,7 @@ def test_mod_pmod_by_zero_not_ansi(data_gen): | |||
@pytest.mark.parametrize('rhs', [byte_gen, short_gen, int_gen, long_gen, DecimalGen(6, 3), | |||
DecimalGen(10, -2), DecimalGen(15, 3), DecimalGen(30, 12), DecimalGen(3, -3), | |||
DecimalGen(27, 7), DecimalGen(20, -3)], ids=idfn) | |||
@pytest.mark.skipif(is_databricks113_or_later(), reason='https://github.com/NVIDIA/spark-rapids/issues/7595') |
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.
Doesn't this check need to also include Spark 3.4?
Signed-off-by: Raza Jafri <[email protected]>
Signed-off-by: Raza Jafri <[email protected]>
build |
Please add mod to the allow list for your fallback test and you need to take a look at |
pushed update but need to run sanity tests on db |
Can you also fix the style problems?
|
I have fixed this as well. |
build |
This PR makes change to the DB11.3 shim to disallow GpuRemainder to run on the GPU for DecimalTypes and skips the tests for testing mod on mixed decimal types
fixes #7595
Signed-off-by: Raza Jafri [email protected]