Conversation
…ananigans.jl into ss/improve-benchmarks-pipeline
simone-silvestri
added
the
benchmark performance
|
The benchmarks seem to be fair, as this PR has no effect on the performance results. |
Updated the bottom function to use a fixed denominator for bathymetry calculations and added Random.seed! for consistency in tests.
|
Also, a random bathymetry with values going from -5000 to -1000, as it is right now, is not really representative of an ocean simulation where the bathymetry is somewhat smoother (the benchmarks here are hitting way more reduced advection than what would happen in a real simulation). Therefore, I think it is better to use a sloped bathymetry. |
|
The benchmarks should measure time per time-step (not SYPD). |
|
Where are the benchmarks uploaded? Can we put a link into the documentation or readme or somewhere? |
They do better (for GPU only)! They measure the timing of each kernel independently. For the CPU tests, I have this open PR #4711, which probably still needs some work for the formatting of the output.
No, but we can include them for sure. The tests are run on Tartarus, and a .txt file summarizing the performance of the tested simulation is uploaded as a buildkite artifact. |
How can one view the artifact? Is there documentation or instructions somewhere? |
Why does it matter what the bathymetry is then? |
The tests are run on Tartarus, and a .txt file summarizing the performance of the tested simulation is uploaded as a buildkite artifact. I think they are not really user-friendly, more tests for developers to make sure we don't hit some performance regression: This is an example of the output of the benchmark tests The artifacts are uploaded on the github pipeline (for example) The bathymetry has a role because of the different methods that are used near the immersed boundary which will modify the |
But if we are using non-short-circuiting logic, the same methods are called in every cell regardless of the immersed boundary. |
|
I am not sure that always happens on a GPU; there might be some optimizations that occur under the hood. |
I don't think it's possible because the bathymetry is determined by an array whose values can change / are only known at runtime (not compile time) |
|
Yeah, but at runtime, how CUDA organizes execution might depend on possible zeros. |
|
@navidcy do you know why the documenter key is not found here? |
|
To make Greg happy, we could automatically upload the benchmark results somewhere so that we can produce charts like https://enzymead.github.io/Enzyme-JAX/benchmarks/ and https://enzymead.github.io/Reactant.jl/benchmarks/. Those are using https://github.com/benchmark-action/github-action-benchmark, but we can explore alternative visualisations if you prefer, just to give an idea. This could be another thing I can work on next month, if you're interested |
|
I would be very happy if you would like to take this on! Also, this could help with #4711. I can work on adding some relevant non-hydrostatic benchmarks in another PR (if I can find some time). |
|
I opened #4851 to track this task. |
|
In the meantime, I merge this PR that just changes the initial condition and physical setup slightly. |
3 participants
I am afraid that, given the very low values given to velocity, and an almost constant temperature and salinity, the benchmark tests do not really run in a fair way. I had already noticed a case where the performance depended on the initial conditions (i.e. if your velocities are zeros, the kernel automatically does not compute the reconstructions).
This PR changes the initial conditions of the benchmark tests to make sure they are fair, i.e. a stratified fluid in z with much higher velocity fluctuations that cannot die out in the first iteration