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 numbers #689

Closed
wants to merge 2 commits into from
Closed

Conversation

jsallay
Copy link
Contributor

@jsallay jsallay commented Oct 28, 2023

Resolve #688

Note that we need to merge #606 beforehand because the armv8 unit test fails if we fix the random numbers.

The range of random numbers for integer tests is not correct.  For
uint64_t it was overflowing and only producing 0's and 1's.

Signed-off-by: John Sallay <[email protected]>
Signed-off-by: John Sallay <[email protected]>
@argilo
Copy link
Member

argilo commented Oct 31, 2023

I already have a pull request open to fix this (and some other bugs in random integer generation): #677

Copy link
Contributor

@jdemel jdemel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jdemel
Copy link
Contributor

jdemel commented Nov 4, 2023

It looks like we need to decide whether we want to merge this PR or #677 . At the moment I would prefer #677 because it addresses a bit more.

@jsallay I hope you don't mind if we close this PR and merge @argilo's PR.

@argilo
Copy link
Member

argilo commented Nov 4, 2023

#677 does include all the improvements made here, but one small difference is that it keeps the int16_t range at -7 .. +7 (for now) since some tests would become flaky if it was increased to the full numeric range of int16_t.

@jsallay
Copy link
Contributor Author

jsallay commented Nov 4, 2023 via email

@jdemel
Copy link
Contributor

jdemel commented Nov 4, 2023

Alright, I'm closing this PR in favor of #677 now.

@jdemel jdemel closed this Nov 4, 2023
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.

Poor random numbers in uint tests.
3 participants