[MRG] NMF and non_negative_factorization have inconsistent default init#12989
[MRG] NMF and non_negative_factorization have inconsistent default init#12989qinhanmin2014 merged 11 commits intoscikit-learn:masterfrom
Conversation
sklearn/decomposition/nmf.py
Outdated
|
|
||
| if init == "warn": | ||
| warnings.warn("The default value of init will change from " | ||
| "random to None in 0.22.", FutureWarning) |
There was a problem hiding this comment.
The behaviour won't change if n_components >= n_features so we could add that condition
jnothman
left a comment
There was a problem hiding this comment.
Thanks!
Please add an |API| entry to the change log at doc/whats_new/v0.21.rst. Like the other entries there, please reference this pull request with :issue: and credit yourself (and other contributors if applicable) with :user:
|
Sorry I shouldn't have approved: tests are failing. Maybe I've not reviewed correctly |
|
Travis still unhappy. Please remember to rename to MRG when you think this is safe to merge |
|
Sure. I'm still figuring out how to make Travis happy. Will change the title after figuring it out. |
|
@jnothman Travis is happy now. |
| W, H, _ = non_negative_factorization( | ||
| X, n_components=n_components, solver='mu', beta_loss=beta_loss, | ||
| random_state=0, max_iter=1000) | ||
| X, init='random', n_components=n_components, solver='mu', |
There was a problem hiding this comment.
we should probably make sure that init is explicitly set elsewhere in the tests.
There was a problem hiding this comment.
Sure. Added explicit init to all tests other than the consistency check test between NMF and non_negative_factorization.
qinhanmin2014
left a comment
There was a problem hiding this comment.
Please add the reason of the deprecation in what's new and the deprecation message (e.g., to keep consistent with decomposition.NMF).
please explain init=None in the docstring of non_negative_factorization.
please add init=None to possible options and explain it in the docstring of NMF.
doc/whats_new/v0.21.rst
Outdated
| :func:`decomposition.non_negative_factorization` will change from | ||
| :code:`random` to :code:`None` to match :class:`decomposition.NMF` | ||
| in version 0.23. A FutureWarning is raised when the default value | ||
| is used. :issue:`12988` by :user:`Zijie (ZJ) Poh <zjpoh>`. |
sklearn/decomposition/nmf.py
Outdated
| if init == "warn": | ||
| if n_components < n_features: | ||
| warnings.warn("The default value of init will change from " | ||
| "random to None in 0.23.", FutureWarning) |
There was a problem hiding this comment.
please also mention the reason in the deprecation message
…ult init (scikit-learn#12989)" This reverts commit 6966d73.
…ult init (scikit-learn#12989)" This reverts commit 6966d73.
Reference Issues/PRs
Fixes #12988. See also #11667.
What does this implement/fix? Explain your changes.
Changed the default in
non_negative_factorizationwith a deprecation process.