Skip to content

xorshift7 initialization has multiple flaws #84

@myndzi

Description

@myndzi

Hi there. I wanted to report an unexpected behavior: The code that initializes xorshift7 essentially ignores the first 7 characters of the seed due to an evaluation order? error in the initialization code.

On this line:
https://github.com/davidbau/seedrandom/blob/released/lib/xorshift7.js#L35-L36

(seed.charCodeAt(j) + X[(j + 1) & 7] << 13) is grouped as:
(seed.charCodeAt(j) + X[(j + 1) & 7]) << 13 and not: seed.charCodeAt(j) + (X[(j + 1) & 7] << 13) as appears to be expected. So, for the initial empty array, the value being assigned is NaN << 13 = 0 instead of the (presumably) expected charCode << 13 = charCode.

Edit: This expression also essentially avoids utilizing the bottom 13 bits of the seed space, since the input (seed.charCodeAt(j)) gets shifted left by 13 always, rather than (presumably) set as the bottom bits...

I don't know the algorithm, so perhaps the addition is supposed to occur before the shift, in which case it would need an explicit cast, such as (seed.charCodeAt(j) + (X[(j + 1) & 7]|0) << 13) if not using explicit initialization.

Adding parens in the right place, or initializing the array explicitly to zeroes, would both fix this.

Since the repo seems unmaintained, a workaround for users is to prefix all seeds with some static 8-character prefix:

> xorshift7('hi').double();
0.7404356088362243
> xorshift7('oh no').double();
0.7404356088362243
> xorshift7('7777777hi').double();
0.9368262871086944
> xorshift7('7777777oh no').double();
0.7554118284154382

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions