Add periodic boundary conditions to the heat-equation solver #1382

TobiKattmann · 2021-09-25T14:01:37Z

Dear friends of the Heat Solver and Periodic Boundary conditions,

I feared no exhausting work and added periodic BC to the Heat Solver.

Stock up your supplies, cancel meetings for today and dive into the diff to review this piece.

Ok for real, I don't know if that is all that has to be done? I did some early testing with mixed results, I noticed that I had to turn down CFL for the solid zone (1e4 in non-periodic case-> 1) to get a solution that seems to respect the periodicity. If the CFL is too large then I get some spurious temperature and weird residual pattern. Any comments are appreciated. This was just a first shot so Im gonna do more digging on that issue

Related Work

Periodic bc was initially implemented in #652

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

I am submitting my contribution to the develop branch.
My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
My contribution is commented and consistent with SU2 style.
I have added a test case that demonstrates my contribution, if necessary.
I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

TobiKattmann · 2021-09-25T14:03:43Z

(I will add info later but I am in a bit of a hurry currently) with CFL=1

with CFL=1e3

residuals with CFL=1e3

pcarruscag · 2021-09-25T15:37:02Z

Have a look at my last comment in #763, I think there is a better way of handling linear system periodicity instead of what we do, which may allow you to run at higher CFL (and with simpler periodic comms).

But at the moment you are missing the PERIODIC_IMPLICIT comms after solving the linear system, see CompleteImplicitIteration_impl in CFVMFlowSolverBase.hpp.

…work

…iodic

TobiKattmann · 2021-09-26T13:05:26Z

Thanks @pcarruscag , I had to do a few additional things together with the PERIODIC_IMPLICIT communication to make it work, but that also brought me on the right track 👍 Now it works really nicely from my point of view.

Have a look at my last comment in #763, I think there is a better way of handling linear system periodicity instead of what we do, which may allow you to run at higher CFL (and with simpler periodic comms).

I need to have a second look at this. So non of your thoughts back then are yet incorporated in this PR

TobiKattmann · 2021-09-26T13:17:26Z

Here the "new" solution of the above case:

And the Residual_temperature at the periodic border. Not that the very same pattern can be seen on the non-periodic pin upstream. Also for me this kind of blotchy residual pattern is a sign for me that the case fully converged without some "bad" spot of high residual:

And a picture of a CHT case with now added periodicity in the solids. Above with periodicity and below without.

TobiKattmann · 2021-09-26T13:18:37Z

SU2_CFD/src/solvers/CHeatSolver.cpp

+    const su2double dt = nodes->GetDelta_Time(iPoint);
+    if (dt != 0.0) {
+      /*--- For nodes on periodic boundaries, add the respective partner volume. ---*/
      // Identical for flow and heat
-      const su2double Delta = geometry->nodes->GetVolume(iPoint) / nodes->GetDelta_Time(iPoint);
-      Jacobian.AddVal2Diag(iPoint, Delta);
+      const su2double Vol = geometry->nodes->GetVolume(iPoint) + geometry->nodes->GetPeriodicVolume(iPoint);
+      Jacobian.AddVal2Diag(iPoint, Vol / dt);


I made this bit consistent with the Scalar Solver.. which will faciliate a possible port later a bit

TobiKattmann · 2021-09-26T13:20:07Z

SU2_CFD/src/solvers/CHeatSolver.cpp

+  /*--- Communicate and store volume and the number of neighbors for any dual CVs that lie on on periodic markers. ---*/
+  for (unsigned short iPeriodic = 1; iPeriodic <= config->GetnMarker_Periodic() / 2; iPeriodic++) {
+    InitiatePeriodicComms(geometry, config, iPeriodic, PERIODIC_VOLUME);
+    CompletePeriodicComms(geometry, config, iPeriodic, PERIODIC_VOLUME);
+    InitiatePeriodicComms(geometry, config, iPeriodic, PERIODIC_NEIGHBORS);
+    CompletePeriodicComms(geometry, config, iPeriodic, PERIODIC_NEIGHBORS);
+  }
+  /*--- Store if implicit scheme is used. This has implications on the Residual and Jacobian handling for periodic
+   * boundaries  ---*/
+  const bool euler_implicit = (config->GetKind_TimeIntScheme_Heat() == EULER_IMPLICIT);
+  SetImplicitPeriodic(euler_implicit);


The ScalarSolver is currently not doing that. I think there it is enough as the flow solver already does that. But if a ScalarSolver-child is standalone then it prob has to be incorporated. But this is more of a guess than safe info

TobiKattmann · 2021-09-26T13:21:59Z

@NAnand-TUD We talked about this briefly in a recent dev-meeting, so that you know :)

TobiKattmann · 2021-09-26T13:27:13Z

I'll adapt one of the existing Testcases to use this feature

pcarruscag

LGTM

pcarruscag · 2021-09-26T15:12:13Z

With what CFL can you run now?

TobiKattmann · 2021-09-26T16:20:52Z

With what CFL can you run now?

For the solid only case I had the feeling that I could go arbitrarily high. 1e8 and 1e12 had no difference.

For the cht case I kept the cfl of 1e4 from the non-periodic case. There I will try a bit more to see what's possible.

NAnand-TUD · 2021-09-27T06:28:49Z

@TobiKattmann Thanks for sharing! I will be testing this part of the code with our test case soon.

Note that the warning is unjustified and probably a bug in this gcc version. See here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59124

TobiKattmann · 2021-09-27T10:30:10Z

I'll adapt one of the existing Testcases to use this feature

