Skip to content

[FEA] Remove kmeans::fit_predict, optionally keep predicted labels from fit#1939

Open
lowener wants to merge 5 commits intorapidsai:mainfrom
lowener:26.06-kmeans-fit
Open

[FEA] Remove kmeans::fit_predict, optionally keep predicted labels from fit#1939
lowener wants to merge 5 commits intorapidsai:mainfrom
lowener:26.06-kmeans-fit

Conversation

@lowener
Copy link
Copy Markdown
Contributor

@lowener lowener commented Mar 20, 2026

In this PR I remove the kmeans::fit_predict() function, to instead add an optional to keep the labels already computed in fit().

Some unnecessary template instantiation are removed as well.

@lowener lowener added the non-breaking Introduces a non-breaking change label Mar 20, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot bot commented Mar 20, 2026

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.

@aamijar aamijar added the improvement Improves an existing functionality label Mar 20, 2026
@aamijar aamijar moved this to In Progress in Unstructured Data Processing Mar 20, 2026
@lowener lowener added the C++ label Mar 20, 2026
@lowener lowener marked this pull request as ready for review March 24, 2026 15:33
@lowener lowener requested review from a team as code owners March 24, 2026 15:33
typename IndexT,
typename LabelT,
typename MappingOpT = raft::identity_op>
void fit_predict(const raft::resources& handle,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question- can we just remove fit_predict() and use fit() as the only mechanism for passing optional labels? Would be nice to clean this up and remove it altogether.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes it would be possible to only have fit().
Note: I created #1944 to add a C API to fit_predict(). If we want to go with only fit() then we should skip it before introducing it.

lowener added 3 commits March 30, 2026 07:31
Signed-off-by: Mickael Ide <mide@nvidia.com>
Signed-off-by: Mickael Ide <mide@nvidia.com>
Signed-off-by: Mickael Ide <mide@nvidia.com>
@lowener lowener changed the base branch from main to release/26.04 March 30, 2026 14:33
@lowener lowener requested review from a team as code owners March 30, 2026 14:33
@lowener lowener requested a review from KyleFromNVIDIA March 30, 2026 14:33
@lowener lowener removed request for a team March 30, 2026 14:34
@lowener lowener removed request for a team and KyleFromNVIDIA March 30, 2026 14:34
@lowener lowener changed the title [FEA] Improve kmeans::fit_predict by keeping predicted labels from fit [FEA] Remove kmeans::fit_predict, optionally keep predicted labels from fit Mar 30, 2026
@tfeher tfeher changed the base branch from release/26.04 to main April 1, 2026 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants