Multinomial Bayes issue #5814

Closed
gg42554 opened this Issue Nov 13, 2015 · 8 comments

Comments

6 participants
@gg42554

gg42554 commented Nov 13, 2015

Given the digits dataset (available in sklearn.datasets), we split it into train and test set.
We fit a MultinomialNB classifier on the train set, and generate predictions on that same train
set. When this is done without smoothing, the classification performance is rather low.
This is contrary to expectations, as the classifier has already seen all possible feature values.
So not including smoothing shouldn't really make such a big difference.

It seems the BernoulliNB class from sklearn also has this problem.

My inefficient but straightforward implementation performs expectedly well on the training set.
Code to reproduce the issue with some more tests is available at http://pastebin.com/2hsrA8xL
I hope the issue is not caused by some trivial implementation detail I've overlooked.

@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort

agramfort Nov 14, 2015

Member
Member

agramfort commented Nov 14, 2015

@gg42554

This comment has been minimized.

Show comment
Hide comment
@gg42554

gg42554 Nov 14, 2015

Might be a silly question but where do i send it :)? The scikit-learn-issues mailing list?
Thanks.

gg42554 commented Nov 14, 2015

Might be a silly question but where do i send it :)? The scikit-learn-issues mailing list?
Thanks.

@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort

agramfort Nov 14, 2015

Member
Member

agramfort commented Nov 14, 2015

@gg42554

This comment has been minimized.

Show comment
Hide comment
@gg42554

gg42554 Nov 15, 2015

Since it's my first time submitting an error report I'm completely lost :(

It says here that PR-s are used to tell others about changes I pushed to a repository. I didn't push anything O_o, nor do I want to. The demo script I submitted (via pastebin link) doesn't fix the issue, or change anything in sklearn code. It's just there to reproduce the issue, so someone who knows the sklearn code can debug it more easily. Perhaps I'm misunderstanding something.

gg42554 commented Nov 15, 2015

Since it's my first time submitting an error report I'm completely lost :(

It says here that PR-s are used to tell others about changes I pushed to a repository. I didn't push anything O_o, nor do I want to. The demo script I submitted (via pastebin link) doesn't fix the issue, or change anything in sklearn code. It's just there to reproduce the issue, so someone who knows the sklearn code can debug it more easily. Perhaps I'm misunderstanding something.

@absolutelyNoWarranty

This comment has been minimized.

Show comment
Hide comment
@absolutelyNoWarranty

absolutelyNoWarranty Nov 19, 2015

Contributor

It seems when alpha=0.0 and the data has features which never change, feature_log_prob_ has -inf which causes the calculations down the line to become all nan.

Given
X = np.array([[1, 0],[1, 1]])
y = np.array([0, 1])

Compare

nb = BernoulliNB(alpha=0.)
nb.fit(X, y)

nb.predict_proba(X)
Out[135]:
array([[ nan,  nan],
       [ nan,  nan]])

nb.feature_log_prob_
Out[139]:
array([[  0., -inf],
       [  0.,   0.]])

and

nb = BernoulliNB(alpha=1e-15)
nb.fit(X, y)
nb.predict_proba(X)

Out[136]:
array([[  1.0000e+00,   8.8818e-16],
       [  1.0000e-15,   1.0000e+00]])
Contributor

absolutelyNoWarranty commented Nov 19, 2015

It seems when alpha=0.0 and the data has features which never change, feature_log_prob_ has -inf which causes the calculations down the line to become all nan.

Given
X = np.array([[1, 0],[1, 1]])
y = np.array([0, 1])

Compare

nb = BernoulliNB(alpha=0.)
nb.fit(X, y)

nb.predict_proba(X)
Out[135]:
array([[ nan,  nan],
       [ nan,  nan]])

nb.feature_log_prob_
Out[139]:
array([[  0., -inf],
       [  0.,   0.]])

and

nb = BernoulliNB(alpha=1e-15)
nb.fit(X, y)
nb.predict_proba(X)

Out[136]:
array([[  1.0000e+00,   8.8818e-16],
       [  1.0000e-15,   1.0000e+00]])

@amueller amueller added the Bug label Sep 14, 2016

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Sep 14, 2016

Member

We should probably throw an error when that happens if that creates issues.

Member

amueller commented Sep 14, 2016

We should probably throw an error when that happens if that creates issues.

@amueller amueller added this to the 0.19 milestone Sep 14, 2016

yl565 added a commit to yl565/scikit-learn that referenced this issue Sep 23, 2016

@yl565

This comment has been minimized.

Show comment
Hide comment
@yl565

yl565 Sep 24, 2016

Contributor

@amueller Please see my PR to this issue.

Contributor

yl565 commented Sep 24, 2016

@amueller Please see my PR to this issue.

yl565 added a commit to yl565/scikit-learn that referenced this issue Oct 17, 2016

herilalaina added a commit to herilalaina/scikit-learn that referenced this issue Jun 15, 2017

@jmschrei jmschrei closed this in #9131 Jun 19, 2017

@jmschrei

This comment has been minimized.

Show comment
Hide comment
@jmschrei

jmschrei Jun 19, 2017

Member

Fixed via #9131

Member

jmschrei commented Jun 19, 2017

Fixed via #9131

jmschrei added a commit that referenced this issue Jun 19, 2017

[MRG+1] Fix MultinomialNB and BernoulliNB alpha=0 bug (continuation) (#…
…9131)

* Fix #5814

* Fix pep8 in naive_bayes.py:716

* Fix sparse matrix incompatibility

* Fix python 2.7 problem in test_naive_bayes

* Make sure the values are probabilities before log transform

* Improve docstring of  `_safe_logprob`

* Clip alpha solution

* Clip alpha solution

* Clip alpha in fit and partial_fit

* Add what's new entry

* Add test

* Remove .project

* Replace assert method

* Update what's new

* Format float into %.1e

* Update ValueError msg

dmohns added a commit to dmohns/scikit-learn that referenced this issue Aug 7, 2017

[MRG+1] Fix MultinomialNB and BernoulliNB alpha=0 bug (continuation) (#…
…9131)

* Fix #5814

* Fix pep8 in naive_bayes.py:716

* Fix sparse matrix incompatibility

* Fix python 2.7 problem in test_naive_bayes

* Make sure the values are probabilities before log transform

* Improve docstring of  `_safe_logprob`

* Clip alpha solution

* Clip alpha solution

* Clip alpha in fit and partial_fit

* Add what's new entry

* Add test

* Remove .project

* Replace assert method

* Update what's new

* Format float into %.1e

* Update ValueError msg

dmohns added a commit to dmohns/scikit-learn that referenced this issue Aug 7, 2017

[MRG+1] Fix MultinomialNB and BernoulliNB alpha=0 bug (continuation) (#…
…9131)

* Fix #5814

* Fix pep8 in naive_bayes.py:716

* Fix sparse matrix incompatibility

* Fix python 2.7 problem in test_naive_bayes

* Make sure the values are probabilities before log transform

* Improve docstring of  `_safe_logprob`

* Clip alpha solution

* Clip alpha solution

* Clip alpha in fit and partial_fit

* Add what's new entry

* Add test

* Remove .project

* Replace assert method

* Update what's new

* Format float into %.1e

* Update ValueError msg

NelleV added a commit to NelleV/scikit-learn that referenced this issue Aug 11, 2017

[MRG+1] Fix MultinomialNB and BernoulliNB alpha=0 bug (continuation) (#…
…9131)

* Fix #5814

* Fix pep8 in naive_bayes.py:716

* Fix sparse matrix incompatibility

* Fix python 2.7 problem in test_naive_bayes

* Make sure the values are probabilities before log transform

* Improve docstring of  `_safe_logprob`

* Clip alpha solution

* Clip alpha solution

* Clip alpha in fit and partial_fit

* Add what's new entry

* Add test

* Remove .project

* Replace assert method

* Update what's new

* Format float into %.1e

* Update ValueError msg

paulha added a commit to paulha/scikit-learn that referenced this issue Aug 19, 2017

[MRG+1] Fix MultinomialNB and BernoulliNB alpha=0 bug (continuation) (#…
…9131)

* Fix #5814

* Fix pep8 in naive_bayes.py:716

* Fix sparse matrix incompatibility

* Fix python 2.7 problem in test_naive_bayes

* Make sure the values are probabilities before log transform

* Improve docstring of  `_safe_logprob`

* Clip alpha solution

* Clip alpha solution

* Clip alpha in fit and partial_fit

* Add what's new entry

* Add test

* Remove .project

* Replace assert method

* Update what's new

* Format float into %.1e

* Update ValueError msg

AishwaryaRK added a commit to AishwaryaRK/scikit-learn that referenced this issue Aug 29, 2017

[MRG+1] Fix MultinomialNB and BernoulliNB alpha=0 bug (continuation) (#…
…9131)

* Fix #5814

* Fix pep8 in naive_bayes.py:716

* Fix sparse matrix incompatibility

* Fix python 2.7 problem in test_naive_bayes

* Make sure the values are probabilities before log transform

* Improve docstring of  `_safe_logprob`

* Clip alpha solution

* Clip alpha solution

* Clip alpha in fit and partial_fit

* Add what's new entry

* Add test

* Remove .project

* Replace assert method

* Update what's new

* Format float into %.1e

* Update ValueError msg

maskani-moh added a commit to maskani-moh/scikit-learn that referenced this issue Nov 15, 2017

[MRG+1] Fix MultinomialNB and BernoulliNB alpha=0 bug (continuation) (#…
…9131)

* Fix #5814

* Fix pep8 in naive_bayes.py:716

* Fix sparse matrix incompatibility

* Fix python 2.7 problem in test_naive_bayes

* Make sure the values are probabilities before log transform

* Improve docstring of  `_safe_logprob`

* Clip alpha solution

* Clip alpha solution

* Clip alpha in fit and partial_fit

* Add what's new entry

* Add test

* Remove .project

* Replace assert method

* Update what's new

* Format float into %.1e

* Update ValueError msg

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

[MRG+1] Fix MultinomialNB and BernoulliNB alpha=0 bug (continuation) (#…
…9131)

* Fix #5814

* Fix pep8 in naive_bayes.py:716

* Fix sparse matrix incompatibility

* Fix python 2.7 problem in test_naive_bayes

* Make sure the values are probabilities before log transform

* Improve docstring of  `_safe_logprob`

* Clip alpha solution

* Clip alpha solution

* Clip alpha in fit and partial_fit

* Add what's new entry

* Add test

* Remove .project

* Replace assert method

* Update what's new

* Format float into %.1e

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