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

Fix random seeding #77

Merged
merged 1 commit into from
Jan 6, 2025
Merged

Fix random seeding #77

merged 1 commit into from
Jan 6, 2025

Conversation

mikeyford
Copy link
Contributor

@mikeyford mikeyford commented Nov 19, 2024

See monome/teletype#337

The use of a constant to initialize z meant the first random values aren't sufficiently different for similar seeds. I'm not very handy with C, so a Python script was easier to demonstrate:

class RandomState:
    def __init__(self, seed):
        self.z = 12345  
        self.w = seed  

    def random_next(self):
        self.z = 36969 * (self.z & 0xFFFF) + (self.z >> 16)
        self.w = 18000 * (self.w & 0xFFFF) + (self.w >> 16)
        return ((self.z << 16) + self.w) & 0x7FFFFFFF

for seed in range(10):
    rand = RandomState(seed)
    print(rand.random_next())

returns

1465974784
1465992784
1466010784
1466028784
1466046784
1466064784
1466082784
1466100784
1466118784
1466136784

The values aren't exactly the same, but are similar. This effect disappears on further iterations of the initial seed. In specific situations like PN.RND where we take the mod of the random value this manifests as the same value each time on the first call. For ops like RAND 10000 the initial values were different on reseeding, but tended to be clustered.

Updating to the change in this PR:

class RandomState:
    def __init__(self, seed):
        self.z = ~seed  
        self.w = seed  

and re-running we get the values

1872101376
1596802640
1321503904
1046205168
770906432
495607696
220308960
2092493872
1817195136
1541896400

Built firmware and tested in Teletype hardware.

@tehn tehn merged commit 6d9c2ca into monome:main Jan 6, 2025
@tehn
Copy link
Member

tehn commented Jan 6, 2025

Thanks for spotting this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants