From dc281645f68adcaef40ee638f3cc779d3a6859e4 Mon Sep 17 00:00:00 2001 From: Matthew Murray Date: Tue, 5 May 2026 06:38:58 -0700 Subject: [PATCH 1/7] Fix assertion failures in assert_tpch_result_equal due to float sort ambiguity --- .../experimental/benchmarks/asserts.py | 65 ++++++++++++++----- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py index e8c80e480cd5..f57976cb2def 100644 --- a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py +++ b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py @@ -250,15 +250,17 @@ def assert_tpch_result_equal( details={"error": str(e)}, ) from e - # We know that each dataframe is sorted on `sort_by` according to itself. - # Now we have some freedom to reorder the rows. We'll use this freedom to avoid - # any kind of sorting on floating-point columns, which introduces all sorts of - # fuzziness we don't want to deal with. + # Sort by non-float columns first to avoid floating-point fuzziness. non_float_columns = [ col for col in left.columns if left.schema[col] not in (pl.Float32, pl.Float64) ] + float_columns = [ + col for col in left.columns if left.schema[col] in (pl.Float32, pl.Float64) + ] + # if non-float sort leaves ambiguity, use float as tie-breaker + all_sort_columns = non_float_columns + float_columns left_sorted = left.sort(by=non_float_columns, nulls_last=nulls_last) right_sorted = right.sort(by=non_float_columns, nulls_last=nulls_last) @@ -269,10 +271,17 @@ def assert_tpch_result_equal( right_sorted, **polars_kwargs, # type: ignore[arg-type] ) - except AssertionError as e: - raise ValidationError( - message="Result mismatch", details={"error": str(e)} - ) from e + except AssertionError: + try: + polars.testing.assert_frame_equal( + left.sort(by=all_sort_columns, nulls_last=nulls_last), + right.sort(by=all_sort_columns, nulls_last=nulls_last), + **polars_kwargs, # type: ignore[arg-type] + ) + except AssertionError as e2: + raise ValidationError( + message="Result mismatch", details={"error": str(e2)} + ) from e2 else: # Handle the .sort_by(...).head(n) case; First, split the data into two parts @@ -329,11 +338,19 @@ def assert_tpch_result_equal( expected_first.sort(by=non_float_columns, nulls_last=nulls_last), **polars_kwargs, # type: ignore[arg-type] ) - except AssertionError as e: - raise ValidationError( - message="Result mismatch in non-ties part", - details={"error": str(e)}, - ) from e + except AssertionError: + # Non-float sort left ambiguous ties; retry with float columns as secondary key + try: + polars.testing.assert_frame_equal( + result_first.sort(by=all_sort_columns, nulls_last=nulls_last), + expected_first.sort(by=all_sort_columns, nulls_last=nulls_last), + **polars_kwargs, # type: ignore[arg-type] + ) + except AssertionError as e2: + raise ValidationError( + message="Result mismatch in non-ties part", + details={"error": str(e2)}, + ) from e2 # We already know that the lengths match (we've validated that the # *total* lengths match and the non-ties lengths match, so this rump @@ -352,11 +369,23 @@ def assert_tpch_result_equal( ), **polars_kwargs, # type: ignore[arg-type] ) - except AssertionError as e: - raise ValidationError( - message="Result mismatch in ties part", - details={"error": str(e)}, - ) from e + except AssertionError: + # Non-float sort left ambiguous ties; retry with float columns + try: + polars.testing.assert_frame_equal( + result_ties.sort( + all_sort_columns, nulls_last=nulls_last + ).select(by), + expected_ties.sort( + all_sort_columns, nulls_last=nulls_last + ).select(by), + **polars_kwargs, # type: ignore[arg-type] + ) + except AssertionError as e2: + raise ValidationError( + message="Result mismatch in ties part", + details={"error": str(e2)}, + ) from e2 else: # no sort_by, just a straight comparison. From c270bb190a379b95965bad65edaed44b283361c2 Mon Sep 17 00:00:00 2001 From: Matthew Murray Date: Tue, 5 May 2026 06:41:38 -0700 Subject: [PATCH 2/7] remove comment --- .../cudf_polars/cudf_polars/experimental/benchmarks/asserts.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py index f57976cb2def..34eda03f20e0 100644 --- a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py +++ b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py @@ -250,7 +250,6 @@ def assert_tpch_result_equal( details={"error": str(e)}, ) from e - # Sort by non-float columns first to avoid floating-point fuzziness. non_float_columns = [ col for col in left.columns From c6719926d722aa2c60da73f19ce5e53bc385ed38 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 12 May 2026 05:51:55 -0700 Subject: [PATCH 3/7] Revert "remove comment" This reverts commit c270bb190a379b95965bad65edaed44b283361c2. --- .../cudf_polars/cudf_polars/experimental/benchmarks/asserts.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py index 34eda03f20e0..f57976cb2def 100644 --- a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py +++ b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py @@ -250,6 +250,7 @@ def assert_tpch_result_equal( details={"error": str(e)}, ) from e + # Sort by non-float columns first to avoid floating-point fuzziness. non_float_columns = [ col for col in left.columns From 3c7fda9640d70e9538476a0ff6fdb3335c3770d5 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 12 May 2026 05:52:01 -0700 Subject: [PATCH 4/7] Revert "Fix assertion failures in assert_tpch_result_equal due to float sort ambiguity" This reverts commit dc281645f68adcaef40ee638f3cc779d3a6859e4. --- .../experimental/benchmarks/asserts.py | 65 +++++-------------- 1 file changed, 18 insertions(+), 47 deletions(-) diff --git a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py index f57976cb2def..e8c80e480cd5 100644 --- a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py +++ b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py @@ -250,17 +250,15 @@ def assert_tpch_result_equal( details={"error": str(e)}, ) from e - # Sort by non-float columns first to avoid floating-point fuzziness. + # We know that each dataframe is sorted on `sort_by` according to itself. + # Now we have some freedom to reorder the rows. We'll use this freedom to avoid + # any kind of sorting on floating-point columns, which introduces all sorts of + # fuzziness we don't want to deal with. non_float_columns = [ col for col in left.columns if left.schema[col] not in (pl.Float32, pl.Float64) ] - float_columns = [ - col for col in left.columns if left.schema[col] in (pl.Float32, pl.Float64) - ] - # if non-float sort leaves ambiguity, use float as tie-breaker - all_sort_columns = non_float_columns + float_columns left_sorted = left.sort(by=non_float_columns, nulls_last=nulls_last) right_sorted = right.sort(by=non_float_columns, nulls_last=nulls_last) @@ -271,17 +269,10 @@ def assert_tpch_result_equal( right_sorted, **polars_kwargs, # type: ignore[arg-type] ) - except AssertionError: - try: - polars.testing.assert_frame_equal( - left.sort(by=all_sort_columns, nulls_last=nulls_last), - right.sort(by=all_sort_columns, nulls_last=nulls_last), - **polars_kwargs, # type: ignore[arg-type] - ) - except AssertionError as e2: - raise ValidationError( - message="Result mismatch", details={"error": str(e2)} - ) from e2 + except AssertionError as e: + raise ValidationError( + message="Result mismatch", details={"error": str(e)} + ) from e else: # Handle the .sort_by(...).head(n) case; First, split the data into two parts @@ -338,19 +329,11 @@ def assert_tpch_result_equal( expected_first.sort(by=non_float_columns, nulls_last=nulls_last), **polars_kwargs, # type: ignore[arg-type] ) - except AssertionError: - # Non-float sort left ambiguous ties; retry with float columns as secondary key - try: - polars.testing.assert_frame_equal( - result_first.sort(by=all_sort_columns, nulls_last=nulls_last), - expected_first.sort(by=all_sort_columns, nulls_last=nulls_last), - **polars_kwargs, # type: ignore[arg-type] - ) - except AssertionError as e2: - raise ValidationError( - message="Result mismatch in non-ties part", - details={"error": str(e2)}, - ) from e2 + except AssertionError as e: + raise ValidationError( + message="Result mismatch in non-ties part", + details={"error": str(e)}, + ) from e # We already know that the lengths match (we've validated that the # *total* lengths match and the non-ties lengths match, so this rump @@ -369,23 +352,11 @@ def assert_tpch_result_equal( ), **polars_kwargs, # type: ignore[arg-type] ) - except AssertionError: - # Non-float sort left ambiguous ties; retry with float columns - try: - polars.testing.assert_frame_equal( - result_ties.sort( - all_sort_columns, nulls_last=nulls_last - ).select(by), - expected_ties.sort( - all_sort_columns, nulls_last=nulls_last - ).select(by), - **polars_kwargs, # type: ignore[arg-type] - ) - except AssertionError as e2: - raise ValidationError( - message="Result mismatch in ties part", - details={"error": str(e2)}, - ) from e2 + except AssertionError as e: + raise ValidationError( + message="Result mismatch in ties part", + details={"error": str(e)}, + ) from e else: # no sort_by, just a straight comparison. From 65827dc64f58b0e1fe0d4e94c1d49305b0219900 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 12 May 2026 05:57:33 -0700 Subject: [PATCH 5/7] Update floating-point handling Always sort by non-float columns, but do it after sorting by float columns. --- .../experimental/benchmarks/asserts.py | 64 ++++++++++++++----- .../cudf_polars/tests/testing/test_asserts.py | 32 ++++++++++ 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py index e8c80e480cd5..fc7b1a658af2 100644 --- a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py +++ b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py @@ -252,15 +252,33 @@ def assert_tpch_result_equal( # We know that each dataframe is sorted on `sort_by` according to itself. # Now we have some freedom to reorder the rows. We'll use this freedom to avoid - # any kind of sorting on floating-point columns, which introduces all sorts of - # fuzziness we don't want to deal with. + # any kind of fuzziness from sorting on floating-point columns. + # + # As long as we sort by the non-float columns *first*, we'll avoid any + # false positives / false negatives from comparing two tables that have the + # same values but happen to be in a different order. Sorting by floating-point + # columns *last* ensures that records that are close to each other appear in + # (roughly) the same order, such that polar's approximate equality checks + # will allow them to be considered equal (or not, if the aren't actually close). non_float_columns = [ col for col in left.columns if left.schema[col] not in (pl.Float32, pl.Float64) ] - left_sorted = left.sort(by=non_float_columns, nulls_last=nulls_last) - right_sorted = right.sort(by=non_float_columns, nulls_last=nulls_last) + float_columns = [ + col for col in left.columns if left.schema[col] in (pl.Float32, pl.Float64) + ] + grouped_sort_columns = [*non_float_columns, *float_columns] + + def sort_for_comparison(df: pl.DataFrame) -> pl.DataFrame: + return ( + df.sort(by=grouped_sort_columns, nulls_last=nulls_last) + if grouped_sort_columns + else df + ) + + left_sorted = sort_for_comparison(left) + right_sorted = sort_for_comparison(right) if limit is None or left.is_empty(): try: @@ -325,8 +343,8 @@ def assert_tpch_result_equal( try: polars.testing.assert_frame_equal( - result_first.sort(by=non_float_columns, nulls_last=nulls_last), - expected_first.sort(by=non_float_columns, nulls_last=nulls_last), + sort_for_comparison(result_first), + sort_for_comparison(expected_first), **polars_kwargs, # type: ignore[arg-type] ) except AssertionError as e: @@ -344,12 +362,8 @@ def assert_tpch_result_equal( try: polars.testing.assert_frame_equal( - result_ties.sort(non_float_columns, nulls_last=nulls_last).select( - by - ), - expected_ties.sort(non_float_columns, nulls_last=nulls_last).select( - by - ), + sort_for_comparison(result_ties).select(by), + sort_for_comparison(expected_ties).select(by), **polars_kwargs, # type: ignore[arg-type] ) except AssertionError as e: @@ -359,11 +373,31 @@ def assert_tpch_result_equal( ) from e else: - # no sort_by, just a straight comparison. + non_float_columns = [ + col + for col in left.columns + if left.schema[col] not in (pl.Float32, pl.Float64) + ] + float_columns = [ + col for col in left.columns if left.schema[col] in (pl.Float32, pl.Float64) + ] + grouped_sort_columns = [*non_float_columns, *float_columns] + left_sorted = ( + left.sort(by=grouped_sort_columns, nulls_last=nulls_last) + if grouped_sort_columns + else left + ) + right_sorted = ( + right.sort(by=grouped_sort_columns, nulls_last=nulls_last) + if grouped_sort_columns + else right + ) + + # no sort_by, compare after grouped sort to ignore nondeterministic row order. try: polars.testing.assert_frame_equal( - left, - right, + left_sorted, + right_sorted, **polars_kwargs, # type: ignore[arg-type] ) except AssertionError as e: diff --git a/python/cudf_polars/tests/testing/test_asserts.py b/python/cudf_polars/tests/testing/test_asserts.py index ee5279181f1e..d1ca3e0ba5de 100644 --- a/python/cudf_polars/tests/testing/test_asserts.py +++ b/python/cudf_polars/tests/testing/test_asserts.py @@ -474,3 +474,35 @@ def test_assert_tpch_result_equal_sort_keys_raises_not_sorted() -> None: sort_keys=sort_keys, nulls_last=True, ) + + +@pytest.mark.parametrize("sort_by", [[("a", True)], []]) +@pytest.mark.parametrize("drop_columns", [[], ["b"], ["a", "b"]]) +def test_assert_tpch_result_equal_grouped_float_sort( + sort_by: list[tuple[str, bool]], drop_columns: list[str] +) -> None: + # https://github.com/rapidsai/cudf/issues/22129 + # Same non-float values with float values reordered inside each non-float group. + left = pl.DataFrame({"a": [1, 1, 1], "b": [2, 2, 2], "c": [1.0, 2.0, 3.0]}) + right = pl.DataFrame({"a": [1, 1, 1], "b": [2, 2, 2], "c": [1.0, 2.999, 2.0]}) + + if drop_columns: + left = left.drop(drop_columns) + right = right.drop(drop_columns) + if "a" in drop_columns: + sort_by = [] + + assert_tpch_result_equal( + left, right, sort_by=sort_by, abs_tol=0.01, check_exact=False + ) + + # But this table is different, since row 3.0 - 2.9 > abs_tol. + right_different = pl.DataFrame( + {"a": [1, 1, 1], "b": [2, 2, 2], "c": [1.0, 2.90, 2.0]} + ) + if drop_columns: + right_different = right_different.drop(drop_columns) + with pytest.raises(ValidationError, match="Result mismatch"): + assert_tpch_result_equal( + left, right_different, sort_by=sort_by, abs_tol=0.01, check_exact=False + ) From 75a5e6090348e9befdc7a18209f3c2120aa938bd Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 12 May 2026 07:06:41 -0700 Subject: [PATCH 6/7] Deduplicate sort handling --- .../experimental/benchmarks/asserts.py | 73 +++++++------------ 1 file changed, 27 insertions(+), 46 deletions(-) diff --git a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py index 6be48b3652db..7f1d1d86b4fa 100644 --- a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py +++ b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py @@ -207,6 +207,31 @@ def assert_tpch_result_equal( right = right.with_columns(*float_casts) left = left.with_columns(*float_casts) + non_float_columns = [ + col for col in left.columns if left.schema[col] not in (pl.Float32, pl.Float64) + ] + float_columns = [ + col for col in left.columns if left.schema[col] in (pl.Float32, pl.Float64) + ] + grouped_sort_columns = [*non_float_columns, *float_columns] + + def sort_for_comparison(df: pl.DataFrame) -> pl.DataFrame: + # We know that each dataframe is sorted on `sort_by` according to itself. + # Now we have some freedom to reorder the rows. We'll use this freedom to avoid + # any kind of fuzziness from sorting on floating-point columns. + # + # As long as we sort by the non-float columns *first*, we'll avoid any + # false positives / false negatives from comparing two tables that have the + # same values but happen to be in a different order. Sorting by floating-point + # columns *last* ensures that records that are close to each other appear in + # (roughly) the same order, such that polar's approximate equality checks + # will allow them to be considered equal (or not, if the aren't actually close). + return ( + df.sort(by=grouped_sort_columns, nulls_last=nulls_last) + if grouped_sort_columns + else df + ) + if sort_by: by, descending = list(zip(*sort_by, strict=True)) @@ -245,33 +270,6 @@ def assert_tpch_result_equal( details={"error": str(e)}, ) from e - # We know that each dataframe is sorted on `sort_by` according to itself. - # Now we have some freedom to reorder the rows. We'll use this freedom to avoid - # any kind of fuzziness from sorting on floating-point columns. - # - # As long as we sort by the non-float columns *first*, we'll avoid any - # false positives / false negatives from comparing two tables that have the - # same values but happen to be in a different order. Sorting by floating-point - # columns *last* ensures that records that are close to each other appear in - # (roughly) the same order, such that polar's approximate equality checks - # will allow them to be considered equal (or not, if the aren't actually close). - non_float_columns = [ - col - for col in left.columns - if left.schema[col] not in (pl.Float32, pl.Float64) - ] - float_columns = [ - col for col in left.columns if left.schema[col] in (pl.Float32, pl.Float64) - ] - grouped_sort_columns = [*non_float_columns, *float_columns] - - def sort_for_comparison(df: pl.DataFrame) -> pl.DataFrame: - return ( - df.sort(by=grouped_sort_columns, nulls_last=nulls_last) - if grouped_sort_columns - else df - ) - left_sorted = sort_for_comparison(left) right_sorted = sort_for_comparison(right) @@ -368,25 +366,8 @@ def sort_for_comparison(df: pl.DataFrame) -> pl.DataFrame: ) from e else: - non_float_columns = [ - col - for col in left.columns - if left.schema[col] not in (pl.Float32, pl.Float64) - ] - float_columns = [ - col for col in left.columns if left.schema[col] in (pl.Float32, pl.Float64) - ] - grouped_sort_columns = [*non_float_columns, *float_columns] - left_sorted = ( - left.sort(by=grouped_sort_columns, nulls_last=nulls_last) - if grouped_sort_columns - else left - ) - right_sorted = ( - right.sort(by=grouped_sort_columns, nulls_last=nulls_last) - if grouped_sort_columns - else right - ) + left_sorted = sort_for_comparison(left) + right_sorted = sort_for_comparison(right) # no sort_by, compare after grouped sort to ignore nondeterministic row order. try: From 43d46e241a91adfca2dd6d3d1a1f84288474a89e Mon Sep 17 00:00:00 2001 From: Matthew Murray Date: Tue, 12 May 2026 17:26:24 +0000 Subject: [PATCH 7/7] address review --- .../experimental/benchmarks/asserts.py | 13 +++++--- .../cudf_polars/tests/testing/test_asserts.py | 30 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py index 7f1d1d86b4fa..772e30175f5b 100644 --- a/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py +++ b/python/cudf_polars/cudf_polars/experimental/benchmarks/asserts.py @@ -226,9 +226,14 @@ def sort_for_comparison(df: pl.DataFrame) -> pl.DataFrame: # columns *last* ensures that records that are close to each other appear in # (roughly) the same order, such that polar's approximate equality checks # will allow them to be considered equal (or not, if the aren't actually close). + # + # Sort keys are intersected with df.schema so callers can pre-project + # the frame (e.g. to compare only `sort_by` columns) without payload + # columns influencing the order. + local_sort_columns = [c for c in grouped_sort_columns if c in df.schema] return ( - df.sort(by=grouped_sort_columns, nulls_last=nulls_last) - if grouped_sort_columns + df.sort(by=local_sort_columns, nulls_last=nulls_last) + if local_sort_columns else df ) @@ -355,8 +360,8 @@ def sort_for_comparison(df: pl.DataFrame) -> pl.DataFrame: try: polars.testing.assert_frame_equal( - sort_for_comparison(result_ties).select(by), - sort_for_comparison(expected_ties).select(by), + sort_for_comparison(result_ties.select(by)), + sort_for_comparison(expected_ties.select(by)), **polars_kwargs, # type: ignore[arg-type] ) except AssertionError as e: diff --git a/python/cudf_polars/tests/testing/test_asserts.py b/python/cudf_polars/tests/testing/test_asserts.py index d1ca3e0ba5de..d58b5af502c0 100644 --- a/python/cudf_polars/tests/testing/test_asserts.py +++ b/python/cudf_polars/tests/testing/test_asserts.py @@ -205,6 +205,36 @@ def test_assert_tpch_result_equal_ties_multi_column_sort_by() -> None: ) +def test_assert_tpch_result_equal_ties_payload_does_not_drive_order() -> None: + # Within the ties partition, payload column values must not determine + # the row order used to compare the sort_by columns. Both sides have + # the same set of v values in the tolerance band around the split + # point, but the payload column k is aligned with v in opposite orders. + # Sorting the full ties frame by k before projecting to v would put the + # v values in opposite orders on the two sides and fail the + # approximate comparison even though the result is correct. + left = pl.DataFrame( + { + "v": [1.0, 2.0, 3.099, 3.100, 3.101], + "k": ["a", "b", "x", "y", "z"], + } + ) + right = pl.DataFrame( + { + "v": [1.0, 2.0, 3.099, 3.100, 3.101], + "k": ["a", "b", "z", "y", "x"], + } + ) + assert_tpch_result_equal( + left, + right, + sort_by=[("v", False)], + abs_tol=1e-3, + check_exact=False, + limit=5, + ) + + @pytest.mark.parametrize("limit", [None, 5]) def test_assert_tpch_result_equal_float_sort_raises(limit: int | None) -> None: # Sort on a floating point column with a limit,