Checking execution time in a test

[JohnAAbbott]

I recently received notification from Github of a failed test. It was in line 26 of normal_toric_varities.jl

        duration = @elapsed stanley_reisner_ideal(ntv5)
        @test duration < 10

Is it appropriate to include checks on execution time in the tests? An "absolute" check such as that above does suggest that there are assumptions about the speed of the underlying compute system (not to mention compilation time and GC time).

[fieker]

On Mon, Jul 10, 2023 at 07:47:21AM -0700, JohnAAbbott wrote: I recently received notification from Github of a failed test. It was in line 26 of `normal_toric_varities.jl` ``` duration = @Elapsed stanley_reisner_ideal(ntv5) @test duration < 10 ``` Is it appropriate to include checks on execution time in the tests? An "absolute" check such as that above does suggest that there are assumptions about the speed of the underlying compute system (not to mention compilation time and GC time).

This is done to catch speed-regressions. So yes, it is a bit deliberate. It would be good to see if this is - a fluke - caused by s.th. makeing it always slower (unintended side effect) - ???

…

-- Reply to this email directly or view it on GitHub: #2540 You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[JohnAAbbott]

The actual duration reported in the error message was about 10.8 (presumably seconds). I suppose the expected time was considerably less than this (e.g. no more than 5 seconds, perhaps even less). Do we know that the execution environments for these tests are all comparably quick?

[benlorenz]

The origin of this test is explained here, where some seemingly unrelated changed changed the running time from a few seconds to several minutes: #1086 (comment)

The current testcase takes about 1-1.5 seconds on my laptop after a fresh start of Oscar (i.e. including all compilation time). Once the compilation is done repeating it takes about 0.2 seconds.

This test has been running quite stable for more than a year and we are now suddenly seeing these timeouts on nightly and 1.10, on multiple branches including master but only on Linux. So there is definitely something wrong now and we are already investigating the reason, see #2539.

[benlorenz]

Similar to the other timing test I have added a repetition to this test, i.e. it will run up to five times and getting below the threshold once suffices for the test to succeed. This should catch jumps in the runtime and avoid GC related spikes. (See #2542)

[JohnAAbbott]

It is probably overkill, but we could compute an "Oscar time unit" by including a simple computation which takes, say, about 1 sec. Then do a relative time comparison like duration/Oscar_time_unit < 10. This would be more portable, but is also extra code/hassle. It is also not a perfect solution.

[fingolfin]

Timings on the VMs used for CI can vary a lot even during a single run, i.e. the VM may get slower/faster over time, due to other workloads running in parallel on the same hardware. Thus I have some doubts whether a "Oscar time unit" would help us there much.

[JohnAAbbott]

I think that the replies from @benlorenz imply that this matter is being (or has been) resolved. Also the comment from @fingolfin shows why my proposal is not a good one. So I shall close the discussion.