Speed up coverage tests

[efaulhaber]

Closes #948.

Comparison of the latest run on main and this PR:

	main test time	main total time	new test time without coverage	new test time with coverage	new total time
tree_part1	36m 47s	42m 34s	19m 15s	19m 50s	46m 19s
tree_part2	55m 1s	1h 1m 48s	14m 42s	38m 57s	59m 34s
tree_part3	57m 15s	1h 2m 49s	12m 0s	38m 33s	56m 28s
tree_part4	53m 38s	1h 0m 29s	10m 33s	29m 41s	46m 34s
tree_part5	38m 11s	44m 5s	9m 40s	25m 40s	41m 30s
tree_part6	57m 7s	1h 2m 47s	10m 26s	20m 23s	37m 16s
structured	1h 43m 46s	1h 50m 1s	14m 38s	1h 1m 10s	1h 21m 31s
p4est_part1	42m 18s	49m 8s	14m 11s	19m 58s	41m 26s
p4est_part2	1h 58m 18s	2h 4m 48s	14m 56s	55m 36s	1h 17m 36s
unstructured_dgmulti	44m 30s	51m 6s	15m 4s	34m 15s	54m 47s
paper_self_gravitating _gas_dynamics	43m 10s	49m 48s	8m 38s	12m 20s	26m 48s
misc_part1	1h 30m 48s	1h 36m 25s	20m 9s	1h 39m 7s	2h 5m 2s
misc_part2	30m 38s	37m 30s	12m 47s	12m 38s	32m 18s
mpi (Ubuntu)	23m 38s	30m 37s	12m 19s	14m 19s	33m 17s
threaded (Ubuntu)	13m 58s	21m 16s	14m 0s	8m 46s	29m 14s

Note that the misc_part2 timings are not from the latest CI run of this PR but from the one before that. The two runs before the latest one both took about 30 minutes, while the latest one took over one hour. I didn't change anything on this job since then. This job seems to vary greatly between runs for some reason. In this CI run on main, the misc_part2 job took over 2 hours, for example.

The greatest improvements can be seen in the jobs structured, p4est_part2, and paper_self_gravitating_gas_dynamics.
The job paper_self_gravitating_gas_dynamics contains some long-running tests that are now significantly faster without coverage and can be reduced to a few time steps when running with coverage. The jobs structured and p4est_part2 were the slowest, so I optimized a few tests by hand (for example, letting the polydeg=5 tests run with polydeg=3 instead in the coverage tests greatly reduced the test time, probably because the recompilation for polydeg=5 now disappears).

In general, this PR allows running more complicated non-coverage tests without greatly increasing the CI times. It also allows optimizing coverage times even more, as I did (roughly) for structured and p4est_part2.

[codecov]

Codecov Report

Merging #954 (d5d6cbb) into main (1d2fc8d) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #954      +/-   ##
==========================================
- Coverage   93.79%   93.79%   -0.00%     
==========================================
  Files         284      284              
  Lines       20615    20616       +1     
==========================================
  Hits        19335    19335              
- Misses       1280     1281       +1     

