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

Current Xoshiro128ss implementation is based on an incorrect version 1.0 #8

Closed
rinart73 opened this issue Jul 23, 2024 · 1 comment · Fixed by #9
Closed

Current Xoshiro128ss implementation is based on an incorrect version 1.0 #8

rinart73 opened this issue Jul 23, 2024 · 1 comment · Fixed by #9
Assignees
Labels
bug Something isn't working

Comments

@rinart73
Copy link

Current library implementation: https://github.com/michaeldzjap/rand-seed/blob/develop/src/Algorithms/Xoshiro128ss.ts#L40
Current version 1.1 written in C: https://xoshiro.di.unimi.it/xoshiro128starstar.c

There are 2 major differences:

  1. Version 1.0 generated new number based on s[0] (this.a), rather than s[1] (this.b). This is explicitly marked as a mistake that they fixed:

Note that version 1.0 had mistakenly s[0] instead of s[1] as state word passed to the scrambler.

  1. Rotl operation performed on generated number r has incorrect order of operations:
// current
r = (r << 7) | ((r >>> 25) * 9);
// proper
r = ((r << 7) | (r >>> 25)) * 9;

With changes applied new implementation will of course generate different numbers for the same seeds, which means it's not backwards compatible.

@michaeldzjap michaeldzjap added the bug Something isn't working label Jul 24, 2024
@michaeldzjap michaeldzjap added this to the Next Major Release milestone Jul 24, 2024
@michaeldzjap
Copy link
Owner

Thanks for reporting @rinart73 . Will pick this up as soon as I have time.

@michaeldzjap michaeldzjap self-assigned this Jul 24, 2024
@michaeldzjap michaeldzjap linked a pull request Jul 31, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants