CI: use different seed for github actions #1456
Conversation
also use random seed when running the tests manually
Codecov Report
@@ Coverage Diff @@
## master #1456 +/- ##
==========================================
+ Coverage 84.60% 84.65% +0.05%
==========================================
Files 110 110
Lines 29373 29373
==========================================
+ Hits 24851 24866 +15
+ Misses 4522 4507 -15 see 10 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -3,8 +3,11 @@ using AbstractAlgebra.RandomExtensions: RandomExtensions, make | |||
|
|
|||
| # initialize RNGs to a fixed initial state, to improve reproducibility | |||
| # of this test suite. | |||
| Random.seed!(42) | |||
| const rng = MersenneTwister(42) | |||
| # The seed 22 was chosen to make the tests run more reliably on github actions | |||
There was a problem hiding this comment.
But isn't this just a bandaid? We are not closer to understanding what's going on, are we?
There was a problem hiding this comment.
Yes, but it will at least let the tests succeed in CI so we can detect other issues with nightly or 1.10.
|
OK with the "very bad seed" it hangs in And that's doing Gröbner basis computations... with a rather... simple implementation. These tests look like this: And then similar for a few other domains. I think we should radically reduce these tests, and also switch from random ideals to fixed ideals. The "random" test may or may not be great for smoking out bugs in the code, but this is just to expensive if we accidentally choose a bad ideal. Plus we are not going to use this code in Oscar anyway.... |
|
The real culprit here seems to be |
|
I've now made PR #1458 which "solves" the problematic behavior for me, at least for that one bad seed. It would be interesting to know if there are other even worse seeds for this remaining. And also if this solves the memory usage issue or not ... ? I.e. maybe there are more similarly problematic test cases left... |
|
The bad ideal from seed 8764 seems to be this: and then doing I also think we should probably use fixed ideals. To reduce the complexity we would need to significantly reduce the degrees. |
|
This is obsolete now after #1458. |
I did a few experiments with different seeds to get a very crude memory estimate and using 22 as a seed seems better suited for the github runners than 42 where we did see some failures already. Test-run with seed 22.
I also changed it to use that seed only on CI and always print the seed value, opinions?
I can also add code to parse the seed from ARGS or an environment variable?
PS: The output of the random number generator seems identical for 1.9-1.11 on the supported platforms. Only 1.6 gives different values for a given seed.
Details
A test run with a very bad seed (8764): https://github.com/benlorenz/AbstractAlgebra.jl/actions/runs/6371995051
(This failed even with julia 1.9 on linux and windows, and succeeded after 3 hours on macos with julia 1.9)
The following tests were done locally with a lot of free memory so there is basically no GC pressure and the new GC code (1.10+1.11) does a lot less collections (except during the Weak* tests with the explicit GC calls). Still the difference in memory usage is quite significant.
(At the beginning I did three runs per seed to make sure the values are somewhat consistent)
Max-RSS values at the end of the test-suite for julia-nightly:
Max-RSS values for julia-1.10:
Max-RSS values for julia-1.9: