Fix for axisymmetric terms in NEMO + general NEMO updates #1326

WallyMaier · 2021-07-12T15:53:06Z

Proposed Changes

-This work is focused on fixing some of the axisymmetric loop structures within NEMO, which prevented residual evolution. This also changes a viscous planar wedge regression to an axisymmetric case.

-Code updates to make NEMO more-in-line with SU2 code style. i.e. variable declarations, SU2 MPI errors, etc.

-Fixes bug in vibrational-electronic functionality for CSU2TCLib

-Updating ionization/nheavy loops so CSU2TCLib and CMutationTCLib are consistent.

Related Work

This addresses/builds on #1106 #1205

PR Checklist

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.

WallyMaier · 2021-07-12T15:53:35Z

Common/src/CConfig.cpp

@@ -3582,14 +3582,6 @@ void CConfig::SetPostprocessing(SU2_COMPONENT val_software, unsigned short val_i
      SU2_MPI::Error("AUSMPW+ is extremely unstable. Feel free to fix me!", CURRENT_FUNCTION);
  }

-  if (GetGasModel() == "ARGON" && GetKind_FluidModel() == SU2_NONEQ){
-      SU2_MPI::Error("ARGON is not working with SU2_NONEQ fluid model!", CURRENT_FUNCTION);


These are now working.

WallyMaier · 2021-07-12T15:54:12Z

Common/src/CConfig.cpp

@@ -5023,7 +5015,8 @@ void CConfig::SetPostprocessing(SU2_COMPONENT val_software, unsigned short val_i
  /*--- Specifying a deforming surface requires a mesh deformation solver. ---*/
  if (GetSurface_Movement(DEFORMING)) Deform_Mesh = true;

-  if (GetGasModel() == "ARGON") monoatomic = true;
+  if (GetGasModel() == "ARGON") {monoatomic = true;}


Prevents some issues with unintialized jumps

SU2_CFD/src/fluid/CSU2TCLib.cpp

WallyMaier · 2021-07-12T15:56:49Z

SU2_CFD/src/numerics/flow/flow_sources.cpp

-                         -TWO3*(AuxVar_Grad_i[1][1]+AuxVar_Grad_i[2][0]));
+                                                 +v*TWO3*(2*PrimVar_Grad_i[1][1]-PrimVar_Grad_i[1][0]
+                                                 -v*yinv+U_i[0]*turb_ke_i))
+                                                 +total_conductivity_i*PrimVar_Grad_i[0][1])


@FlorianDm should this value be "-total_conductivity_i*PrimVar"?

The term appears as "-qy" in the literature. Where "qy" is defined as "-k dt/du"

Indeed I believe it should actually be +k dT/dy then. Thanks for spotting this. The test case i made breaks so I will rerun it with this branch

Ok so I reran the test case with this branch. I guess I should wait until this PR is to be merged to push the changes in the regression files to the TestCases repo so any other open PRs don't get failed regressions

or is the rest ready to go? If it may take a while maybe I better do the fix now in another PR in concert with the TestCases changes so it can be merged straight away as a simple fix and then you just merge the latest develop in here.. let me know what you think is best

@FlorianDm Glad you could confirm! This PR should be done on Monday/Tuesday (hopefully) and Ill update the regression tests. However, if you want to go ahead to address it case before then, thats fine as well!

WallyMaier · 2021-07-12T15:57:03Z

SU2_CFD/src/output/CNEMOCompOutput.cpp

@@ -324,10 +324,6 @@ void CNEMOCompOutput::SetVolumeOutputFields(CConfig *config){
    break;
  }

-  //Auxiliary variables for post-processment


removing repeated line

SU2_CFD/src/solvers/CNEMOEulerSolver.cpp

WallyMaier · 2021-07-12T16:00:25Z

SU2_CFD/src/solvers/CNEMOEulerSolver.cpp

@@ -1670,23 +1665,23 @@ void CNEMOEulerSolver::BC_Far_Field(CGeometry *geometry, CSolver **solver_contai

        /*--- Species diffusion coefficients ---*/
        visc_numerics->SetDiffusionCoeff(nodes->GetDiffusionCoeff(iPoint),
-                                         node_infty->GetDiffusionCoeff(0) );
+                                         nodes->GetDiffusionCoeff(iPoint));


This is more consistent with the baseline SU2.

…ion restructure

SU2_CFD/src/solvers/CNEMOEulerSolver.cpp

pcarruscag · 2021-07-12T22:28:23Z

SU2_CFD/src/solvers/CNEMOEulerSolver.cpp

+    for (iPoint = 0; iPoint < nPoint; iPoint++) {
+      su2double yCoord          = geometry->nodes->GetCoord(iPoint, 1);
+      su2double yVelocity       = nodes->GetVelocity(iPoint,1);
+      su2double xVelocity       = nodes->GetVelocity(iPoint,0);
+      su2double Total_Viscosity = nodes->GetLaminarViscosity(iPoint) + nodes->GetEddyViscosity(iPoint);
+
+      if (yCoord > EPS){
+        su2double nu_v_on_y = Total_Viscosity*yVelocity/yCoord;
+        nodes->SetAuxVar(iPoint, 0, nu_v_on_y);
+        nodes->SetAuxVar(iPoint, 1, nu_v_on_y*yVelocity);
+        nodes->SetAuxVar(iPoint, 2, nu_v_on_y*xVelocity);
+      }
+   }
+
+   /*--- Compute the auxiliary variable gradient with GG or WLS. ---*/
+   if (config->GetKind_Gradient_Method() == GREEN_GAUSS) {
+     SetAuxVar_Gradient_GG(geometry, config);
+   }
+   if (config->GetKind_Gradient_Method() == WEIGHTED_LEAST_SQUARES) {
+     SetAuxVar_Gradient_LS(geometry, config);
+   }


Move this to CFVMFlowSolverBase.inl and share it between CEulerSolver and CNEMOEulerSolver please (the bug is in CEuler solver, not here)

Could you explain why this would go in .inl and not .hpp? Just for future reference? Is it because the function is identical and not a template? And if thats the case, should I make it a template and include the incompressible portion of the code?

Yes, the logic is more or less: if the method does not have to be a template then it is declared in the .hpp and implemented in .inl of CFVMSolverBase.
If it has to be a template, or you want it to be "inlineable", then it has to go in the .hpp.
But both files are headers, the only thing that changes is the "visibility", e.g. CEulerSolver includes the hpp but not the inl.
At the moment the inl is included by the NS solvers, at the top of the associated .cpp's you'll see

/*--- Explicit instantiation of the parent class of CIncEulerSolver, * to spread the compilation over two cpp files. ---*/ template class CFVMFlowSolverBase<CIncEulerVariable, ENUM_REGIME::INCOMPRESSIBLE>;

That also means that if you change something in the .inl only the NS solvers recompile, but not the Euler solvers (which sounded like a good idea because e.g. CEulerSolver is a reasonably large file).

The incompressible version looks different enough to be either its own method or left as is.
My rule of thumb for such master versions is not to have physics-based conditionals in there (e.g. "if compressible"), if necessary, that type of detail should be provided from the outside, and not already present in the body of the "master" version.
In other words, we want those versions to be generic in the sense they allow externally provided specialization, not in the sense they already try to cover all possible options.
I hope this makes some sense.

SU2_CFD/src/numerics/NEMO/NEMO_sources.cpp

SU2_CFD/src/fluid/CSU2TCLib.cpp

WallyMaier · 2021-07-20T00:09:28Z

Everything is up-to-date. I just need to update the TestCase folder....not sure what that order is...

FlorianDm · 2021-07-20T10:36:22Z

TestCases/hybrid_regression.py

@@ -227,7 +227,7 @@ def main():
    axi_rans_air_nozzle.cfg_dir   = "axisymmetric_rans/air_nozzle"
    axi_rans_air_nozzle.cfg_file  = "air_nozzle.cfg"
    axi_rans_air_nozzle.test_iter = 10
-    axi_rans_air_nozzle.test_vals = [-12.094937, -6.622043, -8.814412, -2.393288]
+    axi_rans_air_nozzle.test_vals = [-6.348077, -0.827162, -2.241982, 2.313210]


Just to clarify, the way I had done the regressions was with RESTART=YES in the config file where the restart file in TestCases solution_flow.dat was already converged to rho<1e-12 so it would just do 10 iterations and then those values I took. I see you did it differently since the values are only at 1e-6. I guess it does not matter but just wanted to clarify

@FlorianDm, thanks for the clarification. I only updated the values in the regression test, so it still should be a restart. I could make an update to the current solution, or change the cfg to RESTART=NO. Though I was dumb and already pushed the changes in the TestCases...I dont this to sit broken, so I may just put in another small PR

SU2_CFD/include/solvers/CFVMFlowSolverBase.hpp