Flag	Coverage Δ
unittests	93.79% <100.00%> (-<0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/callbacks_step/glm_speed.jl	73.33% <100.00%> (+0.92%)	⬆️
...discretization/semidiscretization_euler_gravity.jl	90.86% <0.00%> (-0.51%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d2fc8d...d5d6cbb. Read the comment docs.

[efaulhaber]

Preliminary results from my first run: The job p4est_part2 has been sped up from 1h 48m to 1h 18m.

While this is a significant speedup, it's still of the same order. However, this approach has two big advantages:

1. This approach is fail-fast. The tests without coverage are done within the first ~20 minutes. If this step fails, the job will fail and not run the coverage step. Note that after a failing test, coverage results haven't been reported before anyway.
2. We can easily let the simulations run for more time steps without significantly slowing down the pipeline.

[efaulhaber]

Here's my suggestion to make tests even faster (writing that here, so it doesn't get lost in the review comments):
Instead of writing

@test_trixi_include(joinpath(EXAMPLES_DIR, "elixir_advection_basic.jl"),
      l2   = [8.311947673061856e-6],
      linf = [6.627000273229378e-5],
      maxiters_coverage = 10)

to run this test with 10 iterations in the coverage run, we could do something like this:

@test_trixi_include(joinpath(EXAMPLES_DIR, "elixir_advection_basic.jl"),
      l2   = [8.311947673061856e-6],
      linf = [6.627000273229378e-5],
      coverage_kwargs = Dict(maxiters => 10, cells_per_dimension => (2, 2, 2), polydeg => 2))

If no coverage_kwargs are specified, or a Dict that doesn't contain maxiters, a maxiters = 1 is added by default.
The coverage_kwargs would then be ignored for normal tests, but in coverage tests, the corresponding kwargs are applied (overwritten). This would allow us to easily tweak more complicated tests to run with fewer cells and/or a lower polydeg, or even a shorter AMR interval to be able to use less iterations.

We could also do the same for convergence_test to run convergence tests with fewer cells for the first iteration and with fewer iterations in general.

[ranocha]

Sounds good at first 👍 We need to realize that some tests will be really heavy, e.g., if they need to good resolution etc. to run at all because of positivity issues or something like that. Thus, we might want an option to disable some tests for coverage

[sloede]

Sounds good to me too, at first glance!

🚲🏠ing:

• I'd not use coverage_kwargs since you can also override regular variables. Thus I suggest to use something like either coverage_override or just coverage.

• Unless there are technical/performance reasons against it, I like named tuples better than Dicts since they are easier to type and read IMHO. Compare

  Dict(maxiters => 10, cells_per_dimension => (2, 2, 2), polydeg => 2)

  vs

  (maxiters = 10, cells_per_dimension = (2, 2, 2), polydeg = 2)

[efaulhaber]

There is now only one line uncovered that was covered in main. This one's weird:

This is `src/semidiscretization/semidiscretization_euler_gravity.jl`.

[ranocha]

That's okay - inlined function definitions are not reported as covered, see #841

[ranocha]

Great work, @efaulhaber 👍

Do you see additional potential to reduce the CI times of misc_part1? It looks like some of the tests like 3D plotting and time series callback take quite long with coverage.

[efaulhaber]

Do you see additional potential to reduce the CI times of misc_part1? It looks like some of the tests like 3D plotting and time series callback take quite long with coverage.

I haven't looked into this, which means that it probably has a lot of potential for optimization.

[efaulhaber]

Any better name ideas for these guys?

[sloede]

Why are there two arguments? This seems like a recipe for confusion in case they are both false. Also, having two keywords increases the chance that someone makes a silent typo that is never found, e.g., when using run_without_coverge=false.

[ranocha]

Aren't these kind of tests that we want to drop easier to discard at a higher level? For example, we could check whether coverage is turned on and avoid including the threaded tests if so. Similarly, we will probably need special cases for AD tests etc. Thus, I think we should probably remove these keyword arguments again and extend this PR or merge this PR (if @sloede approves) and make another PR to improve the handling of expensive misc_partx tests.

[sloede]

I agree. Merging the first part that is already looking OK and then addressing other issues in a second PR seems like a good approach!

[efaulhaber]

I tried to avoid having some magic values (:both, coverage, ...) and to use binary switches instead. Anyway, I'm removing it again.

[ranocha]

CI / p4est_part2 - ubuntu-latest - x64 - pull_request (pull_request) Successful in 139m became expensive again?

[efaulhaber]

CI / p4est_part2 - ubuntu-latest - x64 - pull_request (pull_request) Successful in 139m became expensive again?

I think that's just the runner being terribly slow this time. The first elixir_advection_basic.jl, which usually takes ~300s due to compilation time, took about 40 minutes this time.

[efaulhaber]

Okay, I have no idea why p4est_part2 is so slow again. I haven't changed anything since it was fast.