Skip to content

MNT Clean-up deprecations for 1.7: TSNE's n_iter #31140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2025
+5 −64
Merged

MNT Clean-up deprecations for 1.7: TSNE's n_iter #31140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 5 additions & 42 deletions sklearn/manifold/_t_sne.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
# * Fast Optimization for t-SNE:
# https://cseweb.ucsd.edu/~lvdmaaten/workshops/nips2010/papers/vandermaaten.pdf

import warnings
from numbers import Integral, Real
from time import time

Expand All @@ -26,7 +25,7 @@
from ..neighbors import NearestNeighbors
from ..utils import check_random_state
from ..utils._openmp_helpers import _openmp_effective_n_threads
from ..utils._param_validation import Hidden, Interval, StrOptions, validate_params
from ..utils._param_validation import Interval, StrOptions, validate_params
from ..utils.validation import _num_samples, check_non_negative, validate_data

# mypy error: Module 'sklearn.manifold' has no attribute '_utils'
Expand Down Expand Up @@ -702,14 +701,6 @@ class TSNE(ClassNamePrefixFeaturesOutMixin, TransformerMixin, BaseEstimator):

.. versionadded:: 0.22

n_iter : int
Maximum number of iterations for the optimization. Should be at
least 250.

.. deprecated:: 1.5
`n_iter` was deprecated in version 1.5 and will be removed in 1.7.
Please use `max_iter` instead.

Attributes
----------
embedding_ : array-like of shape (n_samples, n_components)
Expand Down Expand Up @@ -794,7 +785,7 @@ class TSNE(ClassNamePrefixFeaturesOutMixin, TransformerMixin, BaseEstimator):
StrOptions({"auto"}),
Interval(Real, 0, None, closed="neither"),
],
"max_iter": [Interval(Integral, 250, None, closed="left"), None],
Copy link
Member

Choose a reason for hiding this comment

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

The message below says:

# Also make sure to change `max_iter` default back to 1000 and deprecate None

This removes None straight away and doesn't deprecate it, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes but this is not how we do it usually. We've decided that when we add None as a valid option for a deprecation, then we can remove it right away at the end of the deprecation. It's not like it's a meaningful option, just a sentinel during deprecation.

The writer of the message must have not been aware of this, but it's alright to do it anyway.

Copy link
Member

Choose a reason for hiding this comment

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

OK my main point was that in scikit-learn 1.6.1 the following code does not give any warning and in 1.7 this is going to break (with this PR merged):

import numpy as np
from sklearn.manifold import TSNE

TSNE(max_iter=None).fit(np.random.randn(1000, 10))

I guess that's OK because it's unlikely that someone has used max_iter=None explicitly?

Copy link
Member Author

Choose a reason for hiding this comment

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

indeed, we never encouraged to set None explicitely. It's there only to serve as a sentinel while we have the 2 max_iter and n_iter params. We already merged several PRs like this one for 1.7 and and before 1.6 as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

This make sense @jeremiedbb. So I guess this review comment can be resolved?

"max_iter": [Interval(Integral, 250, None, closed="left")],
"n_iter_without_progress": [Interval(Integral, -1, None, closed="left")],
"min_grad_norm": [Interval(Real, 0, None, closed="left")],
"metric": [StrOptions(set(_VALID_METRICS) | {"precomputed"}), callable],
Expand All @@ -808,10 +799,6 @@ class TSNE(ClassNamePrefixFeaturesOutMixin, TransformerMixin, BaseEstimator):
"method": [StrOptions({"barnes_hut", "exact"})],
"angle": [Interval(Real, 0, 1, closed="both")],
"n_jobs": [None, Integral],
"n_iter": [
Interval(Integral, 250, None, closed="left"),
Hidden(StrOptions({"deprecated"})),
],
}

# Control the number of exploration iterations with early_exaggeration on
Expand All @@ -827,7 +814,7 @@ def __init__(
perplexity=30.0,
early_exaggeration=12.0,
learning_rate="auto",
max_iter=None, # TODO(1.7): set to 1000
max_iter=1000,
n_iter_without_progress=300,
min_grad_norm=1e-7,
metric="euclidean",
Expand All @@ -838,7 +825,6 @@ def __init__(
method="barnes_hut",
angle=0.5,
n_jobs=None,
n_iter="deprecated",
):
self.n_components = n_components
self.perplexity = perplexity
Expand All @@ -855,7 +841,6 @@ def __init__(
self.method = method
self.angle = angle
self.n_jobs = n_jobs
self.n_iter = n_iter

def _check_params_vs_input(self, X):
if self.perplexity >= X.shape[0]:
Expand Down Expand Up @@ -1108,9 +1093,9 @@ def _tsne(
# Learning schedule (part 2): disable early exaggeration and finish
# optimization with a higher momentum at 0.8
P /= self.early_exaggeration
remaining = self._max_iter - self._EXPLORATION_MAX_ITER
remaining = self.max_iter - self._EXPLORATION_MAX_ITER
if it < self._EXPLORATION_MAX_ITER or remaining > 0:
opt_args["max_iter"] = self._max_iter
opt_args["max_iter"] = self.max_iter
opt_args["it"] = it + 1
opt_args["momentum"] = 0.8
opt_args["n_iter_without_progress"] = self.n_iter_without_progress
Expand Down Expand Up @@ -1155,28 +1140,6 @@ def fit_transform(self, X, y=None):
X_new : ndarray of shape (n_samples, n_components)
Embedding of the training data in low-dimensional space.
"""
# TODO(1.7): remove
# Also make sure to change `max_iter` default back to 1000 and deprecate None
if self.n_iter != "deprecated":
if self.max_iter is not None:
raise ValueError(
"Both 'n_iter' and 'max_iter' attributes were set. Attribute"
" 'n_iter' was deprecated in version 1.5 and will be removed in"
" 1.7. To avoid this error, only set the 'max_iter' attribute."
)
warnings.warn(
(
"'n_iter' was renamed to 'max_iter' in version 1.5 and "
"will be removed in 1.7."
),
FutureWarning,
)
self._max_iter = self.n_iter
elif self.max_iter is None:
self._max_iter = 1000
else:
self._max_iter = self.max_iter

self._check_params_vs_input(X)
embedding = self._fit(X)
self.embedding_ = embedding
Expand Down
22 changes: 0 additions & 22 deletions sklearn/manifold/tests/test_t_sne.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1185,25 +1185,3 @@ def test_tsne_works_with_pandas_output():
with config_context(transform_output="pandas"):
arr = np.arange(35 * 4).reshape(35, 4)
TSNE(n_components=2).fit_transform(arr)


# TODO(1.7): remove
def test_tnse_n_iter_deprecated():
"""Check `n_iter` parameter deprecated."""
random_state = check_random_state(0)
X = random_state.randn(40, 100)
tsne = TSNE(n_iter=250)
msg = "'n_iter' was renamed to 'max_iter'"
with pytest.warns(FutureWarning, match=msg):
tsne.fit_transform(X)


# TODO(1.7): remove
def test_tnse_n_iter_max_iter_both_set():
"""Check error raised when `n_iter` and `max_iter` both set."""
random_state = check_random_state(0)
X = random_state.randn(40, 100)
tsne = TSNE(n_iter=250, max_iter=500)
msg = "Both 'n_iter' and 'max_iter' attributes were set"
with pytest.raises(ValueError, match=msg):
tsne.fit_transform(X)