limit the scope of srand in tests #16940
Conversation
|
This should probably restore the RNG state even on an exception, to be consistent with the other do-block forms. |
|
Seems useful, but I don't really understand the problem with overloading |
|
Ah yes I didn't think about exceptions, will do. |
|
In the link of my previous comment, Stefan also said:
I think this can be addressed with #16924: we can run systematically the whole testsuite by providing a fixed seed (or a set thereof) on the command line, know to pass (I guess this would entail inserting new tests only at the end of test files). He also conceded:
Running the testsuite a second time without providing any seed (selected then at random) allows to test many different random values. The user facing a failure would be asked to provide the seed (printed on the screen) in her bug report. |
|
Another alternative: putting independant tests in blocks. This is what is already often done with If a particular test fails this allows to debug it more easily (i.e. without having to rerun the whole file) as the seed would be known (by #16924). |
|
could tie this into a feature of testsets. main reason testsets aren't used for base yet is the default printing is too noisy. a custom testset type to keep the output (and parallel execution) more in line with how the current base tests work should be doable, no one has put in the effort to make it happen yet. |
|
Yes I thought also about testests for that "newtest" funcionality, but didn't know about why they aren't used for base, besides the noisy printing. I saw also that it's a bit rigid: the argument of a |
|
If all you need is to preserve the non-deterministic random state, I used the device before in test/random.jl, but extending the interface |
|
This PR is indeed about formalizing this practice of non-determinism preservation with a simple do-block. |
I don't think that has to be set in stone. I guess a question is whether you would really want this scoped RNG seeding feature in contexts other than testing. |
|
I don't have in mind a use case of this feature in contexts other than testing. That said, updating the |
|
+1 for overloading |
|
I guess |
rfourquet
force-pushed
the
rf/srand-tests
branch
2 times, most recently
from
6acfae0
to
3f47b6a
Compare
|
I rebased with the following simplification: I merged
As said above, there are few test files from the "linalg/" folder that I didn't touch, when Note also that it would be possible that some tests start to fail sometimes, if I missed its dependence on a previous Will merge in a few days if no objection. |
rfourquet
force-pushed
the
rf/srand-tests
branch
from
3f47b6a
to
6569594
Compare
rfourquet
force-pushed
the
rf/srand-tests
branch
from
6569594
to
98f0991
Compare
|
Is this essentially an existing bug in the test being exposed by this: https://ci.appveyor.com/project/JuliaLang/julia/build/1.0.19809/job/n6q5jgmeti16sy7m (lots of sprand calls in this file aren't using a fixed seed anymore, the failing one is near Line 1715 in 98f0991 |
|
It's totally possible, even if I didn't think that this change could cause a method error the first time I saw this failure. If the failure is reproducible, this change is likely to be the cause. To test that, one can run |
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at #16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, test are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the seeding; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ```
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at #16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ```
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at JuliaLang#16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at #16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at #16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at #16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at #16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at JuliaLang#16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
[ci skip] [av skip] [bsd skip] This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at #16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at #16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at JuliaLang/julia#16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at JuliaLang/julia#16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
This is a follow-up on the recently introduced `guardsrand` functionality, first suggested at JuliaLang/julia#16940 (comment). Each `testset` block is now implicitly wrapped in a `guardsrand` block, which is more user-friendly and more systematic (only few users know about `guardsrand`). These are essentially two new features: 1) "in": in base, tests are run with the global RNG randomly seeded, but the seed is printed in case of failure to allow reproducing the failure; but even if the failure occurs after many tests, when they alter the global RNG state, one has to re-run the whole file, which can be time-consuming; with this change, at the beginning of each `testset`, the global RNG is re-seeded with its own seed: this allows to re-run only the failing `testset`, which can be done easily in the REPL (the seeding occurs for each loop in a `testset for`; this also allows to re-arrange `testset`s in arbitrary order w.r.t. the global RNG; 2) "out": a `testset` leaves no tracks of its use of `srand` or `rand` (this "feature" should be less and less needed/useful with the generalization of the use of `testset` blocks). Example: ``` @testset begin srand(123) rand() @testset for T in (Int, Float64) # here is an implicit srand(123), by 1) rand() end rand() # this value will not be affected if the sub-`testset` block # above is removed, or if another rand() call is added therein, by 2) end ``` Note that guardsrand can't be used directly, as then the testset's body is wrapped in a function, which causes problem with overwriting loop variable ("outer"), using `using`, defining new methods, etc. So we need to duplicate guardsrand's logic.
(Cf. #8339 for context).
The purpose of this change is to solve the following problem: someone adds a test containg a call to
srand(123)(in order to test a particular configuration). Then every new test added below that one will be "infected" by thissrandcall: the tester will userand()with the intention to test any random value, but didn't notice thatrand()had become deterministic at that point; only one unique value will ever be tested. This limits dramatically the coverage of the tests.An objection against changing this behavior was that with ever-changing random values, a failed test cannot be reproduced: this is addressed by #16924.
I'm not quite sure of what the API should be. Here is the current implemented solution:
A shortcut is
I thought
srandguardis necessary over simply this overload ofsrand, as I think we don't want the following to be validSo an explicit call to
srand()withinguardsrandwould be required.I updated some test files when it's obvious what to do, but almost didn't touch the test files in linalg (not always clear to me what is the intention of
srand). I guess some instances could simply be removed if #16924 gets merged.An alternative simpler implementation could be to have a function
reseed() = srand(GLOBAL_RNG.seed), to close the scope of a previous call tosrand(but I find it less clean).Note: branched off unmerged #16919, only one new commit here.