Skip to content

Inspect KNN graph during smooth KNN generation and fail if necessary#6491

Merged
rapids-bot[bot] merged 11 commits intorapidsai:branch-25.06from
viclafargue:fix-smooth-knn-issue
May 27, 2025
Merged

Inspect KNN graph during smooth KNN generation and fail if necessary#6491
rapids-bot[bot] merged 11 commits intorapidsai:branch-25.06from
viclafargue:fix-smooth-knn-issue

Conversation

@viclafargue
Copy link
Copy Markdown
Contributor

Answers #6490

@viclafargue viclafargue requested a review from a team as a code owner April 1, 2025 13:53
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Apr 1, 2025
@viclafargue viclafargue added bug Something isn't working non-breaking Non-breaking change labels Apr 1, 2025
@csadorf
Copy link
Copy Markdown
Contributor

csadorf commented May 1, 2025

@viclafargue Is this ready for review?

@github-actions github-actions Bot removed the Cython / Python Cython or Python issue label May 2, 2025
@viclafargue
Copy link
Copy Markdown
Contributor Author

Yes, it's ready for review.

@csadorf csadorf removed the request for review from a team May 14, 2025 21:30
Comment thread cpp/src/umap/fuzzy_simpl_set/naive.cuh Outdated
Comment thread cpp/src/umap/fuzzy_simpl_set/naive.cuh Outdated
@divyegala
Copy link
Copy Markdown
Member

@viclafargue looking at the test failures, it looks like the fix is not going to be so easy. Should we slip this to 25.08?

@github-actions github-actions Bot added the Cython / Python Cython or Python issue label May 26, 2025
@viclafargue
Copy link
Copy Markdown
Contributor Author

@viclafargue looking at the test failures, it looks like the fix is not going to be so easy. Should we slip this to 25.08?

This is not exactly a fix, but rather a safeguard to alert users when a malformed graph is detected. The issue likely does not originate from the PR itself, but from the test setup. Specifically, the test_umap_distance_metrics_fit_transform_trust_on_sparse_input test using the jaccard metric (which compares sets) included samples that had no neighbors with a non-zero distance indicating the presence of many similar sets (n_neighbors occurrences). Increasing the number of features in the test resolves this issue.

@viclafargue viclafargue requested a review from divyegala May 27, 2025 15:09
@csadorf
Copy link
Copy Markdown
Contributor

csadorf commented May 27, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 969219a into rapidsai:branch-25.06 May 27, 2025
93 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CUDA/C++ Cython / Python Cython or Python issue non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] UMAP OOB with all-zero neighboring distances and local_connectivity = 1

4 participants