From 002a134284fffd168b9c4628d0f0fe6bdd4c6d7d Mon Sep 17 00:00:00 2001 From: "Corey J. Nolet" Date: Mon, 21 Aug 2023 16:57:20 -0400 Subject: [PATCH 1/6] Using expanded distance computations instead of unexpanded --- python/pylibraft/pylibraft/distance/pairwise_distance.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/pylibraft/pylibraft/distance/pairwise_distance.pyx b/python/pylibraft/pylibraft/distance/pairwise_distance.pyx index 3037b9a725..4111e32740 100644 --- a/python/pylibraft/pylibraft/distance/pairwise_distance.pyx +++ b/python/pylibraft/pylibraft/distance/pairwise_distance.pyx @@ -61,8 +61,8 @@ cdef extern from "raft_runtime/distance/pairwise_distance.hpp" \ DISTANCE_TYPES = { "l2": DistanceType.L2SqrtUnexpanded, - "sqeuclidean": DistanceType.L2Unexpanded, - "euclidean": DistanceType.L2SqrtUnexpanded, + "sqeuclidean": DistanceType.L2Expanded, + "euclidean": DistanceType.L2SqrtExpanded, "l1": DistanceType.L1, "cityblock": DistanceType.L1, "inner_product": DistanceType.InnerProduct, From e449ffd00a0bf7691f3d89d6743bf397f0c404e1 Mon Sep 17 00:00:00 2001 From: "Corey J. Nolet" Date: Tue, 22 Aug 2023 12:40:25 -0400 Subject: [PATCH 2/6] Modifying thresholds of tests --- python/pylibraft/pylibraft/distance/pairwise_distance.pyx | 2 +- python/pylibraft/pylibraft/test/test_brute_force.py | 2 +- python/pylibraft/pylibraft/test/test_distance.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/pylibraft/pylibraft/distance/pairwise_distance.pyx b/python/pylibraft/pylibraft/distance/pairwise_distance.pyx index 4111e32740..20dadf0275 100644 --- a/python/pylibraft/pylibraft/distance/pairwise_distance.pyx +++ b/python/pylibraft/pylibraft/distance/pairwise_distance.pyx @@ -60,7 +60,7 @@ cdef extern from "raft_runtime/distance/pairwise_distance.hpp" \ float metric_arg) except + DISTANCE_TYPES = { - "l2": DistanceType.L2SqrtUnexpanded, + "l2": DistanceType.L2SqrtExpanded, "sqeuclidean": DistanceType.L2Expanded, "euclidean": DistanceType.L2SqrtExpanded, "l1": DistanceType.L1, diff --git a/python/pylibraft/pylibraft/test/test_brute_force.py b/python/pylibraft/pylibraft/test/test_brute_force.py index 2e118d210d..42095c3b9f 100644 --- a/python/pylibraft/pylibraft/test/test_brute_force.py +++ b/python/pylibraft/pylibraft/test/test_brute_force.py @@ -89,7 +89,7 @@ def test_knn(n_index_rows, n_query_rows, n_cols, k, inplace, metric, dtype): cpu_ordered = pw_dists[i, expected_indices] np.testing.assert_allclose( - cpu_ordered[:k], gpu_dists, atol=1e-4, rtol=1e-4 + cpu_ordered[:k], gpu_dists, atol=1e-3, rtol=1e-3 ) diff --git a/python/pylibraft/pylibraft/test/test_distance.py b/python/pylibraft/pylibraft/test/test_distance.py index 2c0a842fe5..2f1156fa86 100644 --- a/python/pylibraft/pylibraft/test/test_distance.py +++ b/python/pylibraft/pylibraft/test/test_distance.py @@ -81,4 +81,4 @@ def test_distance(n_rows, n_cols, inplace, metric, order, dtype): actual[actual <= 1e-5] = 0.0 - assert np.allclose(expected, actual, rtol=1e-4) + assert np.allclose(expected, actual, rtol=1e-3) From 03ec7c8824cd6d24a0fba2080af81b7a73b3b1ac Mon Sep 17 00:00:00 2001 From: "Corey J. Nolet" Date: Tue, 22 Aug 2023 18:24:56 -0400 Subject: [PATCH 3/6] Proper thresholding --- cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh b/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh index 95577fd311..0894c8d883 100644 --- a/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh +++ b/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh @@ -33,7 +33,7 @@ struct l2_exp_cutlass_op { // outVal could be negative due to numerical instability, especially when // calculating self distance. // clamp to 0 to avoid potential NaN in sqrt - outVal = outVal * (outVal > DataT(0.0)); + outVal = outVal * (fabs(outVal) >= DataT(0.0001)); return sqrt ? raft::sqrt(outVal) : outVal; } @@ -88,7 +88,7 @@ struct l2_exp_distance_op { DataT val = regxn[i] + regyn[j] - (DataT)2.0 * acc[i][j]; // val could be negative due to numerical instability, especially when // calculating self distance. Clamp to 0 to avoid potential NaN in sqrt - acc[i][j] = val * (val > DataT(0.0)); + acc[i][j] = val * (fabs(val) >= DataT(0.0001)); } } if (sqrt) { From ae45065a0182ee701757de516eaf1411dbe9af8f Mon Sep 17 00:00:00 2001 From: "Corey J. Nolet" Date: Wed, 23 Aug 2023 10:20:00 -0400 Subject: [PATCH 4/6] Hoping this works without the fabs --- cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh b/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh index 0894c8d883..45a93f8c1c 100644 --- a/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh +++ b/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh @@ -33,7 +33,7 @@ struct l2_exp_cutlass_op { // outVal could be negative due to numerical instability, especially when // calculating self distance. // clamp to 0 to avoid potential NaN in sqrt - outVal = outVal * (fabs(outVal) >= DataT(0.0001)); + outVal = outVal * (outVal <= DataT(0.0001)); return sqrt ? raft::sqrt(outVal) : outVal; } @@ -88,7 +88,7 @@ struct l2_exp_distance_op { DataT val = regxn[i] + regyn[j] - (DataT)2.0 * acc[i][j]; // val could be negative due to numerical instability, especially when // calculating self distance. Clamp to 0 to avoid potential NaN in sqrt - acc[i][j] = val * (fabs(val) >= DataT(0.0001)); + acc[i][j] = val * (val <= DataT(0.0001)); } } if (sqrt) { From 0b9ce69ecea2cc8ae907ecd25d3471b69adb6516 Mon Sep 17 00:00:00 2001 From: "Corey J. Nolet" Date: Wed, 23 Aug 2023 13:28:02 -0400 Subject: [PATCH 5/6] Adding abs to clamping --- cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh b/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh index 45a93f8c1c..5e93d9e33b 100644 --- a/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh +++ b/cpp/include/raft/distance/detail/distance_ops/l2_exp.cuh @@ -16,6 +16,7 @@ #pragma once +#include #include // DI namespace raft::distance::detail::ops { @@ -33,7 +34,7 @@ struct l2_exp_cutlass_op { // outVal could be negative due to numerical instability, especially when // calculating self distance. // clamp to 0 to avoid potential NaN in sqrt - outVal = outVal * (outVal <= DataT(0.0001)); + outVal = outVal * (raft::abs(outVal) >= DataT(0.0001)); return sqrt ? raft::sqrt(outVal) : outVal; } @@ -88,7 +89,7 @@ struct l2_exp_distance_op { DataT val = regxn[i] + regyn[j] - (DataT)2.0 * acc[i][j]; // val could be negative due to numerical instability, especially when // calculating self distance. Clamp to 0 to avoid potential NaN in sqrt - acc[i][j] = val * (val <= DataT(0.0001)); + acc[i][j] = val * (raft::abs(val) >= DataT(0.0001)); } } if (sqrt) { From 9da87fa8e37a45f03795de73de09b2ed40e44b0c Mon Sep 17 00:00:00 2001 From: "Corey J. Nolet" Date: Wed, 23 Aug 2023 16:46:17 -0400 Subject: [PATCH 6/6] Update python/pylibraft/pylibraft/test/test_distance.py Co-authored-by: Ben Frederickson --- python/pylibraft/pylibraft/test/test_distance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/pylibraft/pylibraft/test/test_distance.py b/python/pylibraft/pylibraft/test/test_distance.py index 2f1156fa86..f9d3890ff7 100644 --- a/python/pylibraft/pylibraft/test/test_distance.py +++ b/python/pylibraft/pylibraft/test/test_distance.py @@ -81,4 +81,4 @@ def test_distance(n_rows, n_cols, inplace, metric, order, dtype): actual[actual <= 1e-5] = 0.0 - assert np.allclose(expected, actual, rtol=1e-3) + assert np.allclose(expected, actual, atol=1e-3, rtol=1e-3)