Skip to content

Conversation

jinsolp
Copy link
Contributor

@jinsolp jinsolp commented Aug 8, 2025

Using all_neighbors mutual reachability feature to work towards deprecating cuvs::neighbors::detail::reachability::mutual_reachability_graph (related issue: #982).

Also, changes build_linkage to use int64_t, because all_neighbors::build only supports int64_t types, and we will change HDBSCAN to use int64_t types in cuML as well.

This PR will break cuML (which will be solved by rapidsai/cuml#7104)

Waiting for #1016 to be merged.

@jinsolp jinsolp self-assigned this Aug 8, 2025
@jinsolp jinsolp requested a review from a team as a code owner August 8, 2025 20:14
@jinsolp jinsolp added breaking Introduces a breaking change improvement Improves an existing functionality DO NOT MERGE labels Aug 8, 2025
@jinsolp jinsolp moved this from Todo to In Progress in Vector Search, ML, & Data Mining Release Board Aug 8, 2025
@jinsolp jinsolp marked this pull request as draft August 8, 2025 20:15
Copy link

copy-pr-bot bot commented Aug 8, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@beckernick
Copy link
Member

beckernick commented Aug 12, 2025

Excited to see how HDBSCAN scales!

@jinsolp jinsolp changed the title [WIP][DO NOT MERGE] Using all_neighbors for mutual reachability Using all_neighbors for mutual reachability Aug 18, 2025
@jinsolp jinsolp marked this pull request as ready for review August 18, 2025 23:49
@jinsolp jinsolp force-pushed the hdbscan-int64-pub branch from 198e7e9 to 032506d Compare August 18, 2025 23:58
raft::device_matrix_view<const float, int, raft::row_major> X,
raft::device_matrix_view<const float, int64_t, raft::row_major> X,
std::variant<linkage_graph_params::distance_params,
linkage_graph_params::mutual_reachability_params> linkage_graph_params,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add some docs for this new option and highlight how it differs from the distance_params.

@jinsolp
Copy link
Contributor Author

jinsolp commented Aug 27, 2025

@cjnolet @tarang-jain I've added what all_neighbors_params is for in struct mutual_reachability_params. I didn't add details because I thought it makes sense for the users to look into the docs of all_neighbors_params itself for details. Should we be adding the details here too?

@cjnolet
Copy link
Member

cjnolet commented Aug 27, 2025

I didn't add details because I thought it makes sense for the users to look into the docs of all_neighbors_params itself for details

We definitely should document the mutual reachabilty feature in the linkage function itself and not expect users to look for themselves. It doesn't need to be exhaustive, but the docs for every argument should summarize the intention and purpose fo the argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Introduces a breaking change improvement Improves an existing functionality
Projects
Development

Successfully merging this pull request may close these issues.

4 participants