gh-149221:Fix binomialvariate Function for random module

[lighting9999]

Fix:
Replace _log2(random()) with _log2(1.0 - random()). This matches the standard algorithm (1.0 - random() is never zero), produces the correct distribution, and eliminates the crash.

• Issue: random module binomialvariate function returns wrong results #149221

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[lighting9999]

一位Python核心开发人员要求在考虑合并之前对你的拉取请求做一些修改。如果你能回应他们的请求，以及核心开发者在其他评测中提出的任何请求，我将非常感激。

完成请求修改后，请在本拉取请求中留言，包含短语。然后我会通知所有留下评价的核心开发者，你准备好让他们重新审视这个拉取请求。I have made the requested changes; please review again

I have made the requested changes; please review again.

[bedevere-app]

Thanks for making the requested changes!

@rhettinger: please review the changes made to this pull request.

[skirpichev]

I think it's not too hard to find a seed, that trigger an exception. Maybe issue worth a test?

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #32506794 | 📁 Comparing e315b38 against main (be9c7cb)

  🔍 Preview build  

30 files changed · ± 30 modified

± Modified

• contents.html
• download.html
• genindex-B.html
• genindex-C.html
• genindex-E.html
• genindex-H.html
• genindex-I.html
• genindex-L.html
• genindex-O.html
• genindex-P.html
• and 20 more...

[lighting9999]

ok @skirpichev ,Please review.

[skirpichev]

I think you can do a quick exit:

Suggested change

	if c < 0:
	# When c < 0, log(0) mathematically gives +infinity, so y > n.
	y = n + 1
	return x

Remember, c < 0 at this point.

stale

[rhettinger]

@skirpichev Can you let me focus on this one? We're pushing the OP in two different directions.

[rhettinger]

This is out of date.

[rhettinger]

Please just turn the handler into a continue for a reselection. Jumping to infinity isn't desirable. I don't want to distort the distribution.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[skirpichev]

Can you let me focus on this one? We're pushing the OP in two different directions.

Sure, @rhettinger, sorry for the noise. Though, I think the only difference is that you suggest to use "continue" statement instead of "return x".

[rhettinger]

This is all that is needed:

diff --git a/Lib/random.py b/Lib/random.py
index c89cbb755ab..2460ac3d986 100644
--- a/Lib/random.py
+++ b/Lib/random.py
@@ -836,7 +836,11 @@ def binomialvariate(self, n=1, p=0.5):
             if not c:
                 return x
             while True:
-                y += _floor(_log2(random()) / c) + 1
+                try:
+                    y += _floor(_log2(random()) / c) + 1
+                except ValueError:
+                    # Handle rare case of where random() gives 0.0
+                    continue
                 if y > n:
                     return x
                 x += 1

[lighting9999]

This is all that is needed:

diff --git a/Lib/random.py b/Lib/random.py
index c89cbb755ab..2460ac3d986 100644
--- a/Lib/random.py
+++ b/Lib/random.py
@@ -836,7 +836,11 @@ def binomialvariate(self, n=1, p=0.5):
             if not c:
                 return x
             while True:
-                y += _floor(_log2(random()) / c) + 1
+                try:
+                    y += _floor(_log2(random()) / c) + 1
+                except ValueError:
+                    # Handle rare case of where random() gives 0.0
+                    continue
                 if y > n:
                     return x
                 x += 1

What I originally had in mind was this:

u = random()
if u == 0.0:
    continue
y += _floor(_log2(u) / c) + 1

No try-except, just a simple check and skip. @rhettinger

[rhettinger]

I prefer the zero-cost try/except to not slow down the loop.

[lighting9999]

I prefer the zero-cost try/except to not slow down the loop.

Ok wait to change

[lighting9999]

I prefer the zero-cost try/except to not slow down the loop.

please review this @rhettinger.