NEMO - Improvements on the Preprocessing phase and inclusion of Chapmann-Enskog for Mutation++ by fmpmorgado · Pull Request #1343 · su2code/SU2

fmpmorgado · 2021-08-02T12:03:47Z

Proposed Changes

Optimization of the Preprocessing phase;
Inclusion of the Chapmann-Enskog option for Mutation++

Related Work

None

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.

[ X ] I am submitting my contribution to the develop branch.
[ X ] 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).
[ X ] My contribution is commented and consistent with SU2 style.
[ X ] I have added a test case that demonstrates my contribution, if necessary.
[ X ] I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

Work Performed

I have reduced almost by half the time of the preprocessing phase, leading to a ~10% less time per iteration. This was done by:

Calculate dPdU, dTdU, dTvedU only for the implicit case
Avoid the calculation of eve_min and eve_max using Mutation++. Instead, return nonphysical if the vibrational temperatures reach the specified minimum and maximum value

Additionally, I have included the option of using Chapmann-Enskog for the calculation of transport equations and thermal conductivity using Mutation++

Common/src/CConfig.cpp

SU2_CFD/src/variables/CNEMOEulerVariable.cpp

WallyMaier · 2021-08-02T14:28:01Z

SU2_CFD/src/variables/CNEMOEulerVariable.cpp

-
-  V[RHOCVTR_INDEX] = rhoCvtr;
-  V[RHOCVVE_INDEX] = rhoCvve;
+  V[RHOCVTR_INDEX] = fluidmodel->ComputerhoCvtr();


WallyMaier · 2021-08-02T14:40:24Z

SU2_CFD/src/variables/CNEMOEulerVariable.cpp

-  fluidmodel->ComputedTdU  (V, val_dTdU );
-  fluidmodel->ComputedTvedU(V, eves, val_dTvedU);
+  if(implicit){
+    fluidmodel->ComputedPdU  (V, eves, val_dPdU  );


These may be required for some explicit schemes/viscous

The rest of the code code seems okay, going to take a further look to see which schemes and routines use those quantities

If in doubt and the computations are not expensive...

lgtm-com · 2021-08-02T15:05:51Z

This pull request fixes 1 alert when merging 8450d94 into f47e22b - view on LGTM.com

fixed alerts:

1 for Comparison of narrow type with wide type in loop condition

Common/include/option_structure.hpp

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

fmpmorgado · 2021-08-03T19:11:45Z

@WallyMaier @CatarinaGarbacz just checked with the testcases Wally provided. All of them pass

SU2_CFD/include/variables/CNEMOEulerVariable.hpp

WallyMaier · 2021-08-03T20:28:43Z

SU2_CFD/src/variables/CNEMOEulerVariable.cpp

  unsigned short iDim, iSpecies;
+
+  /*--- Setting flags ---*/
+  implicit = (config->GetKind_TimeIntScheme_Flow() == EULER_IMPLICIT);


something like this?

Suggested change

implicit = (config->GetKind_TimeIntScheme_Flow() == EULER_IMPLICIT);

bool implicit = (config->GetKind_TimeIntScheme_Flow() == EULER_IMPLICIT);

If I initialize the variable inside the CNEMOEulerVariable instantiaton function, it can't be used for other functions. If I am not mistaken, that variable will be deleted when the function returns

is it used in other functions wuth the variable structure?

Sorry @WallyMaier didn't get it. What variable structure are you mentioning ?

If the boolean implicit is only used in this function in NEMOVariable, I dont think we should declare in the hpp.

I don't think I can do it any other way, else the CConfig parameter would have to be passed as argument to the Cons2PrimVar function.

Don't rely on passing the config around too much.

lgtm-com · 2021-08-04T17:11:30Z

This pull request fixes 1 alert when merging fbbc6ee into f47e22b - view on LGTM.com

fixed alerts:

1 for Comparison of narrow type with wide type in loop condition

Common/include/option_structure.hpp

Common/src/CConfig.cpp

lgtm-com · 2021-08-05T19:57:34Z

This pull request fixes 1 alert when merging 0cd78e4 into f47e22b - view on LGTM.com

fixed alerts:

1 for Comparison of narrow type with wide type in loop condition

SU2_CFD/include/variables/CNEMOEulerVariable.hpp

SU2_CFD/src/variables/CNEMOEulerVariable.cpp

lgtm-com · 2021-08-09T11:09:21Z

This pull request fixes 1 alert when merging a8b0b93 into df2469f - view on LGTM.com

fixed alerts:

1 for Comparison of narrow type with wide type in loop condition

pcarruscag · 2021-08-09T12:24:25Z

Common/include/option_structure.hpp

+static const MapType<std::string, TRANSCOEFFMODEL> TransCoeffModel_Map = {
+MakePair("WILKE", TRANSCOEFFMODEL::WILKE)
+MakePair("GUPTA-YOS", TRANSCOEFFMODEL::GUPTAYOS)
+MakePair("CHAPMANN-ENSKOG", TRANSCOEFFMODEL::CHAPMANN_ENSKOG)


Do you guys have these options documented somewhere? I don't see them in the config_template but maybe in one of NEMO testcases?

I have now included this option in the config_template file. It's not used on NEMO test-cases though

…EMO_opt_init

lgtm-com · 2021-08-09T13:45:32Z

This pull request fixes 1 alert when merging b2a783e into df2469f - view on LGTM.com

fixed alerts:

1 for Comparison of narrow type with wide type in loop condition

TobiKattmann

Yeah I'm late to the party... but lgtm @fmpmorgado . Just 2 very little comments one might consider for the next PR

TobiKattmann · 2021-08-11T12:27:23Z

SU2_CFD/src/variables/CNEMOEulerVariable.cpp

+  if (V[T_INDEX] <= Tmin)      { nonPhys = true; return nonPhys;}
+  else if (V[T_INDEX] >= Tmax) { nonPhys = true; return nonPhys;}
+  else if (V[T_INDEX] != V[T_INDEX]){ nonPhys = true; return nonPhys;}


Here I would have gone with sth like

if ((V[T_INDEX] <= Tmin) || condition2 || condition3) { nonPhys = true; return nonPhys; }

The same below for Tve

Thanks @TobiKattmann, I'll address your comments in #1347

TobiKattmann · 2021-08-11T12:28:41Z

SU2_CFD/src/variables/CNEMOEulerVariable.cpp

+                                       Gradient_Reconstruction(config->GetReconstructionGradientRequired() ? Gradient_Aux : Gradient_Primitive),
+                                       implicit(config->GetKind_TimeIntScheme_Flow() == EULER_IMPLICIT) {


here a little indentation more

A little clang-format you mean? 😄

fmpmorgado added 3 commits August 1, 2021 20:36

First iteration optimization

8955576

Optimize Cons2PrimVar

e25f18b

Inclusion of Chapmann-Enskog option for Mutation++

4f3aaf2

fmpmorgado requested review from CatarinaGarbacz and WallyMaier August 2, 2021 12:03

pr-triage bot added the PR: draft label Aug 2, 2021

fmpmorgado added 2 commits August 2, 2021 13:05

Additional spacing removed

42f2fd2

Merge branch 'develop' into NEMO_opt_init

8450d94

WallyMaier reviewed Aug 2, 2021

View reviewed changes

pcarruscag reviewed Aug 2, 2021

View reviewed changes

Common/include/option_structure.hpp Outdated Show resolved Hide resolved

Common/include/option_structure.hpp Outdated Show resolved Hide resolved

Apply suggestions from code review

2dbe2d1

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

fmpmorgado marked this pull request as ready for review August 3, 2021 19:11

pr-triage bot added PR: unreviewed and removed PR: draft labels Aug 3, 2021

WallyMaier reviewed Aug 3, 2021

View reviewed changes

Return to previous commit, last one was giving problems

fbbc6ee

WallyMaier mentioned this pull request Aug 4, 2021

Updating some of the NEMO viscous solver routines. #1347

Merged

5 tasks

pcarruscag reviewed Aug 4, 2021

View reviewed changes

Common/include/option_structure.hpp Outdated Show resolved Hide resolved

Common/include/option_structure.hpp Outdated Show resolved Hide resolved

Common/src/CConfig.cpp Outdated Show resolved Hide resolved

WallyMaier added changelog:chore enhancement labels Aug 5, 2021

Change to enum class TRANSCOEFFMODEL

0cd78e4

pcarruscag reviewed Aug 7, 2021

View reviewed changes

SU2_CFD/include/variables/CNEMOEulerVariable.hpp Outdated Show resolved Hide resolved

pcarruscag reviewed Aug 7, 2021

View reviewed changes

SU2_CFD/src/variables/CNEMOEulerVariable.cpp Outdated Show resolved Hide resolved

pcarruscag approved these changes Aug 7, 2021

View reviewed changes

pr-triage bot added the PR: partially-approved label Aug 7, 2021

Adress implicit flag

d62b1b7

Merge branch 'develop' into NEMO_opt_init

a8b0b93

pcarruscag reviewed Aug 9, 2021

View reviewed changes

fmpmorgado added 2 commits August 9, 2021 13:59

Add tranport models to the config_template

b2b8dae

Merge branch 'NEMO_opt_init' of https://github.com/su2code/SU2 into N…

b2a783e

…EMO_opt_init

fmpmorgado merged commit 3f745be into develop Aug 9, 2021

fmpmorgado deleted the NEMO_opt_init branch August 9, 2021 13:49

pr-triage bot added the PR: merged label Aug 9, 2021

TobiKattmann reviewed Aug 11, 2021

View reviewed changes

WallyMaier mentioned this pull request Aug 21, 2021

[WIP] Test for adding in the implicit viscous solver for NEMO #1356

Closed

5 tasks

WallyMaier mentioned this pull request Oct 9, 2021

Implicit solver for inviscid NEMO problems #1399

Merged

5 tasks

WallyMaier mentioned this pull request Nov 3, 2021

Implementation of Implicit for viscous NEMO problems #1422

Merged

5 tasks

	implicit = (config->GetKind_TimeIntScheme_Flow() == EULER_IMPLICIT);
	bool implicit = (config->GetKind_TimeIntScheme_Flow() == EULER_IMPLICIT);

		Gradient_Reconstruction(config->GetReconstructionGradientRequired() ? Gradient_Aux : Gradient_Primitive),
		implicit(config->GetKind_TimeIntScheme_Flow() == EULER_IMPLICIT) {

Conversation

fmpmorgado commented Aug 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Related Work

PR Checklist

Work Performed

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Aug 2, 2021

Uh oh!

Uh oh!

Uh oh!

fmpmorgado commented Aug 3, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Aug 4, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Aug 5, 2021

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Aug 9, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Aug 9, 2021

Uh oh!

TobiKattmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fmpmorgado commented Aug 2, 2021 •

edited

Loading