Skip to content
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.

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

Fix covariance initialization when matrix is not invertible #277

Merged
merged 17 commits into from
Feb 4, 2020
74 changes: 69 additions & 5 deletions test/test_mahalanobis_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
from scipy.stats import ortho_group
from sklearn import clone
from sklearn.cluster import DBSCAN
from sklearn.datasets import make_spd_matrix
from sklearn.utils import check_random_state
from sklearn.datasets import make_spd_matrix, make_blobs
from sklearn.utils import check_random_state, shuffle
from sklearn.utils.multiclass import type_of_target
from sklearn.utils.testing import set_random_state

from metric_learn._util import make_context
from metric_learn._util import make_context, _initialize_metric_mahalanobis
from metric_learn.base_metric import (_QuadrupletsClassifierMixin,
_PairsClassifierMixin)
from metric_learn.exceptions import NonPSDError
Expand Down Expand Up @@ -569,7 +569,7 @@ def test_init_mahalanobis(estimator, build_dataset):
in zip(ids_metric_learners,
metric_learners)
if idml[:4] in ['ITML', 'SDML', 'LSML']])
def test_singular_covariance_init_or_prior(estimator, build_dataset):
def test_singular_covariance_init_or_prior_strictpd(estimator, build_dataset):
"""Tests that when using the 'covariance' init or prior, it returns the
appropriate error if the covariance matrix is singular, for algorithms
that need a strictly PD prior or init (see
Expand Down Expand Up @@ -603,6 +603,45 @@ def test_singular_covariance_init_or_prior(estimator, build_dataset):
assert str(raised_err.value) == msg


@pytest.mark.integration
@pytest.mark.parametrize('estimator, build_dataset',
[(ml, bd) for idml, (ml, bd)
in zip(ids_metric_learners,
metric_learners)
if idml[:3] in ['MMC']],
ids=[idml for idml, (ml, _)
in zip(ids_metric_learners,
metric_learners)
if idml[:3] in ['MMC']])
def test_singular_covariance_init_of_non_strict_pd(estimator, build_dataset):
"""Tests that when using the 'covariance' init or prior, it returns the
appropriate warning if the covariance matrix is singular, for algorithms
that don't need a strictly PD init. Also checks that the returned
inverse matrix has finite values
Copy link
Member

Choose a reason for hiding this comment

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

This is not what you check, right? You seem to check the learned M.
Any reason you do this and not a call to _initialize_metric_mahalanobis like in the other test? Maybe you could check both things in both tests?

Copy link
Contributor Author

@grudloff grudloff Feb 3, 2020

Choose a reason for hiding this comment

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

Yes, you are totally right! Forgot to set the max iteration in order to make it stop before the first iteration.

I called _initialize_metric_mahalanobis directly in the other test because currently there is no algorithm that could reproduce that setting. But setting max_iter = 0 should keep it from iterating and would result in the M from the initialization.

Edit: It isn't possible to set max_iter = 0, so instead I will do it like you sugested.

"""
input_data, labels, _, X = build_dataset()
model = clone(estimator)
set_random_state(model)
# We create a feature that is a linear combination of the first two
# features:
input_data = np.concatenate([input_data, input_data[:, ..., :2].dot([[2],
[3]])],
axis=-1)
model.set_params(init='covariance')
msg = ('The initialization matrix is not invertible: '
'using the pseudo-inverse instead.'
'To make the inverse covariance matrix invertible'
' you can remove any linearly dependent features and/or '
'reduce the dimensionality of your input, '
'for instance using `sklearn.decomposition.PCA` as a '
'preprocessing step.')
with pytest.warns(UserWarning) as raised_warning:
model.fit(input_data, labels)
assert np.any([str(warning.message) == msg for warning in raised_warning])
M = model.get_mahalanobis_matrix()
assert np.isfinite(M).all()


@pytest.mark.integration
@pytest.mark.parametrize('estimator, build_dataset',
[(ml, bd) for idml, (ml, bd)
Expand All @@ -614,7 +653,7 @@ def test_singular_covariance_init_or_prior(estimator, build_dataset):
metric_learners)
if idml[:4] in ['ITML', 'SDML', 'LSML']])
@pytest.mark.parametrize('w0', [1e-20, 0., -1e-20])
def test_singular_array_init_or_prior(estimator, build_dataset, w0):
def test_singular_array_init_or_prior_strictpd(estimator, build_dataset, w0):
"""Tests that when using a custom array init (or prior), it returns the
appropriate error if it is singular, for algorithms
that need a strictly PD prior or init (see
Expand Down Expand Up @@ -654,6 +693,31 @@ def test_singular_array_init_or_prior(estimator, build_dataset, w0):
assert str(raised_err.value) == msg


@pytest.mark.parametrize('w0', [1e-20, 0., -1e-20])
def test_singular_array_init_of_non_strict_pd(w0):
"""Tests that when using a custom array init, it returns the
appropriate warning if it is singular. Also checks if the returned
inverse matrix is finite. This isn't checked for model fitting as no
model curently uses this setting.
"""
rng = np.random.RandomState(42)
X, y = shuffle(*make_blobs(random_state=rng),
random_state=rng)
P = ortho_group.rvs(X.shape[1], random_state=rng)
w = np.abs(rng.randn(X.shape[1]))
w[0] = w0
M = P.dot(np.diag(w)).dot(P.T)
Comment on lines +707 to +712
Copy link
Member

Choose a reason for hiding this comment

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

this seems a bit convoluted just to generated a singular matrix? You could just draw a rectangular matrix L and construct M as L.T.dot(L)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you, just did it that way because it was done like that on test_singular_array_init_or_prior_strictpd. But also this way you have only one zero eigenvalue and control its value directly with the parametrization.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good

msg = ('The initialization matrix is not invertible: '
'using the pseudo-inverse instead.')
with pytest.warns(UserWarning) as raised_warning:
_, M_inv = _initialize_metric_mahalanobis(X, init=M,
random_state=rng,
return_inverse=True,
strict_pd=False)
assert str(raised_warning[0].message) == msg
assert np.isfinite(M_inv).all()


@pytest.mark.integration
@pytest.mark.parametrize('estimator, build_dataset', metric_learners,
ids=ids_metric_learners)
Expand Down