8000 DOC Clarify how initial conditions are chosen for NMF. by bsmith89 · Pull Request #17369 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

DOC Clarify how initial conditions are chosen for NMF. #17369

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

Jump to bottom
Merged
merged 2 commits into from
Jun 3, 2020
Merged

DOC Clarify how initial conditions are chosen for NMF. #17369

merged 2 commits into from
Jun 3, 2020

Conversation

bsmith89
Copy link
Contributor
@bsmith89 bsmith89 commented May 28, 2020 •
edited
Loading

Reference Issues/PRs

None yet. I can submit an issue if that's important.

What does this implement/fix? Explain your changes.

For sklearn.decomposition.non_negative_factorization, when update_H is False, any given initial value of W is silently ignored, as are all of the initialization procedures.

See: sklearn/decomposition/_nmf.py#L1032-L1053

This commit fixes that by clarifying this in the non_negative_factorization function docstring.

Any other comments?

Nope. Simple problem, simple fix.
8000

@bsmith89
Clarify how initial conditions are chosen for nmf.
b80adf8 
When update_H is False, any given initial value of W is silently
ignored, as are all of the initialization procedures.

This commit fixes that by clarifying this in the
non_negative_factorization function docstring.
thomasjpfan
thomasjpfan reviewed Jun 1, 2020
View reviewed changes
Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @bsmith89 !

sklearn/decomposition/_nmf.py Outdated
Comment on lines 914 to 917
- 'custom': use custom matrices W and H

Ignored if update_H is False, in which case the initial value of W
is set automatically.
Copy link
Member

Choose a reason for hiding this comment

The reason will be disp 8000 layed to describe this comment to others. Learn more.

This may be clearer:

- 'custom': use custom matrices W and H if `update_H=True`. If `update_H=False`, then
  only custom matrix H is used.

The init=='custom' and update_H=False will down the following code path:

scikit-learn/sklearn/decomposition/_nmf.py

Lines 1040 to 1050 in f27adc5

elif not update_H:
_check_init(H, (n_components, n_features), "NMF (input H)")
if H.dtype != X.dtype:
raise TypeError("H should have the same dtype as X. Got H.dtype = "
"{}.".format(H.dtype))
# 'mu' solver should not be initialized by zeros
if solver == 'mu':
avg = np.sqrt(X.mean() / n_components)
W = np.full((n_samples, n_components), avg, dtype=X.dtype)
else:
W = np.zeros((n_samples, n_components), dtype=X.dtype)

thomasjpfan
thomasjpfan approved these changes Jun 2, 2020
View reviewed changes
Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasjpfan thomasjpfan changed the title Clarify how initial conditions are chosen for NMF. DOC Clarify how initial conditions are chosen for NMF. Jun 2, 2020
NicolasHug
NicolasHug approved these changes Jun 3, 2020
View reviewed changes
Copy link
Member
@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

thanks @bsmith89

@NicolasHug NicolasHug merged commit 0e04239 into scikit-learn:master Jun 3, 2020
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
@bsmith89 @viclafargue
DOC Clarify how initial conditions are chosen for NMF. (scikit-learn#…
f0e277f 
…17369)
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
@bsmith89 @jayzed82
DOC Clarify how initial conditions are chosen for NMF. (scikit-learn#…
8c7241b 
…17369)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NicolasHug NicolasHug NicolasHug approved these changes

@thomasjpfan thomasjpfan < 5652 /span> thomasjpfan approved these changes

Assignees
No one assigned
Labels
module:decomposition
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bsmith89 @NicolasHug @thomasjpfan
0