format examples

[ranocha]

It would be nice if several people could review this PR, e.g., @trixi-framework/developers

TODO

• update style guide

[codecov]

Codecov Report

Attention: 1145 lines in your changes are missing coverage. Please review.

Comparison is base (08a2e09) 82.77% compared to head (541a510) 72.65%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1531       +/-   ##
===========================================
- Coverage   82.77%   72.65%   -10.12%     
===========================================
  Files         431      431               
  Lines       34647    34637       -10     
===========================================
- Hits        28678    25164     -3514     
- Misses       5969     9473     +3504     

Flag	Coverage Δ
unittests	72.65% <33.12%> (-10.12%)	⬇️

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

Files	Coverage Δ
examples/dgmulti_2d/elixir_euler_bilinear.jl	100.00% <100.00%> (ø)
...les/dgmulti_2d/elixir_euler_brown_minion_vortex.jl	100.00% <100.00%> (ø)
examples/dgmulti_2d/elixir_euler_curved.jl	100.00% <100.00%> (ø)
...ti_2d/elixir_euler_kelvin_helmholtz_instability.jl	100.00% <100.00%> (ø)
...lti_2d/elixir_euler_rayleigh_taylor_instability.jl	100.00% <100.00%> (ø)
...s/dgmulti_2d/elixir_euler_shockcapturing_curved.jl	100.00% <100.00%> (ø)
examples/dgmulti_2d/elixir_euler_weakform.jl	100.00% <100.00%> (ø)
examples/dgmulti_2d/elixir_mhd_reflective_wall.jl	100.00% <100.00%> (ø)
examples/dgmulti_3d/elixir_euler_curved.jl	100.00% <100.00%> (ø)
...les/dgmulti_3d/elixir_euler_taylor_green_vortex.jl	100.00% <100.00%> (ø)
... and 178 more

... and 54 files with indirect coverage changes

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

[jlchan]

Looked over all of the dgmulti_1d and 2d elixirs. A few minor comments but nothing too major.

[sloede]

I've randomly looked at a number of files from different mesh types. Mostly, it seems like everything is as good or as bad as before, sometimes there are slight improvements

In some cases (already noted by @JoshuaLampert @SimonCan @jlchan), there exists a somewhat weird formatting with binary operators being alone on one line. This is obviously a decline in readability. However, they are fixable (by moving the binary op to the previous line). Thus, from my side there are no hard reasons to not proceed with merging this once everyone had.a chance to review (and improve) things

[ranocha]

I updated this PR based on the recent main. I also added the workaround for the weird formatting of binary operators everywhere. Some stochastic last reviews would be nice before merging this when tests pass.

There are some instances where the arguments x, t are on separate lines when defining boundary conditions - but that's necessary given our line length limit we use right now. The other cases with t, alone on a separate line are fixed.

[sloede]

I did some spot checks but nothing rang besides that, like for the test/ folder, this reformatting is unfortunately not a clear improvement for readability but rather a lateral move. I still think it is useful to do!

It would be good if maybe @JoshuaLampert or @lchristm could have a look as well, maybe also @DanielDoehring? Not at everything, of course, but at least at a few random elixirs...

[DanielDoehring]

Looks alright to me, we loose some "nice" formatting like

coordinates_min = (-1.0, -1.0) # minimum coordinates (min(x), min(y))
coordinates_max = ( 1.0,  1.0) # maximum coordinates (max(x), max(y))

which becomes

coordinates_min = (-1.0, -1.0) # minimum coordinates (min(x), min(y))
coordinates_max = (1.0, 1.0) # maximum coordinates (max(x), max(y))

but apart from that this looks fine to me.

[lchristm]

I only looked at a few files (~two dozen; we have a lot of examples these days 😅) and did not find any issues. IMHO, overall the readability is not much better than before, but I think it is good that the formatting is consistent with the main code base now.

[JoshuaLampert]

LGTM!

[JoshuaLampert]

Looks alright to me, we loose some "nice" formatting like

coordinates_min = (-1.0, -1.0) # minimum coordinates (min(x), min(y))
coordinates_max = ( 1.0,  1.0) # maximum coordinates (max(x), max(y))

which becomes

coordinates_min = (-1.0, -1.0) # minimum coordinates (min(x), min(y))
coordinates_max = (1.0, 1.0) # maximum coordinates (max(x), max(y))

but apart from that this looks fine to me.

Personally, I actually prefer the formatted one.

[JoshuaLampert]

Does someone know why some of the tests fail? Some of them even quite considerably (the actual error differs by one order of magnitude to the expected one)... We having similar issues also in other PRs, but strangely not the same tests fail.

[ranocha]

Does someone know why some of the tests fail? Some of them even quite considerably (the actual error differs by one order of magnitude to the expected one)... We having similar issues also in other PRs, but strangely not the same tests fail.

I have no clue... 😞

[ranocha]

Since CI also fails in other PRs, I think we should go ahead and merge this PR to avoid merge conflicts with other works. Okay for you, @sloede?

[sloede]

Since CI also fails in other PRs, I think we should go ahead and merge this PR to avoid merge conflicts with other works. Okay for you, @sloede?

I think we should wait until all tests have run, but yes, I agree that if this is just the same errors as elsewhere, we can merge

[DanielDoehring]

Does someone know why some of the tests fail? Some of them even quite considerably (the actual error differs by one order of magnitude to the expected one)... We having similar issues also in other PRs, but strangely not the same tests fail.

So for the elixirs using the error-based timestepping I remember that there where actually different errors depending on the machine. This time, I can actually reproduce the errors on my machine and we have differences for other examples using CFL-based timestepping.

In principle, we should be able to find the point where things went wrong, right...?

[ranocha]

Yes, you're right. Did some update of one of the time stepping dependencies introduce the change we see here?

[jlchan]

Formatting appears to have removed something crucial

[JoshuaLampert]

I don't think the formatter did that, but the manual formatting, e.g., here.