Skip to content

Navigation Menu

Sign in
Appearance settings
Sign up
Appearance settings

BUG: random: biased samples from integers() with 8 or 16 bit dtype. - #14781

#14781
Merged
charris merged 1 commit into
numpy:maintenance/1.17.xnumpy/numpy:maintenance/1.17.xfrom
charris:backport-14777charris/numpy:backport-14777Copy head branch name to clipboard
Oct 25, 2019
Merged

BUG: random: biased samples from integers() with 8 or 16 bit dtype.#14781
charris merged 1 commit into
numpy:maintenance/1.17.xnumpy/numpy:maintenance/1.17.xfrom
charris:backport-14777charris/numpy:backport-14777Copy head branch name to clipboard

Conversation

@charris

@charris charris commented Oct 25, 2019

Copy link
Copy Markdown
Member

Backport of #14777.

When an 8 or 16 bit dtype was given to the integers() method of the
Generator class, the resulting sample was biased. The problem was
the lines of the form

const uint8_t threshold = -rng_excl % rng_excl;

in the implementations of Lemire's method, in the C file
distributions.c. The intent was to compute
(UINT8_MAX+1 - rng_excl) % rng_excl
However, when the type of rng_excl has integer conversion rank lower
than a C int (which is almost certainly the case for the 8 and 16
bit types), the terms in the expression -rng_excl % rng_excl are
promoted to int, and the result of the calculation is always 0.

The fix is to make the expression explicit, and write it as

const uint8_t threshold = (UINT8_MAX - rng) % rng_excl;

rng is used, because rng_excl is simply rng + 1; by using rng, we
we only need the constant UINT#_MAX, without the extra +1.

For consistency, I made the same change for all the data types
(8, 16, 32 and 64 bit).

Closes gh-14774.

When an 8 or 16 bit dtype was given to the integers() method of the
Generator class, the resulting sample was biased.  The problem was
the lines of the form

    const uint8_t threshold = -rng_excl % rng_excl;

in the implementations of Lemire's method, in the C file
distributions.c.  The intent was to compute
    (UINT8_MAX+1 - rng_excl) % rng_excl
However, when the type of rng_excl has integer conversion rank lower
than a C int (which is almost certainly the case for the 8 and 16
bit types), the terms in the expression -rng_excl % rng_excl are
promoted to int, and the result of the calculation is always 0.

The fix is to make the expression explicit, and write it as

    const uint8_t threshold = (UINT8_MAX - rng) % rng_excl;

rng is used, because rng_excl is simply rng + 1; by using rng, we
we only need the constant UINT#_MAX, without the extra +1.

For consistency, I made the same change for all the data types
(8, 16, 32 and 64 bit).

Closes numpygh-14774.
@charris charris added this to the 1.17.4 release. milestone Oct 25, 2019
@charris
charris merged commit a8cf76a into numpy:maintenance/1.17.x Oct 25, 2019
@charris
charris deleted the backport-14777 branch November 7, 2019 20:56
iyanmv added a commit to iyanmv/galois that referenced this pull request Nov 16, 2021
NumPy 1.17.4 fixed biased generation of 8 and 16 bit integers.

Refs:
 - https://numpy.org/devdocs/release/1.17.4-notes.html
 - numpy/numpy#14781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.