SU2_CFD/include/solvers/CFVMFlowSolverBase.inl

SU2_CFD/src/solvers/CEulerSolver.cpp

SU2_CFD/src/solvers/CNEMOEulerSolver.cpp

pcarruscag

The order is to merge into testcases first just before merging this branch (then you can manually trigger the tests to re-run).

WallyMaier and others added 4 commits June 30, 2021 13:51

making initial updates

521b960

cleanup

61d68db

more fixes

c5f02cb

Compiles, no segfaults

6ff5768

pr-triage bot added the PR: draft label Jul 12, 2021

WallyMaier commented Jul 12, 2021

View reviewed changes

unknown and others added 5 commits July 12, 2021 10:03

addressing github compilation issues. also adding in electron/ionizat…

5a4ae9f

…ion restructure

more github compiling fixes

8445016

more github compiling fixes

addd126

fixing ionization shift

7593aae

fixing loop mistake causing mpi failures

fe817eb

pcarruscag reviewed Jul 12, 2021

View reviewed changes

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

pcarruscag reviewed Jul 12, 2021

View reviewed changes

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

Apply suggestions from code review

4a039f7

pcarruscag reviewed Jul 12, 2021

View reviewed changes

SU2_CFD/src/numerics/NEMO/NEMO_sources.cpp Outdated Show resolved Hide resolved

WallyMaier commented Jul 12, 2021

View reviewed changes

SU2_CFD/src/fluid/CSU2TCLib.cpp Outdated Show resolved Hide resolved

WallyMaier added bug changelog:fix labels Jul 13, 2021

WallyMaier added 5 commits July 19, 2021 13:43

address pr comments fixing some of the regressions

5dfd51d

fix code issue warning

69988a6

update to develop

398b356

removing sneaky conflict

9c9657e

updating regressions

ae0df98

WallyMaier changed the title ~~[WIP] Fix for axisymmetric terms in NEMO + general NEMO updates~~ Fix for axisymmetric terms in NEMO + general NEMO updates Jul 20, 2021

FlorianDm reviewed Jul 20, 2021

View reviewed changes

pcarruscag approved these changes Jul 20, 2021

View reviewed changes

Apply suggestions from code review

32b7c95

pcarruscag reviewed Jul 20, 2021

View reviewed changes

pcarruscag marked this pull request as ready for review July 20, 2021 14:21

pr-triage bot added PR: unreviewed and removed PR: draft labels Jul 20, 2021

WallyMaier merged commit cf4677b into develop Jul 21, 2021

WallyMaier deleted the nemo_update branch July 21, 2021 06:09

pr-triage bot added PR: merged and removed PR: unreviewed labels Jul 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for axisymmetric terms in NEMO + general NEMO updates #1326

Fix for axisymmetric terms in NEMO + general NEMO updates #1326

WallyMaier commented Jul 12, 2021 •

edited

Loading

WallyMaier Jul 12, 2021

WallyMaier Jul 12, 2021

WallyMaier Jul 12, 2021 •

edited

Loading

FlorianDm Jul 17, 2021 •

edited

Loading

FlorianDm Jul 17, 2021

FlorianDm Jul 17, 2021

WallyMaier Jul 17, 2021

WallyMaier Jul 12, 2021

WallyMaier Jul 12, 2021

pcarruscag Jul 12, 2021

WallyMaier Jul 13, 2021

pcarruscag Jul 14, 2021

WallyMaier Jul 20, 2021

WallyMaier commented Jul 20, 2021

FlorianDm Jul 20, 2021

WallyMaier Jul 20, 2021 •

edited

Loading

pcarruscag left a comment

Fix for axisymmetric terms in NEMO + general NEMO updates #1326

Fix for axisymmetric terms in NEMO + general NEMO updates #1326

Conversation

WallyMaier commented Jul 12, 2021 • edited Loading

Proposed Changes

Related Work

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WallyMaier Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

FlorianDm Jul 17, 2021 • edited Loading

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

WallyMaier commented Jul 20, 2021

Choose a reason for hiding this comment

WallyMaier Jul 20, 2021 • edited Loading

Choose a reason for hiding this comment

pcarruscag left a comment

Choose a reason for hiding this comment

WallyMaier commented Jul 12, 2021 •

edited

Loading

WallyMaier Jul 12, 2021 •

edited

Loading

FlorianDm Jul 17, 2021 •

edited

Loading

WallyMaier Jul 20, 2021 •

edited

Loading