MRG : add support for sample_weights in StandardScaler when X is sparse by agramfort · Pull Request #18682 · scikit-learn/scikit-learn

agramfort · 2020-10-25T13:34:52Z

glemaitre

Just a quick partial review. I need more time to have a better look.

ogrisel

Besides @glemaitre review and the comments below, this LGTM.

I think we should also add a test that checks that the results of fitting a scaler on sparse data with missing values and sample weights is the same as fitting a scaler with a dense version of this data. I don't think the current tests cover this comparison.

maikia

LGTM thanks @agramfort

maikia · 2020-10-26T09:50:40Z

        The number of samples processed by the estimator for each feature.
        If there are not missing samples, the ``n_samples_seen`` will be an
-        integer, otherwise it will be an array.
+        integer, otherwise it will be an array. If sample_weights are


Suggested change

integer, otherwise it will be an array. If sample_weights are

integer, otherwise it will be an array of dtype=int. If sample_weights are

maikia · 2020-10-26T09:51:14Z

        If there are not missing samples, the ``n_samples_seen`` will be an
-        integer, otherwise it will be an array.
+        integer, otherwise it will be an array. If sample_weights are
+        used it will be an array that sums the weights seen so far.


Suggested change

used it will be an array that sums the weights seen so far.

used it will be an array of dtype=float that sums the weights seen so far.

maikia

I would also update the dtype test (test_standard_scaler_dtype) to include sparse in test_data.py (it revealed some problems when we did it for dense).
This could work:

@pytest.mark.parametrize("sparse_constructor",
                         [None, sparse.csc_matrix, sparse.csr_matrix])
@pytest.mark.parametrize("add_sample_weight", [False, True])
def test_standard_scaler_dtype(add_sample_weight, sparse_constructor):
    # Ensure scaling does not affect dtype
    rng = np.random.RandomState(0)
    n_samples = 10
    n_features = 3
    if add_sample_weight:
        sample_weight = np.ones(n_samples)
    else:
        sample_weight = None
    for dtype in [np.float16, np.float32, np.float64]:
        with_mean = True
        X = rng.randn(n_samples, n_features).astype(dtype)
        if sparse_constructor is not None:
            X = sparse_constructor(X)
            with_mean = False

        scaler = StandardScaler(with_mean=with_mean)
        X_scaled = scaler.fit(X, sample_weight=sample_weight).transform(X)
        assert X.dtype == X_scaled.dtype
        assert scaler.mean_.dtype == np.float64
        assert scaler.scale_.dtype == np.float64

agramfort · 2020-10-26T18:42:46Z

@maikia @ogrisel @glemaitre I think I addressed all comments here.

ogrisel

LGTM (again :)

glemaitre

Otherwise LGTM

glemaitre · 2020-10-27T16:55:40Z

-        integer, otherwise it will be an array.
+        If there are no missing samples, the ``n_samples_seen`` will be an
+        integer, otherwise it will be an array of dtype int. If
+        sample_weights are used it will be a float (if no missing data)


Suggested change

sample_weights are used it will be a float (if no missing data)

`sample_weights` are used it will be a float (if no missing data)

glemaitre · 2020-10-27T16:58:03Z

        X = self._validate_data(X, accept_sparse=('csr', 'csc'),
                                estimator=self, dtype=FLOAT_DTYPES,
                                force_all_finite='allow-nan', reset=first_call)
+        n_samples, n_features = X.shape


Suggested change

n_samples, n_features = X.shape

n_features = X.shape[1]

glemaitre · 2020-10-27T17:00:24Z

+        elif (hasattr(self, 'n_samples_seen_') and
+              np.size(self.n_samples_seen_) == 1):


Suggested change

elif (hasattr(self, 'n_samples_seen_') and

np.size(self.n_samples_seen_) == 1):

elif np.size(self.n_samples_seen_) == 1:

glemaitre · 2020-10-27T18:26:39Z

+@pytest.mark.parametrize("sparse_constructor",
+                         [None, sparse.csc_matrix, sparse.csr_matrix])
+def test_standard_scaler_sample_weight(
+        Xw, X, sample_weight, sparse_constructor):
+
+    with_mean = True
+    if sparse_constructor is not None:
+        X = sparse_constructor(X)
+        Xw = sparse_constructor(Xw)
+        with_mean = False
+    else:
+        Xw = np.array(Xw)
+        X = np.array(X)


Suggested change

@pytest.mark.parametrize("sparse_constructor",

[None, sparse.csc_matrix, sparse.csr_matrix])

def test_standard_scaler_sample_weight(

Xw, X, sample_weight, sparse_constructor):

with_mean = True

if sparse_constructor is not None:

X = sparse_constructor(X)

Xw = sparse_constructor(Xw)

