Skip to content

Expose TSNE.n_iter_ fitted attribute#7142

Merged
rapids-bot[bot] merged 4 commits intorapidsai:branch-25.10from
jcrist:tsne-n-iter
Aug 29, 2025
Merged

Expose TSNE.n_iter_ fitted attribute#7142
rapids-bot[bot] merged 4 commits intorapidsai:branch-25.10from
jcrist:tsne-n-iter

Conversation

@jcrist
Copy link
Copy Markdown
Member

@jcrist jcrist commented Aug 26, 2025

This:

  • Plumbs through n_iter_ as a fitted attribute of TSNE. This should be an integer between 0 and n_iter (the maximum number of iterations supported), and represents the number of iterations the algorithm actually ran for.
  • Exposes TSNE.n_iter_ through cuml.accel.

A couple notes:

  • Our C++ APIs are inconsistent in how we handle output parameters. I went for the smallest amount of code change possible, but no strong thoughts if this is the best way to structure things 🤷.
  • I'm unable to find a case where we exit early, even though all algorithms (excluding barnes_hut) do have an early stopping condition. I still think exposing this this way is good (rather than just assuming max iterations and doing self.n_iter_ = self.n_iter) since it leaves plumbing in place in case the early stopping criteria changes (or there are cases where we do succeed in stopping early).
  • This only makes the test for the existence of TSNE.n_iter_ pass, since the other failing tests around n_iter_ were also related to the exact logging message shown to the user. I think that's fine.

Fixes #6857

@jcrist jcrist self-assigned this Aug 26, 2025
@jcrist jcrist added the improvement Improvement / enhancement to an existing function label Aug 26, 2025
@jcrist jcrist requested a review from a team as a code owner August 26, 2025 21:37
@jcrist jcrist added the non-breaking Non-breaking change label Aug 26, 2025
@jcrist jcrist requested a review from a team as a code owner August 26, 2025 21:37
@jcrist jcrist added the cuml-accel Issues related to cuml.accel label Aug 26, 2025
@github-actions github-actions Bot added Cython / Python Cython or Python issue CUDA/C++ labels Aug 26, 2025
Copy link
Copy Markdown
Member

@betatim betatim left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. I think having the plumping here is sensible, even if it doesn't seem needed.

My one review comment is more for my education/asking why we have all this templating if the types are anyway fixed (YAGNI)

Comment thread cpp/src/tsne/barnes_hut_tsne.cuh
Copy link
Copy Markdown
Contributor

@viclafargue viclafargue left a comment

Choose a reason for hiding this comment

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

LGTM! Just one comment.

Comment thread python/cuml/cuml/manifold/t_sne.pyx
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Aug 29, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 1dc8ebe into rapidsai:branch-25.10 Aug 29, 2025
76 checks passed
@jcrist jcrist deleted the tsne-n-iter branch August 29, 2025 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CUDA/C++ cuml-accel Issues related to cuml.accel 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.

Expose n_iter_ in TSNE to increase cuml.accel compatibility

5 participants