[MRG] Adding variable alphaCorrection to classes in naive_bayes.py. #16747

arka204 · 2020-03-22T19:06:39Z

Reference Issues/PRs

Fixes #10772
References PR #9131
References PR #10775

What does this implement/fix? Explain your changes.

This PR adds a new variable alphaCorrection in classes in naive_bayes.py, which is set to True by default and if set to False, then for alpha=0 (or greater, but still smaller than _ALPHA_MIN) alpha is not being rounded up to _ALPHA_MIN.

Any other comments?

I decided to propose a solution to a problem mentioned in issues above.
This makes possible for user to use functions with smaller alpha, even equal 0, which was earlier impossible, because after setting it to something smaller than _ALPHA_MIN it was changed to _ALPHA_MIN in _check_alpha(self).
By adding a new variable I hope to ensure that small (or equal 0) alpha is used by user intentionally.

jnothman

I think it strange that we should need two parameters to control this. As @amueller wrote in #10775 (comment), going back to just allowing small alpha but making sure there is a warning seems a reasonable way forward. On the other hand, we can't do it without breaking backwards compatibility. So I suppose we could use something akin to this PR as a temporary measure, but change the default of alphaCorrection to False over a deprecation period.

jnothman · 2020-03-25T20:29:14Z

sklearn/naive_bayes.py

+        Additive (Laplace/Lidstone) smoothing parameter
+        (set alpha=0 and alphaCorrection=False, for no smoothing).
+
+    alphaCorrection : bool, default=True


we only use camel case in class names.

Should I change name to: alpha_correction or something like that?

arka204 · 2020-03-27T17:38:34Z

I'm not sure if I don't understand or was misunderstood. Perhaps instead of alphaCorrection I should have used something akin to allow_small_alpha (and set it by default to False)?

Trying to set alpha=0 was resulting in changing it to _ALPHA_MIN in _check_alpha(self).

alphaCorrection=True means we want to have our alpha corrected/adjusted -> set to _ALPHA_MIN when it is lower, so the behavior stays as it was.

On the other hand alphaCorrection=False would mean that it won't be changed/adjusted, thus allowing user to set it to 0 and giving a warning instead of changing it's value to _ALPHA_MIN.

That means if user sets alphaCorrection=True, they would get exact same result as before, only setting it to False changes something.

This is why I cannot understand changing default value of this variable to False.

arka204 · 2020-04-11T13:20:08Z

@jnothman what do You think?

Merging changes from the main repository

arka204 · 2020-04-18T21:11:00Z

@amueller what is Your opinion on this?

Merging changes from the main repository

KumarGanesha1996 · 2020-05-11T16:22:55Z

hello kind sirs... what is status on it? i think you treat it gentleman very bad. please respond to him questions... @jnothman @amueller

cmarmo · 2020-05-12T18:52:06Z

Hi @arka204, your code have a linting issue.
scikit-learn use lint flake8 directives: in order to check your code locally you can run

$ flake8 <your_modified_file>.py

Hope this can help.

cmarmo · 2020-05-22T07:51:50Z

HI @arka204, as long as there are conflicts in your code there is small chance it will be reviewed. Could you please synchronize with the master version of scikit-learn (you can check how to do that at this page), and resolve conflicts? Thanks!

Merging changes from the main repository

Update branch

Resolving conflicts

arka204 · 2020-05-22T21:19:54Z

Thanks for your help @cmarmo !
I changed the naming from alphaCorrection to force_alpha and swapped True with False regarding this variable.
Hope that this will make my code more intuitive and readible.

KumarGanesha1996

well done kind sir

arka204 · 2020-05-30T16:35:21Z

Ping @jnothman, @amueller.
Do You think it can be merged now?

jnothman

Otherwise looking good

jnothman · 2020-05-31T13:16:05Z

sklearn/tests/test_naive_bayes.py

@@ -752,6 +752,17 @@ def test_alpha():
                         X, y, classes=[0, 1])


+def test_check_alpha():


please also test that appropriate warnings are raised in each case (use pytest.warns)

jnothman · 2020-05-31T13:16:23Z

Please add an entry to the change log at doc/whats_new/v0.24.rst. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:

…ka204/scikit-learn into alpha-close-or-equal-0-update

Alpha close or equal 0 update

Update master

…a-is-close-or-equal-0' into master-copy

Update branch

cmarmo · 2020-08-24T20:11:09Z

@arka204 will you be able to address comments and fix conflicts? Thanks for you work!

hongshaoyang · 2020-11-10T09:30:34Z

@arka204 , I've opened #18805 to take over this stalled PR. Thank you.

cmarmo · 2022-05-10T20:27:19Z

Closing as superseded by #18805 and now #22269.

arka204 added 2 commits March 22, 2020 11:06

Adding variable alphaCorrection to classes in naive_bayes.py.

