8000 Documentation for #10772 MultinomialNB alpha>0 by bhaskarb · Pull Request #10775 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Documentation for #10772 MultinomialNB alpha>0 #10775

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
Closed
wants to merge 1 commit into from
Closed

Documentation for #10772 MultinomialNB alpha>0 #10775

wants to merge 1 commit into from

Conversation

bhaskarb
Copy link
@bhaskarb bhaskarb commented Mar 8, 2018 •
edited by rth
Loading

Reference Issues/PRs

Documentation for BernoulliNB and MultinomialNB documentation for alpha=0
Partially fixes issue #10772

What does this implement/fix? Explain your changes.

The MultinomialNB cannot have an alpha = 0, it originally set alpha>=0 and i changed it to alpha>0

Any other comments?

@rth
Copy link
Member
rth commented Mar 8, 2018

Thanks for this PR @bhaskarb

I do not see a reference to the this in the case of BernoulliNB. I was looking at the naive_bayes.rst file in the source.Thanks in advance

scikit-learn/sklearn/naive_bayes.py

Line 645 in 011c0b3

(0 for no smoothing).

also would need to be changed (and same for BernoulliNB) as setting to 0 will raise a warning following #9131 on this line.

But then I'm not sure what should be said there... "set to a small positive number to disable smoothing" doesn't sound right.

I'm not sure if raising a warning for alpha=0 is such a good thing in the first place. On one side, it's understandable to warn the user but it's also unreasobable to expect him/her to enter a small value that is close enough to zero to disable smoothing but large enough to avoid the warning.

I could be worth waiting for a second opinion on this.

@amueller amueller mentioned this pull request Jul 14, 2018
Closed
@amueller
Copy link
Member

I would allow alpha=0 and warn / error if we divide by zero later?

@amueller amueller added the Needs Decision Requires decision label Aug 6, 2019
@arka204 arka204 mentioned this pull request Mar 22, 2020
Closed
@hongshaoyang hongshaoyang mentioned this pull request Nov 10, 2020
Closed
Base automatically changed from master to main January 22, 2021 10:50
@Micky774 Micky774 mentioned this pull request Mar 19, 2022
Merged
@cmarmo cmarmo added Superseded PR has been replace by a newer PR and removed Needs Decision Requires decision labels May 10, 2022
@cmarmo
Copy link
Contributor
cmarmo commented May 10, 2022

Closing as superseded by #22269.

@cmarmo cmarmo closed this May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Superseded PR has been replace by a newer PR
Projects
None yet
Milestone
42A5 No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bhaskarb @rth @amueller @cmarmo
0