`DGMulti` support for curved meshes and parabolic terms #1400

jlchan · 2023-04-17T19:57:04Z

This PR adds support for curved parabolic terms. The previous approach taken for parabolic terms was to

compute the weak-form DG gradient and store it at nodal points
interpolate the gradient to quadrature points, compute the viscous flux
compute the weak-form DG divergence and store it at nodal points in du

Here, we switch to the following workflow

compute the strong-form DG gradient and store it at quadrature points.
compute/store the viscous flux at quadrature points (the only interpolation is to interpolate the solution to quadrature points)
compute the weak-form DG divergence and store it at nodal points in du

This is mathematically equivalent to the former approach, but ensures that the resulting scheme is symmetric and entropy/energy dissipative even on curved meshes. It should also be slightly more efficient (fewer multiplications by interpolation matrices).

Unfortunately, because this involved rewriting much of the parabolic framework, it is a somewhat large PR - apologies in advance.

- it's not necessary for coercivity (BR1 is coercive with any penalty parameter > 0; a factor of 1/h is only necessary for IPDG) - simplifies the implementation for curved meshes - it's not in CI tests yet (should be added soon)

codecov · 2023-04-18T04:14:48Z

Codecov Report

Merging #1400 (a58580e) into main (f5e6f21) will decrease coverage by 4.78%.
The diff coverage is 97.94%.

@@            Coverage Diff             @@
##             main    #1400      +/-   ##
==========================================
- Coverage   95.97%   91.19%   -4.78%     
==========================================
  Files         351      353       +2     
  Lines       29122    29337     +215     
==========================================
- Hits        27949    26752    -1197     
- Misses       1173     2585    +1412

Flag	Coverage Δ
unittests	`91.19% <97.94%> (-4.78%)`	⬇️

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

Impacted Files	Coverage Δ
src/equations/laplace_diffusion_2d.jl	`83.33% <0.00%> (ø)`
src/solvers/dgmulti/dg_parabolic.jl	`90.20% <95.33%> (+0.63%)`	⬆️
...multi_2d/elixir_navierstokes_convergence_curved.jl	`100.00% <100.00%> (ø)`
...multi_3d/elixir_navierstokes_convergence_curved.jl	`100.00% <100.00%> (ø)`
src/solvers/dgmulti/dg.jl	`94.12% <100.00%> (+0.02%)`	⬆️

... and 45 files with indirect coverage changes

…gmulti

the call isn't that complex, and it's easier for me to debug

src/equations/laplace_diffusion_2d.jl

ranocha

Thanks a lot for working on this, @jlchan! Please find below some comments after an initial review. It would be great to get some additional eyes on this.

src/equations/laplace_diffusion_2d.jl

src/solvers/dgmulti/dg.jl

examples/dgmulti_2d/elixir_navierstokes_convergence_curved.jl

ranocha · 2023-04-26T10:48:08Z

examples/dgmulti_2d/elixir_navierstokes_convergence_curved.jl

+                                      -       p    * rho   * rho_yy ) * mu_ )
+
+  return SVector(du1, du2, du3, du4)
+end


I just assume you copied this setup and did not review it properly (same in 3D).

Is this referring to the type-unstable (indexing) comment above?

No - to the actual formulas of the manufactured solution

It's the exact same as the setup for elixir_navierstokes_convergence.jl. I can add a comment mentioning this?

Alternatively, I could just run elixir_navierstokes_convergence.jl and pass a new mesh through trixi_include instead of creating new elixirs.

Nevermind, using trixi_include didn't work. I added some comments noting the origin of the manufactured solution.

Alternatively... shouldn't we add the NSE convergence test to the CompressibleNSEDiffusionXD types? IIRC, we do have a similar function for most other equation systems. Or was there an issue with that why this didn't work/why this is super awkward?

It's fine from my side to keep it as it is - I just wanted to say that I didn't want to read it 😅

@sloede we could. The only potential issue I see is that the source and initial condition require functions mu() and prandtl_number() to be defined (see https://github.com/jlchan/Trixi.jl/blob/a8c0933bd0077c45372c3937c40fce772e348886/examples/dgmulti_2d/elixir_navierstokes_convergence.jl#L56-L57). These functions are necessary due to the fact that initial conditions cannot specialize on only one equation type, since they are called both with equations::CompressibleEulerEquations2D and equations::CompressibleNavierStokesDiffusion2D in different places.

To me, moving the NSE convergence test functions out of the elixir makes these implicit requirements even less clear.

Good point. Let's keep them as they are, please.

src/solvers/dgmulti/dg_parabolic.jl

jlchan · 2023-04-26T19:03:18Z

@ranocha any idea why Invalidations.yml keeps failing?

ranocha · 2023-04-27T05:41:07Z

@ranocha any idea why Invalidations.yml keeps failing?

Invalidations CI run fails because of aviatesk/JET.jl#499. That's not critical right now.

src/solvers/dgmulti/dg_parabolic.jl

Co-authored-by: Hendrik Ranocha <[email protected]>

this makes it so this PR is no longer a breaking change

This reverts commit adc0fde.

src/equations/laplace_diffusion_2d.jl

ranocha

Thanks a lot! This looks good to me.

jlchan added 4 commits April 17, 2023 14:56

factor volume integral out of calc_gradient! and calc_divergence!

26afcbe

add "!" for consistency

2f5732f

minor cleanup

20df8de

remove inv_h from penalty(...)

e689a72

- it's not necessary for coercivity (BR1 is coercive with any penalty parameter > 0; a factor of 1/h is only necessary for IPDG) - simplifies the implementation for curved meshes - it's not in CI tests yet (should be added soon)

jlchan and others added 19 commits April 21, 2023 14:06

Merge remote-tracking branch 'origin/main' into jc/curved_parabolic_d…

616cf57

…gmulti

generalized invert_jacobian

8b914de

remove "prolong2interfaces"

9d90992

the call isn't that complex, and it's easier for me to debug

switch to "strong-weak" formulation

4b032ec

precompute divergence interp matrix

5a2384e

specialize differentiation matrices for Gradient vs Divergence

a6931ae

remove some @unpacks

6d3cd0d

Merge branch 'main' into jc/curved_parabolic_dgmulti

d51d4e8

update parabolic tests

2f13473

fix test again

09554f1

fix test again

99706c7

define md earlier

e14f3ce

use SVector for consistency/efficiency

e1f1d01

add curved gradient volume integral

0c2e7ac

add curved "invert_jacobian_gradient!"

dc78c5a

add curved divergence volume integral

387b6e4

add curved convergence elixir

dd216da

add curved test

2db366a

add 3D test

bfbbb73

jlchan commented Apr 25, 2023

View reviewed changes

src/equations/laplace_diffusion_2d.jl Outdated Show resolved Hide resolved

jlchan marked this pull request as ready for review April 25, 2023 17:09

jlchan requested review from ranocha, andrewwinters5000 and sloede April 25, 2023 17:09

ranocha requested changes Apr 26, 2023

View reviewed changes

jlchan and others added 9 commits April 26, 2023 10:35

Merge branch 'main' into jc/curved_parabolic_dgmulti

0288aaf

add invJ comment for affine elements

0ce8202

NDIMS -> ndims(mesh)

faefe3f

comments

14a45a5

remove outdated TODO

0463178

more comments

f31adb9

remove outdated TODOs

f78abbe

remove broadcasts to reduce allocations

22f4691

add timers to rhs_parabolic

5681534

ranocha reviewed Apr 27, 2023

View reviewed changes

src/solvers/dgmulti/dg_parabolic.jl Outdated Show resolved Hide resolved

src/solvers/dgmulti/dg_parabolic.jl Outdated Show resolved Hide resolved

src/solvers/dgmulti/dg_parabolic.jl Outdated Show resolved Hide resolved

jlchan and others added 5 commits April 27, 2023 08:56

Update src/solvers/dgmulti/dg_parabolic.jl

398be9a

Co-authored-by: Hendrik Ranocha <[email protected]>

Merge branch 'main' into jc/curved_parabolic_dgmulti

774a2f3

remove unused variable

68522b2

replace extra curved elixirs with "trixi_include" calls

adc0fde

revert removal of "inv_h" in penalty

d1a42f2

this makes it so this PR is no longer a breaking change

jlchan requested a review from ranocha April 28, 2023 03:14

jlchan added 2 commits April 27, 2023 22:50

Revert "replace extra curved elixirs with "trixi_include" calls"

02c7fe9

This reverts commit adc0fde.

add comments

5ce9eef

ranocha reviewed Apr 28, 2023

View reviewed changes

src/equations/laplace_diffusion_2d.jl Outdated Show resolved Hide resolved

jlchan and others added 2 commits April 28, 2023 06:41

remove inv_h from penalty formula

a8c0933

Merge branch 'main' into jc/curved_parabolic_dgmulti

a58580e

jlchan requested a review from ranocha April 28, 2023 14:32

ranocha approved these changes Apr 28, 2023

View reviewed changes

jlchan merged commit 458daa8 into trixi-framework:main Apr 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`DGMulti` support for curved meshes and parabolic terms #1400

`DGMulti` support for curved meshes and parabolic terms #1400

jlchan commented Apr 17, 2023 •

edited

Loading

codecov bot commented Apr 18, 2023 •

edited

Loading

ranocha left a comment

ranocha Apr 26, 2023

jlchan Apr 26, 2023

ranocha Apr 27, 2023

jlchan Apr 27, 2023

jlchan Apr 27, 2023

jlchan Apr 28, 2023

sloede Apr 28, 2023

ranocha Apr 28, 2023

jlchan Apr 28, 2023

ranocha Apr 28, 2023

jlchan commented Apr 26, 2023

ranocha commented Apr 27, 2023

ranocha left a comment

DGMulti support for curved meshes and parabolic terms #1400

DGMulti support for curved meshes and parabolic terms #1400

Conversation

jlchan commented Apr 17, 2023 • edited Loading

codecov bot commented Apr 18, 2023 • edited Loading

Codecov Report

ranocha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlchan commented Apr 26, 2023

ranocha commented Apr 27, 2023

ranocha left a comment

Choose a reason for hiding this comment

`DGMulti` support for curved meshes and parabolic terms #1400

`DGMulti` support for curved meshes and parabolic terms #1400

jlchan commented Apr 17, 2023 •

edited

Loading

codecov bot commented Apr 18, 2023 •

edited

Loading