with_mean = False

else:

Xw = np.array(Xw)

X = np.array(X)

@pytest.mark.parametrize(

"array_constructor", ["array", "sparse_csr", "sparse_csc"]

)

def test_standard_scaler_sample_weight(

Xw, X, sample_weight, array_constructor):

with_mean = not array_constructor.startswith("sparse")

X, = _convert_container(X, array_constructor)

Xw = _convert_container(Xw, array_constructor)

You need to add the following line sklearn.utils._testing._convert_container:

elif constructor_name == 'sparse_csr': return sp.sparse.csr_matrix(container) elif constructor_name == 'sparse_csc': return sp.sparse.csc_matrix(container)

glemaitre · 2020-10-27T18:33:15Z

        Axis along which the axis should be computed.

+    weights : ndarray of shape (n_samples,) or (n_features,), default=None
+        if axis is set to 0 shape is (n_samples,) or


Suggested change

if axis is set to 0 shape is (n_samples,) or

If axis is set to 0 shape is (n_samples,) or

glemaitre · 2020-10-27T18:33:37Z

    variances : float array with shape (n_features,)
        Feature-wise variances

+    sum_weights : float array with shape (n_features,)


Suggested change

sum_weights : float array with shape (n_features,)

sum_weights : ndarray of shape (n_features,), dtype=floating

glemaitre · 2020-10-27T18:33:48Z

        Feature-wise variances

+    sum_weights : float array with shape (n_features,)
+        Returned if return_sum_weights is True.


Suggested change

Returned if return_sum_weights is True.

Returned if `return_sum_weights` is `True`.

glemaitre · 2020-10-27T18:34:07Z

+
+    return_sum_weights : bool, default=False
+        If True, returns the sum of weights seen for each feature
+        if axis=0 or each sample if axis=1.


Suggested change

if axis=0 or each sample if axis=1.

if `axis=0` or each sample if `axis=1`.

glemaitre · 2020-10-28T08:34:29Z

Thanks @agramfort @maikia

add support for sample_weights in StandardScaler when X is sparse

4660537

github-actions Bot added module:preprocessing module:utils labels Oct 25, 2020

agramfort added 3 commits October 25, 2020 14:36

clean

b60e530

remove hack + simplify

ed6dc7d

improve tests

f8b88e5

agramfort changed the title ~~WIP : add support for sample_weights in StandardScaler when X is sparse~~ MRG : add support for sample_weights in StandardScaler when X is sparse Oct 25, 2020

agramfort added 2 commits October 25, 2020 17:38

update what's new

f9b0f9a

even better tests

4af7a50

glemaitre reviewed Oct 25, 2020

View reviewed changes

ogrisel approved these changes Oct 26, 2020

View reviewed changes

maikia reviewed Oct 26, 2020

View reviewed changes

address review comments

068b8fa

agramfort added 3 commits October 26, 2020 19:48

some more

a6cd398

some more

c13d4ca

some more

ea92532

ogrisel approved these changes Oct 27, 2020

View reviewed changes

glemaitre self-requested a review October 27, 2020 16:50

glemaitre reviewed Oct 27, 2020

View reviewed changes

review guillaume

18ef071

glemaitre merged commit 5263eea into scikit-learn:master Oct 28, 2020

lorentzenchr mentioned this pull request Feb 10, 2021

Add sample_weight support to StandardScaler (and friends?) #15601

Closed

	integer, otherwise it will be an array. If sample_weights are
	integer, otherwise it will be an array of dtype=int. If sample_weights are

	used it will be an array that sums the weights seen so far.
	used it will be an array of dtype=float that sums the weights seen so far.

	sample_weights are used it will be a float (if no missing data)
	`sample_weights` are used it will be a float (if no missing data)

		elif (hasattr(self, 'n_samples_seen_') and
		np.size(self.n_samples_seen_) == 1):

	elif (hasattr(self, 'n_samples_seen_') and
	np.size(self.n_samples_seen_) == 1):
	elif np.size(self.n_samples_seen_) == 1:

	if axis is set to 0 shape is (n_samples,) or
	If axis is set to 0 shape is (n_samples,) or

	sum_weights : float array with shape (n_features,)
	sum_weights : ndarray of shape (n_features,), dtype=floating

	Returned if return_sum_weights is True.
	Returned if `return_sum_weights` is `True`.

	if axis=0 or each sample if axis=1.
	if `axis=0` or each sample if `axis=1`.

Conversation

agramfort commented Oct 25, 2020 • edited by lorentzenchr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maikia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maikia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agramfort commented Oct 26, 2020

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glemaitre commented Oct 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

agramfort commented Oct 25, 2020 •

edited by lorentzenchr

Loading

maikia left a comment •

edited

Loading