282a7dd

Splitting few lines of code.

d78e17b

github-actions bot added the module:naive_bayes label Mar 22, 2020

jnothman reviewed Mar 25, 2020

View reviewed changes

Merge pull request #1 from scikit-learn/master

3b79637

Merging changes from the main repository

Merge pull request #2 from scikit-learn/master

464dc37

Merging changes from the main repository

arka204 added 2 commits May 21, 2020 21:11

Fixing problems and adding tests.

a4429bf

Updating naive_bayes.py.

cf35eb1

arka204 added 4 commits May 22, 2020 22:09

Merge pull request #3 from scikit-learn/master

15a658f

Merging changes from the main repository

Merge branch 'alpha-close-or-equal-0-update' into alpha-1

ec786b3

Merge pull request #5 from arka204/alpha-1

4606d85

Update branch

Merge pull request #6 from arka204/alpha-close-or-equal-0-update

f0debb6

Resolving conflicts

cmarmo added the Waiting for Reviewer label May 22, 2020

KumarGanesha1996 approved these changes May 24, 2020

View reviewed changes

arka204 changed the title ~~[WIP] Adding variable alphaCorrection to classes in naive_bayes.py.~~ [MRG] Adding variable alphaCorrection to classes in naive_bayes.py. May 30, 2020

jnothman reviewed May 31, 2020

View reviewed changes

arka204 added 2 commits May 31, 2020 20:56

Checkig warnings in tests.

dcce4a8

Merge branch 'alpha-close-or-equal-0-update' of https://github.com/ar…

81d5f32

…ka204/scikit-learn into alpha-close-or-equal-0-update

cmarmo removed the Waiting for Reviewer label Jun 4, 2020

arka204 added 2 commits June 8, 2020 23:46

Merge pull request #8 from arka204/alpha-close-or-equal-0-update

43dfda5

Alpha close or equal 0 update

Update v0.24.rst

be0ebfe

arka204 added 3 commits June 9, 2020 00:06

Merge pull request #10 from scikit-learn/master

2968400

Update master

Merge branch 'Proposition-for-BernoulliNB-and-MultinomialNB-when-alph…

7e1b649

…a-is-close-or-equal-0' into master-copy

Merge pull request #11 from arka204/master-copy

5782e6f

Update branch

hongshaoyang mentioned this pull request Nov 10, 2020

[MRG] Adding variable force_alpha to classes in naive_bayes.py #18805

Closed

cmarmo added the Superseded PR has been replace by a newer PR label Nov 10, 2020

Base automatically changed from master to main January 22, 2021 10:52

Micky774 mentioned this pull request Jan 22, 2022

ENH Adding variable force_alpha to classes in naive_bayes.py #22269

Merged

cmarmo closed this May 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] Adding variable alphaCorrection to classes in naive_bayes.py. #16747

[MRG] Adding variable alphaCorrection to classes in naive_bayes.py. #16747

arka204 commented Mar 22, 2020

jnothman left a comment

jnothman Mar 25, 2020

arka204 Mar 27, 2020

arka204 commented Mar 27, 2020

arka204 commented Apr 11, 2020

arka204 commented Apr 18, 2020

KumarGanesha1996 commented May 11, 2020

cmarmo commented May 12, 2020

cmarmo commented May 22, 2020

arka204 commented May 22, 2020

KumarGanesha1996 left a comment

arka204 commented May 30, 2020

jnothman left a comment

jnothman May 31, 2020

jnothman commented May 31, 2020

cmarmo commented Aug 24, 2020

hongshaoyang commented Nov 10, 2020

cmarmo commented May 10, 2022

		@@ -752,6 +752,17 @@ def test_alpha():
		X, y, classes=[0, 1])


		def test_check_alpha():

[MRG] Adding variable alphaCorrection to classes in naive_bayes.py. #16747

[MRG] Adding variable alphaCorrection to classes in naive_bayes.py. #16747

Conversation

arka204 commented Mar 22, 2020

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

jnothman left a comment

Choose a reason for hiding this comment

jnothman Mar 25, 2020

Choose a reason for hiding this comment

arka204 Mar 27, 2020

Choose a reason for hiding this comment

arka204 commented Mar 27, 2020

arka204 commented Apr 11, 2020

arka204 commented Apr 18, 2020

KumarGanesha1996 commented May 11, 2020

cmarmo commented May 12, 2020

cmarmo commented May 22, 2020

arka204 commented May 22, 2020

KumarGanesha1996 left a comment

Choose a reason for hiding this comment

arka204 commented May 30, 2020

jnothman left a comment

Choose a reason for hiding this comment

jnothman May 31, 2020

Choose a reason for hiding this comment

jnothman commented May 31, 2020

cmarmo commented Aug 24, 2020

hongshaoyang commented Nov 10, 2020

cmarmo commented May 10, 2022