-
Notifications
You must be signed in to change notification settings - Fork 913
Explicit Euler in CScalarSolver #1435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
PrepareExplicitIteration. It's a mixture of the implicit Euler in CScalarSolver and the explicit Euler in CHeatSolver.
|
The added lines are a combination of the explicit Euler in CHeatSolver and the implicit Euler in CScalarSolver. Do you consider this approach to be correct? For the testcase Some more details:
|
pcarruscag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the hard route to fix this 👍
I think RK4 is going to be harder to implement as it would involve changes to CSingleGridIntegration::SingleGrid_Iteration if you don't go for it please add an error somewhere in config postprocessing.
| } | ||
| END_SU2_OMP_FOR | ||
|
|
||
| CompleteImplicitIteration(geometry, solver_container, config); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes a lot of sense given the logic needed to handle SA v SST idiosyncrasies 👍
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
PrepareExplicitIteration was mainly about computation of residual norms. We moved this code into two functions called from ExplicitEuler_Iteration, and also called from ImplicitEuler_Iteration now (removing duplicate code).
and use it from other locations in the code as well.
Sometimes the residual norm is computed from residuals other than LinSysRes.
adapted from turb_NACA0012_sst_fixedvalues.cfg I will fill in the residuals of the CI pipeline in the next commit. Note that these tests only check that the behavior of the explicit Euler scheme is constant, but not that it is correct (i.e., it is the explicit Euler scheme) at the time of this commit.
With this cfg, serial SU2 converges to rms[Rho]<-12 in iteration 2053191,
for the new testcase introduced in 022b7c0
pcarruscag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 👍 Good idea with the additional residual reduction cleanup.
TobiKattmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Max for this 💐 looks really neat and kudos for the additional generalizations.
My comments are mostly/only nitpicking stylistic things so nothing major. Only for the curly brace init I would like to get an opinion of pedro
Once develop is merged (and you fix the CodeFactor warning 😄 ) I guess this is good to go then
Co-authored-by: TobiKattmann <31306376+TobiKattmann@users.noreply.github.com>
see C.128 in the C++ Core Guidelines suggested by CodeFactor
|
@maxaehle Can you please also check whether there are problems with periodic BC's? What makes me believe that is CTurbSASolver+167 (and similar for SST) /*--- The turbulence models are always solved implicitly, so set the
implicit flag in case we have periodic BCs. ---*/
SetImplicitPeriodic(true); |
|
@TobiKattmann was right about the periodic comms. |
| CTurbSolver::CTurbSolver(CGeometry* geometry, CConfig *config, bool conservative) | ||
| : CScalarSolver<CTurbVariable>(geometry, config, conservative) { | ||
| /*--- Store if an implicit scheme is used, for use during periodic boundary conditions. ---*/ | ||
| SetImplicitPeriodic(config->GetKind_TimeIntScheme_Turb() == EULER_IMPLICIT); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thanks for checking, then I'll do the same for the SpeciesSolver
Which introduces explicit euler for the scalar solvers. Additional smaller ctor cleanups.
|
@maxaehle as far as I see there are no further comments from pedro or me, so if you merge develop + pass the reg test feel free to merge this. (of course if you also have no further plans with this PR) |
Proposed Changes
Enable time integration by the explicit Euler scheme for e.g. turbulence solvers, by implementing
CScalarSolver<>::ExplicitEuler_Iteration.Related Work
This fixes #1432 with respect to explicit Euler. I need the feature for further investigation of #1414.
CScalarSolverwas introduced in #1330.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.