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

[MRG+1] Fix underflow in nmf._beta_divergence #10142

Merged
merged 1 commit into from Nov 16, 2017

Conversation

Projects
None yet
3 participants
@TomDLT
Member

TomDLT commented Nov 15, 2017

Fix an underflow when computing the Kullback-Leibler divergence in nmf._beta_divergence.

We want to compute sum(X * log(X / WH)), which is zero where X is zero.
So we compute it only where X != 0.
But if X is very small, we can have an underflow: X != 0 but X / WH == 0.

The proposed fix is to compute the expression only where X > epsilon.


To reproduce the underflow:

import numpy as np
from sklearn.decomposition import nmf
rng = np.random.RandomState(0)
n_samples, n_features, n_components = 10, 2, 2
X = np.abs(rng.randn(n_samples, n_features)) * 10
W = np.abs(rng.randn(n_samples, n_components)) * 10
H = np.abs(rng.randn(n_components, n_features))

X[0, 0] = 0
ref = nmf._beta_divergence(X, W, H, beta=1.0)
X[0, 0] = 1e-323
res = nmf._beta_divergence(X, W, H, beta=1.0)
print(res, ref)
# /cal/homes/tdupre/work/src/scikit-learn/sklearn/decomposition/nmf.py:129:
# RuntimeWarning: divide by zero encountered in log
# -inf 316.590497013
@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Nov 15, 2017

Member

LGTM

Member

jnothman commented Nov 15, 2017

LGTM

@jnothman jnothman changed the title from [MRG] Fix underflow in nmf._beta_divergence to [MRG+1] Fix underflow in nmf._beta_divergence Nov 15, 2017

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Nov 15, 2017

Member

Thanks for the clear explanation!

Member

jnothman commented Nov 15, 2017

Thanks for the clear explanation!

@agramfort agramfort merged commit 2160ea0 into scikit-learn:master Nov 16, 2017

6 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 96.21%)
Details
codecov/project 96.21% (+<.01%) compared to 963dd18
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details
@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort
Member

agramfort commented Nov 16, 2017

thx @TomDLT

jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017

@TomDLT TomDLT deleted the TomDLT:nmf_underflow branch Jun 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment