Skip to content
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.

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

gammavariate uses misleading parameter names. #104337

Closed
drj11 opened this issue May 9, 2023 · 8 comments
Closed

gammavariate uses misleading parameter names. #104337

drj11 opened this issue May 9, 2023 · 8 comments
Labels
docs Documentation in the Doc dir

Comments

@drj11
Copy link
Contributor

drj11 commented May 9, 2023

Documentation for gammavariate

The parameters of the gammavariate function (in random module) are alpha, beta.

We can see, from the mathematical description given, that these correspond to shape and scale. Wikipedia conventionally describes these using the symbols k and theta. Wikipedia uses beta for rate, which is the reciprocal of scale: beta = 1/theta

This is a proposal to:

  • explain that alpha and beta are also known as shape and scale;
  • reinforce that by adding that μ is proportional to scale;
  • add a warning that these are not the conventional names.

or, more controversially, scrap all that and rename the parameters k and theta. I understand that the parameter names leak into the API, so maybe not this option.

Side note: there is some variability in naming convention. The only textbook i have to hand is Grimmett and Stirzaker "Probability and Random Process" (OUP, 1982). That uses Γ(λ, t) where lambda is rate and t is shape.

(A clear and concise description of the issue.)

Linked PRs

@drj11 drj11 added the docs Documentation in the Doc dir label May 9, 2023
@mdickinson
Copy link
Member

mdickinson commented May 9, 2023

  • add a warning that these are not the conventional names.

I'm curious: what's the algorithm for determining what the conventional names are? The first textbook I checked was DeGroot's "Probability and Statistics" (2nd edn, ISBN 0-201-11366-X), which uses α and β for the parameter names (p.286 onwards). The C++ std. lib. uses alpha and beta (ref). The first hit I got in an arXiv search was this paper, which uses α and β.

@mdickinson
Copy link
Member

mdickinson commented May 9, 2023

Hmm, sorry; I see - there's variation in whether β represents the scale or the inverse scale. Of the three references mentioned above, DeGroot has β being the inverse of the scale, while the C++ std. lib. and the paper referenced have β as the scale parameter. A Google Books search pulled up this book as the first hit, which also uses β as the scale.

So indeed there's variation, but Python's choice doesn't seem to be particularly unusual here. A note following your first suggestion ("explain that alpha and beta are also known as shape and scale") doesn't seem unreasonable, but I'm not seeing a case for the suggested warning (and still less for renaming).

@mdickinson
Copy link
Member

A Google Books search pulled up this book as the first hit, which also uses β as the scale.

Hmm. Forget the book - that doesn't seem to be a good reference. It hedges its bets weirdly by presenting pdfs using β both as a scale parameter and an inverse scale parameter, and seems to interchange between the two inconsistently (e.g., in Ch III, the variance is computed as ɑβ²).

@mdickinson
Copy link
Member

mdickinson commented May 9, 2023

One more data point: here's Wolfram Alpha:
Screenshot 2023-05-09 at 20 54 30

And one more (though it's not one I'd give much weight to): whatever sources ChatGPT was trained on also seem to think of β as a scale parameter:

Screenshot 2023-05-09 at 20 58 28

@drj11
Copy link
Contributor Author

drj11 commented May 10, 2023

You seem to have resolved most issues whilst i was asleep.

Agreed that this use of alpha and beta fall within existing accepted use, and therefore on not changing the names. Agreed (obviously) that adding a note relating to shape and scale would be useful.

As for convention, i didn't claim any particular convention, i just deferred to Wikipedia (which of course, you are welcome to edit). Although i do claim that i was mislead.

@terryjreedy
Copy link
Member

I propose that in https://github.com/python/cpython/blob/main/Doc/library/random.rst we replace

Conditions on the parameters are alpha > 0 and beta > 0.

with

The shape and scale parameters alpha and beta, must get positive values. (Confusingly, 'beta' is sometimes used instead for the rate parameter 1 / scale.)

@rhettinger ?

@rhettinger
Copy link
Contributor

One other source for comparison: https://support.microsoft.com/en-us/office/gamma-dist-function-9b6f1538-d11c-4d5f-8966-21f6a2201def The MS Excel docs are precise because they show the underlying probability density functions.

The suggested edit looks fine to me though I might have chosen different wording:

- The shape and scale parameters alpha and beta, must get positive values.
+ The shape and scale parameters, alpha and beta, must have positive values.
- (Confusingly, 'beta' is sometimes used instead for the rate parameter 1 / scale.)
+ (Calling conventions vary and some sources define 'beta' as the inverse of the scale).

@terryjreedy
Copy link
Member

PR with Raymond's improvements upon my draft.

terryjreedy added a commit that referenced this issue May 14, 2023
* gh-104337: Clarify random.gammavariate doc entry

* Fix parameter markup.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 14, 2023
* pythongh-104337: Clarify random.gammavariate doc entry

* Fix parameter markup.
(cherry picked from commit 88c5c58)

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
terryjreedy added a commit that referenced this issue May 14, 2023
…104481)

(cherry picked from commit 88c5c58)

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
carljm added a commit to carljm/cpython that referenced this issue May 15, 2023
* main: (29 commits)
  pythongh-101819: Fix _io clinic input for unused base class method stubs (python#104418)
  pythongh-101819: Isolate `_io` (python#101948)
  Bump mypy from 1.2.0 to 1.3.0 in /Tools/clinic (python#104501)
  pythongh-104494: Update certain Tkinter pack/place tests for Tk 8.7 errors (python#104495)
  pythongh-104050: Run mypy on `clinic.py` in CI (python#104421)
  pythongh-104490: Consistently define phony make targets (python#104491)
  pythongh-67056: document that registering/unregistering an atexit func from within an atexit func is undefined (python#104473)
  pythongh-104487: PYTHON_FOR_REGEN must be minimum Python 3.10 (python#104488)
  pythongh-101282: move BOLT config after PGO (pythongh-104493)
  pythongh-104469 Convert _testcapi/float.c to use AC (pythongh-104470)
  pythongh-104456: Fix ref leak in _ctypes.COMError (python#104457)
  pythongh-98539: Make _SSLTransportProtocol.abort() safe to call when closed (python#104474)
  pythongh-104337: Clarify random.gammavariate doc entry  (python#104410)
  Minor improvements to typing docs (python#104465)
  pythongh-87092: avoid gcc warning on uninitialized struct field in assemble.c (python#104460)
  pythonGH-71383: IDLE - Document testing subsets of modules (python#104463)
  pythongh-104454: Fix refleak in AttributeError_reduce (python#104455)
  pythongh-75710: IDLE - add docstrings and comments to editor module (python#104446)
  pythongh-91896: Revert some very noisy DeprecationWarnings for `ByteString` (python#104424)
  Add a mention of PYTHONBREAKPOINT to breakpoint() docs (python#104430)
  ...
carljm added a commit to carljm/cpython that referenced this issue May 15, 2023
* main: (204 commits)
  pythongh-101819: Fix _io clinic input for unused base class method stubs (python#104418)
  pythongh-101819: Isolate `_io` (python#101948)
  Bump mypy from 1.2.0 to 1.3.0 in /Tools/clinic (python#104501)
  pythongh-104494: Update certain Tkinter pack/place tests for Tk 8.7 errors (python#104495)
  pythongh-104050: Run mypy on `clinic.py` in CI (python#104421)
  pythongh-104490: Consistently define phony make targets (python#104491)
  pythongh-67056: document that registering/unregistering an atexit func from within an atexit func is undefined (python#104473)
  pythongh-104487: PYTHON_FOR_REGEN must be minimum Python 3.10 (python#104488)
  pythongh-101282: move BOLT config after PGO (pythongh-104493)
  pythongh-104469 Convert _testcapi/float.c to use AC (pythongh-104470)
  pythongh-104456: Fix ref leak in _ctypes.COMError (python#104457)
  pythongh-98539: Make _SSLTransportProtocol.abort() safe to call when closed (python#104474)
  pythongh-104337: Clarify random.gammavariate doc entry  (python#104410)
  Minor improvements to typing docs (python#104465)
  pythongh-87092: avoid gcc warning on uninitialized struct field in assemble.c (python#104460)
  pythonGH-71383: IDLE - Document testing subsets of modules (python#104463)
  pythongh-104454: Fix refleak in AttributeError_reduce (python#104455)
  pythongh-75710: IDLE - add docstrings and comments to editor module (python#104446)
  pythongh-91896: Revert some very noisy DeprecationWarnings for `ByteString` (python#104424)
  Add a mention of PYTHONBREAKPOINT to breakpoint() docs (python#104430)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

No branches or pull requests

4 participants