Well that was a lie. I thought it would be a good idea to adapt the 2D cht streamwise periodic case but I forgot that the used mesh is used for a streamwise periodic primal, FD and DA case as well as HeatTransfer BC Testcase (and that is a bit of work tbh). And for the periodic BC I had to change the mesh (adding new markers). As we do not have a single standalone (always run in a CHT setup) heat-equation Testcase, I went ahead and cut out the solid zone only and made that a Testcase. I already merged the gmsh geo and .su2 over at su2code/TestCases/pull/76 . If this is not liked I can of course do sth different. Note that the new Testcase takes ~2s to complete its 750 iterations until convergence.

TobiKattmann · 2021-09-27T10:37:11Z

I found solid only and CHT cases to take ~10x the iterations to reach the same convergence levels if periodic boundary conditions are used in the solid (left: using periodic BC, right: not using periodic BC). This is not really ideal tbh, Maybe I have not found a suitable setup yet (I modifed some things, Linear Solver iter, CFL, Gradient Method) or there is a reasonable explanation for this in how the bc is handeled (Therefore I would have to take a look into #763 I guess ;) )

SU2_CFD/src/solvers/CTurbSASolver.cpp

This reverts commit a0d296c.

SU2_CFD/src/solvers/CHeatSolver.cpp

WallyMaier

LGTM

Add BC_Periodic to HeatSolver.

a77ab48

pr-triage bot added the PR: unreviewed label Sep 25, 2021

TobiKattmann changed the title ~~Add BC_periodic to HeatSolver~~ [WIP] Add BC_periodic to HeatSolver Sep 25, 2021

pr-triage bot removed the PR: unreviewed label Sep 25, 2021

TobiKattmann marked this pull request as draft September 25, 2021 14:02

pr-triage bot added the PR: draft label Sep 26, 2021

TobiKattmann added 2 commits September 26, 2021 14:56

Add necessary additional communication to make HeatSolver periodicBC …

2645197

…work

Merge remote-tracking branch 'origin/develop' into feature_heat_bcper…

f24b126

…iodic

TobiKattmann commented Sep 26, 2021

View reviewed changes

TobiKattmann added the changelog:feature label Sep 26, 2021

TobiKattmann changed the title ~~[WIP] Add BC_periodic to HeatSolver~~ Add BC_periodic to HeatSolver Sep 26, 2021

TobiKattmann marked this pull request as ready for review September 26, 2021 13:33

pr-triage bot added PR: unreviewed and removed PR: draft labels Sep 26, 2021

pcarruscag approved these changes Sep 26, 2021

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Sep 26, 2021

pcarruscag changed the title ~~Add BC_periodic to HeatSolver~~ Add periodic boundary conditions to the heat-equation solver Sep 26, 2021

TobiKattmann mentioned this pull request Sep 27, 2021

Intermediate class of variable for all flow solver variables + Fix for MUSCL in NEMO solver #1375

Merged

4 tasks

Fix warning on gnu-5.3.0. Change one auto to unsigned short.

a0d296c

Note that the warning is unjustified and probably a bug in this gcc version. See here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59124

TobiKattmann mentioned this pull request Sep 27, 2021

Add solid heat Testcase for heat periodic boundaries su2code/TestCases#76

Merged

Add new solid heat conduction Testcase with periodic BC's.

f0d4fb9

pr-triage bot added PR: unreviewed and removed PR: reviewed-approved labels Sep 27, 2021

pcarruscag reviewed Sep 27, 2021

View reviewed changes

SU2_CFD/src/solvers/CTurbSASolver.cpp Outdated Show resolved Hide resolved

Revert "Fix warning on gnu-5.3.0. Change one auto to unsigned short."

e0e7e35

This reverts commit a0d296c.

TobiKattmann commented Sep 29, 2021

View reviewed changes

SU2_CFD/src/solvers/CHeatSolver.cpp Outdated Show resolved Hide resolved

Little typo.

3d52392

WallyMaier reviewed Sep 29, 2021

View reviewed changes

TobiKattmann merged commit 057da5b into develop Sep 29, 2021

TobiKattmann deleted the feature_heat_bcperiodic branch September 29, 2021 16:27

pr-triage bot added PR: merged and removed PR: unreviewed labels Sep 29, 2021

Add periodic boundary conditions to the heat-equation solver #1382

Add periodic boundary conditions to the heat-equation solver #1382

Uh oh!

Conversation

TobiKattmann commented Sep 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Work

PR Checklist

Uh oh!

TobiKattmann commented Sep 25, 2021

Uh oh!

pcarruscag commented Sep 25, 2021

Uh oh!

TobiKattmann commented Sep 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TobiKattmann commented Sep 26, 2021

Uh oh!

TobiKattmann Sep 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TobiKattmann Sep 26, 2021

Choose a reason for hiding this comment

Uh oh!

TobiKattmann commented Sep 26, 2021

Uh oh!

TobiKattmann commented Sep 26, 2021

Uh oh!

pcarruscag left a comment

Choose a reason for hiding this comment

Uh oh!

pcarruscag commented Sep 26, 2021

Uh oh!

TobiKattmann commented Sep 26, 2021

Uh oh!

NAnand-TUD commented Sep 27, 2021

Uh oh!

TobiKattmann commented Sep 27, 2021

Uh oh!

TobiKattmann commented Sep 27, 2021

Uh oh!

Uh oh!

Uh oh!

WallyMaier left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

TobiKattmann commented Sep 25, 2021 •

edited

Loading

TobiKattmann commented Sep 26, 2021 •

edited

Loading

TobiKattmann Sep 26, 2021 •

edited

Loading