Skip to content

Fix int64 Spectral Embedding#7689

Merged
rapids-bot[bot] merged 3 commits intorapidsai:mainfrom
aamijar:fix-int64-spectral-embedding
Jan 16, 2026
Merged

Fix int64 Spectral Embedding#7689
rapids-bot[bot] merged 3 commits intorapidsai:mainfrom
aamijar:fix-int64-spectral-embedding

Conversation

@aamijar
Copy link
Copy Markdown
Member

@aamijar aamijar commented Jan 16, 2026

We need to edit the coo wrapper api (rows, cols, vals) to use int64 for the indexes and also the python nnz value.

@aamijar aamijar requested review from a team as code owners January 16, 2026 00:30
@github-actions github-actions Bot added Cython / Python Cython or Python issue CUDA/C++ labels Jan 16, 2026
@aamijar aamijar added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Jan 16, 2026
Copy link
Copy Markdown
Member

@dantegd dantegd left a comment

Choose a reason for hiding this comment

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

PR is correct and well done. The main question in the review is whether spectral_clustering should receive the same treatment for API consistency, or if there's a deliberate reason to keep them different.

Comment thread cpp/include/cuml/manifold/spectral_embedding.hpp
Comment thread python/cuml/cuml/manifold/spectral_embedding.pyx
@aamijar
Copy link
Copy Markdown
Member Author

aamijar commented Jan 16, 2026

The main question in the review is whether spectral_clustering should receive the same treatment for API consistency, or if there's a deliberate reason to keep them different.

Yes, that is in the works. There are some cuvs side changes that need to go in first though. Tracking that here rapidsai/cuvs#1484.

For more API consistency we may also change the output produced by spectral embedding to have int64 as well. That will require this rapidsai/raft#2925.

@dantegd
Copy link
Copy Markdown
Member

dantegd commented Jan 16, 2026

/merge

@rapids-bot rapids-bot Bot merged commit 369ab81 into rapidsai:main Jan 16, 2026
117 checks passed
@csadorf
Copy link
Copy Markdown
Contributor

csadorf commented Jan 16, 2026

@dantegd @aamijar I believe this should have been merged into the release/26.02 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CUDA/C++ Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants