SCML: Raise ValueError if n_features larger than n_triplets #350
Conversation
nikosmichas
force-pushed
the
scml_error_if_large_n_features
branch
from
caba555 to
a15470e
Compare
metric_learn/scml.py
Outdated
| @@ -240,6 +240,12 @@ def _generate_bases_dist_diff(self, triplets, X): | |||
| raise ValueError("n_basis should be an integer, instead it is of type %s" | |||
| % type(self.n_basis)) | |||
|
|
|||
| if n_features > n_triplets: | |||
| raise ValueError( | |||
| "Number of features (%s) is greater than the nuber of triplets(%s).\n" | |||
| "Number of features (%s) is greater than the nuber of triplets(%s).\n" | ||
| "Consider using a dimensionality reduction preprocessing or create " | ||
| "a new basis generation scheme." % (n_features, n_triplets)) | ||
|
|
There was a problem hiding this comment.
Maybe I would reformulate it to
"Consider using dimensionality reduction or using another basis generation scheme."
This way we consider possible future basis generation alternatives as well as just passing in a basis set directly.
nikosmichas
force-pushed
the
scml_error_if_large_n_features
branch
from
a15470e to
51c35b7
Compare
There was a problem hiding this comment.
Thanks for the work @nikosmichas !
This looks good to me, except that I realize that the docstring entry for the ‘triplet_diffs’ option says:
The basis set is constructed from the differences between points of n_basis positive or negative pairs taken from the triplets constrains.
Shoudn't it be something like:
Each basis is constructed from the differences between points of n_features positive or negative pairs taken randomly from the triplets constraints.
@bellet not sure about the exact wording. I can make the docstring change if you think that it's better. Do you thing that the change is related to this PR ? |
|
Merging now, we can fix the docstring separately. |
|
Opened a separate PR for #351 |
|
Thanks @nikosmichas ! |
Related discussion #348
cc @grudloff