Skip to content

Documentation for #10772 MultinomialNB alpha>0#10775

Closed
bhaskarb wants to merge 1 commit intoscikit-learn:mainfrom
bhaskarb:my-feature
Closed

Documentation for #10772 MultinomialNB alpha>0#10775
bhaskarb wants to merge 1 commit intoscikit-learn:mainfrom
bhaskarb:my-feature

Conversation

@bhaskarb
Copy link
Copy Markdown

@bhaskarb bhaskarb commented Mar 8, 2018

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
Copy Markdown
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

(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
Copy link
Copy Markdown
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
Base automatically changed from master to main January 22, 2021 10:50
@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
Copy Markdown
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

Labels

Superseded PR has been replace by a newer PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants