Symmetrize the connecting-edge CSR in host-data connect_knn_graph - #2478
Open
alexfallin wants to merge 1 commit into
Open
Symmetrize the connecting-edge CSR in host-data connect_knn_graph#2478alexfallin wants to merge 1 commit into
alexfallin wants to merge 1 commit into
Conversation
The host-data variant of connect_knn_graph stores each cross-component connecting edge once, in an arbitrary direction, and passes the resulting CSR to raft::sparse::solver::mst, which requires a symmetric input (each undirected edge stored in both directions). The device-data variant already satisfies this via raft::sparse::linalg::symmetrize; this stores both directions in the host path as well.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2477
See linked issue for bug details.
This PR stores both directions of each connecting edge in the host path (rows/cols/vals duplicated with rows and cols swapped) before the sort +
sorted_coo_to_csr+mstsequence, matching the device path's behavior.Why now
Today the contract violation is masked (see bug), but the pending RAFT PR replaces the MST solver internals with an implementation may cause errors (both silent and loud). Without this fix, the host-data
connect_knn_graphpath (exercised by theconnect_knntest with host data) can fail to connect components or throw an error once the RAFT PR is approved.This change is safe to merge independently because the symmetric input satisfies both the current and the replacement solver
(the fixed symmetric edges match the device-data path anyway)
Notes
n_components - 1entries per iteration (typically tiny); doubling it is negligible.n_components > 1).detail/; theconnect_knngtest covers the host path).