add benchmark script and testing

[JoshuaLampert]

I recently came across AirspeedVelocity.jl and think it is a nice tool to run benchmarks for changes in a PR. The GitHub Action file is (almost) a copy of https://github.com/SymbolicML/DynamicQuantities.jl/blob/21b7468801c773c5072c6db358f2fddcb8529ff9/.github/workflows/benchmark_pr.yml.
For the benchmark test I included some elixirs, which should cover most of the features.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.97%. Comparing base (2061e53) to head (820f2b2).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #158   +/-   ##
=======================================
  Coverage   97.97%   97.97%           
=======================================
  Files          19       19           
  Lines        1776     1776           
=======================================
  Hits         1740     1740           
  Misses         36       36           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JoshuaLampert]

Looks like some test values need to be adjusted. Tests were also failing locally. I used the values I now got locally.

[JoshuaLampert]

I'm not sure why there is the UndefVarError: sol not defined error in the new benchmark Action. Looks like some namespace issue. I guess that's related to the fact that AirspeedVelocity.jl creates a module internally, but I don't know how to fix that. Locally, it runs fine.

[ranocha]

Sounds like a nice idea to try out - if it does not cause too much work/trouble to set it up

[JoshuaLampert]

I have set it up in another repo, which worked easily and nicely. But I'm not sure what to do with the error here.

[github-actions]

Benchmark Results

	main	820f2b2...	main/820f2b2d99ae92...
bbm_1d/bbm_1d_basic.jl	13.9 ± 0.29 μs	13.8 ± 0.29 μs	1.01
bbm_1d/bbm_1d_fourier.jl	0.528 ± 0.0039 ms	0.216 ± 0.015 ms	2.45
bbm_bbm_1d/bbm_bbm_1d_basic_reflecting.jl	0.114 ± 0.0034 ms	0.121 ± 0.0034 ms	0.944
bbm_bbm_1d/bbm_bbm_1d_dg.jl	0.0344 ± 0.00048 ms	0.0341 ± 0.00047 ms	1.01
bbm_bbm_1d/bbm_bbm_1d_relaxation.jl	27.6 ± 0.46 μs	27.4 ± 0.41 μs	1.01
bbm_bbm_1d/bbm_bbm_1d_upwind_relaxation.jl	0.0485 ± 0.00054 ms	0.0485 ± 0.00091 ms	1
hyperbolic_serre_green_naghdi_1d/hyperbolic_serre_green_naghdi_dingemans.jl	4.1 ± 0.012 μs	4.1 ± 0.013 μs	1
serre_green_naghdi_1d/serre_green_naghdi_well_balanced.jl	0.198 ± 0.0074 ms	0.201 ± 0.0062 ms	0.985
svaerd_kalisch_1d/svaerd_kalisch_1d_dingemans_relaxation.jl	0.145 ± 0.0029 ms	0.151 ± 0.0037 ms	0.958
time_to_load	1.98 ± 0.017 s	1.99 ± 0.024 s	0.996

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.

[JoshuaLampert]

Seems like trixi_include is the problem. With include instead it works. But then we cannot set a smaller time span for the ODE.

[JoshuaLampert]

This seems fine now. The plots can be found here.
It's not optimal that all elixirs are run for the whole time span (because trixi_include creates namespace issues in combination with AirspeedVelocity.jl), but with a total runtime of less than 10 minutes I think it's still reasonable.

[ranocha]

Seems like trixi_include is the problem. With include instead it works. But then we cannot set a smaller time span for the ODE.

trixi_include([mod::Module=Main,] elixir::AbstractString; kwargs...) uses Main as module whereas include uses the current module. Can you use @__MODULE__ as first argument to trixi_include?

[JoshuaLampert]

Seems like trixi_include is the problem. With include instead it works. But then we cannot set a smaller time span for the ODE.

trixi_include([mod::Module=Main,] elixir::AbstractString; kwargs...) uses Main as module whereas include uses the current module. Can you use @MODULE as first argument to trixi_include?

Ah, I see. Thanks! Let' see.

[ranocha]

Looks reasonable to me. Benchmarks on GitHub runners can be quite noisy but let's see 👍

[ranocha]

This seems fine now. The plots can be found here. It's not optimal that all elixirs are run for the whole time span (because trixi_include creates namespace issues in combination with AirspeedVelocity.jl), but with a total runtime of less than 10 minutes I think it's still reasonable.

Is there some way to display the plots in a PR comment like the table above?

[JoshuaLampert]

This seems fine now. The plots can be found here. It's not optimal that all elixirs are run for the whole time span (because trixi_include creates namespace issues in combination with AirspeedVelocity.jl), but with a total runtime of less than 10 minutes I think it's still reasonable.

Is there some way to display the plots in a PR comment like the table above?

Good question. I agree that this would be nice. Let me see if I can wangle that.

[JoshuaLampert]

This seems fine now. The plots can be found here. It's not optimal that all elixirs are run for the whole time span (because trixi_include creates namespace issues in combination with AirspeedVelocity.jl), but with a total runtime of less than 10 minutes I think it's still reasonable.

Is there some way to display the plots in a PR comment like the table above?

Good question. I agree that this would be nice. Let me see if I can wangle that.

I think it's not so easy. We would need to upload it somewhere else and then put the link into the comment, cf. peter-evans/create-or-update-comment#68. IMHO, that's a bit too sophisticated. The numbers in the table also show most relevant information. EDIT: We can at least link to the artifact, which saves some clicks, but having the plot inside the comment is not so easy, I think. Artifacts are always uploaded as zip, see https://github.com/actions/upload-artifact?tab=readme-ov-file#zip-archives.
What makes this even harder in the general case is the fact that multiple plots are generated by AirspeedVelocity.jl if there are a lot of benchmarks tests (the default is 10 benchmark tests per image, i.e. we have only one image because we currently have 10 benchmark tests).

[ranocha]

I see. Thanks!