Skip to content

Commit

Permalink
Fix is_min_close (#1419)
Browse files Browse the repository at this point in the history
Correlation and Cosine distance both return (1 - similarity) in the pairwise distances apis, meaning that is_min_close is returning the wrong sort order for them. Fix.

Authors:
  - Ben Frederickson (https://github.com/benfred)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #1419
  • Loading branch information
benfred authored Apr 17, 2023
1 parent bd69713 commit c7a72be
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 5 deletions.
2 changes: 0 additions & 2 deletions cpp/include/raft/distance/distance_types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ inline bool is_min_close(DistanceType metric)
bool select_min;
switch (metric) {
case DistanceType::InnerProduct:
case DistanceType::CosineExpanded:
case DistanceType::CorrelationExpanded:
// Similarity metrics have the opposite meaning, i.e. nearest neighbors are those with larger
// similarity (See the same logic at cpp/include/raft/sparse/spatial/detail/knn.cuh:362
// {perform_k_selection})
Expand Down
5 changes: 2 additions & 3 deletions cpp/include/raft/sparse/neighbors/detail/knn.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,7 @@ class sparse_knn_t {
// want to adjust k.
value_idx n_neighbors = std::min(static_cast<value_idx>(k), batch_cols);

bool ascending = true;
if (metric == raft::distance::DistanceType::InnerProduct) ascending = false;
bool ascending = raft::distance::is_min_close(metric);

// kernel to slice first (min) k cols and copy into batched merge buffer
raft::spatial::knn::select_k(batch_dists,
Expand Down Expand Up @@ -425,4 +424,4 @@ class sparse_knn_t {
raft::device_resources const& handle;
};

}; // namespace raft::sparse::neighbors::detail
}; // namespace raft::sparse::neighbors::detail

0 comments on commit c7a72be

Please sign in to comment.