Make random consistent: never output the top of the bound #37
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
idbrii
changed the title
Fix lume.trace test fails on windows because "c:/blah.lua:34 boop" matches `c` as the file and `/blah.lua` as the line. I can't see how line numbers might not be digits (floating point or hex line numbers?).
Conform to the same range regardless of inputs. Changes the single
argument version of random to use the same bounds as the other two.
lume.random(), lume.random(1), lume.random(0,1) should all mean the same
thing: output a number in [0,1). They didn't all produce the same
results. The middle option would produce results in (0,1].
Add a test to try some iterations to test failures. An older version of
this test caught errors for the single argument version, but with lua54
I no longer see those failures (the sequence of numbers of the same seed
differs) so I've reworked the test.
Don't use testeq for each iteration like this:
testeq(0 <= a, true)
testeq(a < 1, true)
Because all that printing is very slow.
All tests pass.
idbrii
force-pushed
the
rxi-fix-random
branch
from
f63d00c
to
0da79f9
Compare
|
Updated to change my test because it was failing on lua5.4. Now I'm no longer expecting any specific values from random since the rng doesn't produce the same sequence it did back on lua 5.1 (or whatever I was using). Also added a commit to fix lume.trace test on Windows. |
jaythomas
approved these changes
Nov 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
lume.random(1)can sometimes return 1, butlume.random()andlume.random(0,1)will never return 1. (And the converse for zero: they should all be able to produce 0.) Change the single argument version to conform to the same range regardless of inputs. Update the documentation to make the output range explicit using language similar to math.random.Add a test that had two failures on the old code for the single argument
version:
FAIL test.lua:110: testeq(postcond.expected.count > 0, true)
Expected "true" got "false"
FAIL test.lua:111: testeq(postcond.unexpected.count, 0)
Expected 0.00 got 3.00
All tests pass.