Hybrid parallel boundary conditions for turbulence and compressible flow solvers#975

pcarruscag · 2020-05-09T20:33:49Z

Proposed Changes

So far BC's were done by a single thread, this PR addresses that which required making all that code thread-safe. There is also some duplication-avoiding cleanup.
A batch of regression tests for hybrid parallel (compilation + execution) was added. Boundary conditions are perhaps the easiest places to introduce non-thread-safe code, due to the common pattern of writing into temporary arrays of CSolver (e.g. Solution_i), thus it becomes important to have some tests in place.

Related Work

Continuation of #789, so far as compressible flow goes we now have "full coverage", the only major areas left out (that matter to scalability) are MPI and parts of the initial preprocessing.

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).
My contribution is commented and consistent with SU2 style.

…d changing the signature of all functions)

pcarruscag · 2020-05-10T17:05:10Z

SU2_CFD/src/solvers/CSolver.cpp

-    for (iVar = 0; iVar < nVar; iVar++)
-      Solution[iVar] = 0.5* (Solution_i[iVar] + Solution_j[iVar]);


This function (CSolver::SetRotatingFrame_GCL) was averaging the solution but then updating the iPoint / jPoint residuals with Solution_i / j instead, if anyone knows of a good reason for that let me know and I'll put it back, otherwise "it's fixed".

Actually this apparent fix makes one of the turbomachinery cases fail, so I've put the apparent bug back.

Can you make an issue for this please with a link to the spot in the code? I think this could be a bug too but need to look through it...

…it and CHT interface

pcarruscag · 2020-05-13T17:44:43Z

Common/include/CConfig.hpp

-   */
-  void SetSpline(vector<su2double> &x, vector<su2double> &y, unsigned long n, su2double yp1, su2double ypn, vector<su2double> &y2);
-
-  /*!
-   * \brief Given the arrays xa[1..n] and ya[1..n], which tabulate a function (with the xai’s in order),
-   and given the array y2a[1..n], which is the output from spline above, and given a value of
-   x, this routine returns a cubic-spline interpolated value y.
-   Numerical Recipes: The Art of Scientific Computing, Third Edition in C++.
-   * \return The interpolated value of for x.
-   */
-  su2double GetSpline(vector<su2double> &xa, vector<su2double> &ya, vector<su2double> &y2a, unsigned long n, su2double x);


This code was unused and duplicated in CGeometry.

pcarruscag · 2020-05-13T17:46:02Z

SU2_CFD/src/integration/CIntegration.cpp

-  SU2_OMP_MASTER
-  {
+  CNumerics* conv_bound_numerics = numerics[CONV_BOUND_TERM + omp_get_thread_num()*MAX_TERMS];
+  CNumerics* visc_bound_numerics = numerics[VISC_BOUND_TERM + omp_get_thread_num()*MAX_TERMS];


Usual change no 1: One numerics object per thread.

pcarruscag · 2020-05-13T17:47:27Z

SU2_CFD/src/solvers/CEulerSolver.cpp

-  Residual_i    = new su2double[nVar]();
-  Residual_j    = new su2double[nVar]();
-  Res_Conv      = new su2double[nVar]();
-  Res_Visc      = new su2double[nVar]();
-  Res_Sour      = new su2double[nVar]();


Usual change no 2: Avoid using the small arrays of CSolver.

pcarruscag · 2020-05-13T17:48:19Z

SU2_CFD/src/solvers/CEulerSolver.cpp

+  SU2_OMP_FOR_DYN(OMP_MIN_SIZE)
  for (iVertex = 0; iVertex < geometry->nVertex[val_marker]; iVertex++) {


Usual change no 3: Add worksharing directives to for loops.

pcarruscag · 2020-05-13T17:50:06Z

SU2_CFD/src/solvers/CNSSolver.cpp

+      AddDynamicGridResidualContribution(iPoint, Point_Normal, geometry, UnitNormal,
+                                         Area, geometry->nodes->GetGridVel(iPoint),
+                                         Jacobian_i, Res_Conv, Res_Visc);


Significant change no 1: Find duplicated code and move it to something that can be reused.

pcarruscag · 2020-05-13T17:54:52Z

SU2_CFD/src/solvers/CNSSolver.cpp

+void CNSSolver::BC_Isothermal_Wall(CGeometry *geometry, CSolver **solver_container, CNumerics *conv_numerics,
+                                   CNumerics *visc_numerics, CConfig *config, unsigned short val_marker) {

-      LinSysRes.SubtractBlock(iPoint, Res_Visc);
+  BC_Isothermal_Wall_Generic(geometry, solver_container, conv_numerics, visc_numerics, config, val_marker);
+}

-      /*--- Enforce the no-slip boundary condition in a strong way by
-       modifying the velocity-rows of the Jacobian (1 on the diagonal). ---*/
+void CNSSolver::BC_ConjugateHeat_Interface(CGeometry *geometry, CSolver **solver_container, CNumerics *conv_numerics,
+                                           CConfig *config, unsigned short val_marker) {

-      if (implicit) {
-        for (iVar = 1; iVar <= nDim; iVar++) {
-          total_index = iPoint*nVar+iVar;
-          Jacobian.DeleteValsRowi(total_index);
-        }
-      }
-    }
-  }
+  BC_Isothermal_Wall_Generic(geometry, solver_container, conv_numerics, nullptr, config, val_marker, true);
 }


Significant change no 2: The Isothermal and ConjugateHeat BC's are basically the same thing but the latter gets its wall temperature from a different place, so I moved the difference to a small private function, half the code, half the bugs.

pcarruscag · 2020-05-13T17:55:35Z

SU2_CFD/src/solvers/CNSSolver.cpp

-  su2double Vel[3] = {0.0, 0.0, 0.0}, VelNormal, VelTang[3], VelTangMod, VelInfMod, WallDist[3], WallDistMod;
-  su2double T_Normal, P_Normal;
-  su2double Density_Wall, T_Wall, P_Wall, Lam_Visc_Wall, Tau_Wall = 0.0, Tau_Wall_Old = 0.0;
-  su2double *Coord, *Coord_Normal;
-  su2double diff, Delta;
-  su2double U_Tau, U_Plus, Gam, Beta, Phi, Q, Y_Plus_White, Y_Plus;
-  su2double TauElem[3], TauNormal, TauTangent[3], WallShearStress;
-  su2double Gas_Constant = config->GetGas_ConstantND();
-  su2double Cp = (Gamma / Gamma_Minus_One) * Gas_Constant;


Usual change no 5: Define everything as it is used.

pcarruscag · 2020-05-13T17:58:47Z

SU2_CFD/src/solvers/CTurbSSTSolver.cpp

 void CTurbSSTSolver::BC_Isothermal_Wall(CGeometry *geometry, CSolver **solver_container, CNumerics *conv_numerics,
                                        CNumerics *visc_numerics, CConfig *config, unsigned short val_marker) {

-  unsigned long iPoint, jPoint, iVertex, total_index;
-  unsigned short iDim, iVar;
-  su2double distance, density = 0.0, laminar_viscosity = 0.0, beta_1;
-
-  for (iVertex = 0; iVertex < geometry->nVertex[val_marker]; iVertex++) {
-    iPoint = geometry->vertex[val_marker][iVertex]->GetNode();
-
-    /*--- Check if the node belongs to the domain (i.e, not a halo node) ---*/
-    if (geometry->nodes->GetDomain(iPoint)) {
-
-      /*--- distance to closest neighbor ---*/
-      jPoint = geometry->vertex[val_marker][iVertex]->GetNormal_Neighbor();
-      distance = 0.0;
-      for (iDim = 0; iDim < nDim; iDim++) {
-        distance += (geometry->nodes->GetCoord(iPoint, iDim) - geometry->nodes->GetCoord(jPoint, iDim))*
-        (geometry->nodes->GetCoord(iPoint, iDim) - geometry->nodes->GetCoord(jPoint, iDim));
-      }
-      distance = sqrt(distance);
-
-      /*--- Set wall values ---*/
-
-      density = solver_container[FLOW_SOL]->GetNodes()->GetDensity(jPoint);
-      laminar_viscosity = solver_container[FLOW_SOL]->GetNodes()->GetLaminarViscosity(jPoint);
-
-      beta_1 = constants[4];
-
-      Solution[0] = 0.0;
-      Solution[1] = 60.0*laminar_viscosity/(density*beta_1*distance*distance);
-
-      /*--- Set the solution values and zero the residual ---*/
-      nodes->SetSolution_Old(iPoint,Solution);
-      nodes->SetSolution(iPoint,Solution);
-      LinSysRes.SetBlock_Zero(iPoint);
-
-      /*--- Change rows of the Jacobian (includes 1 in the diagonal) ---*/
-      for (iVar = 0; iVar < nVar; iVar++) {
-        total_index = iPoint*nVar+iVar;
-        Jacobian.DeleteValsRowi(total_index);
-      }
-
-    }
-  }
+  BC_HeatFlux_Wall(geometry, solver_container, conv_numerics, visc_numerics, config, val_marker);

 }


.github/workflows/regression.yml

…into hybrid_parallel_bcs

pcarruscag · 2020-05-27T15:53:32Z

@talbring files have been moved

talbring

Very nice. You also did a good clean-up of some stuff :) Thanks!

pcarruscag added 3 commits May 9, 2020 16:04

Merge branch 'linsol_output' into hybrid_parallel_bcs

1e7a5ea

thread-safe BC's for turbulent solvers

520ae25

thread-safe compressible solver BC's

6a1ab58

pr-triage bot added the PR: unreviewed label May 9, 2020

pcarruscag added 2 commits May 10, 2020 17:41

avoid using the small member arrays of CSolver also in its methods

496c5f5

pick one boundary numerics object per thread in CIntegration (to avoi…

e70d5fb

…d changing the signature of all functions)

pcarruscag commented May 10, 2020

View reviewed changes

pcarruscag added 6 commits May 11, 2020 00:13

fix for turbo boundaries

4a21bcf

un-fix CSolver::SetRotatingFrame_GCL as one restarted case was diverging

266ee30

Smatrix and Cvector no longer needed

c46166e

CNSSolver::BC_HeatFlux_Wall

6ecc03c

cleanup isothermal boundary condition and remove duplication between …

3e8ba1b

…it and CHT interface

simple fix

a95105a

pcarruscag changed the title ~~Hybrid parallel boundary conditions for turbulence and compressible flow solvers [WIP]~~ Hybrid parallel boundary conditions for turbulence and compressible flow solvers May 11, 2020

pcarruscag added the changelog:none label May 11, 2020

pcarruscag and others added 5 commits May 11, 2020 17:58

cleanup wall functions

885ea02

fix for sym plane BC

33f99e3

delete a few useless lines from CConfig

f6935a4

Merge branch 'develop' into hybrid_parallel_bcs

a59c7ff

Merge branch 'develop' into hybrid_parallel_bcs

2b5f72f

pcarruscag commented May 13, 2020

View reviewed changes

pcarruscag and others added 9 commits May 15, 2020 16:56

small tweak of CSysMatrix preconditioner-allocation logic

4f987cd

Merge branch 'develop' into hybrid_parallel_bcs

67a1051

add hybrid test configuration

faf1382

add regression somewhere else

02be287

try again

ae54af3

try again

a2b1317

logic fix for SU2 kind

f6a3571

bloody hell

5596a5d

wrong script name

2e5c477

pcarruscag added 3 commits May 15, 2020 21:02

few testcase tweaks

32e7391

Merge branch 'linsol_output' into hybrid_parallel_bcs

e483760

cleanup regression script

46b87e1

talbring reviewed May 20, 2020

View reviewed changes

.github/workflows/regression.yml Outdated Show resolved Hide resolved

talbring and others added 2 commits May 22, 2020 19:28

Simplified regression.yml script

ca3e637

Merge branch 'develop' into hybrid_parallel_bcs

30bbd90

pcarruscag commented May 22, 2020

View reviewed changes

.github/workflows/regression.yml Show resolved Hide resolved

talbring and others added 5 commits May 22, 2020 19:47

Small fix

beb3da9

Merge branch 'hybrid_parallel_bcs' of https://github.com/su2code/SU2 …

f88024c

…into hybrid_parallel_bcs

Merge branch 'feature_mixed_precision' into hybrid_parallel_bcs

98c9a0d

put mixed precision under regression

53bd556

fix bug, update hybrid regressions

c8cbf90

pcarruscag mentioned this pull request May 27, 2020

Add wall functions #399

Closed

Merge remote-tracking branch 'upstream/develop' into hybrid_parallel_bcs

d5f033a

pcarruscag added 4 commits May 27, 2020 17:12

fix pywrapper

6413241

fix unit tests

0e86465

fix pywrapper, again

c05943b

give tests more time

ef9eac3

talbring approved these changes May 27, 2020

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels May 27, 2020

pcarruscag merged commit 33adee2 into develop May 28, 2020

pcarruscag deleted the hybrid_parallel_bcs branch May 28, 2020 09:44

pr-triage bot added PR: merged PR: reviewed-approved and removed PR: reviewed-approved PR: merged labels May 28, 2020

pcarruscag mentioned this pull request May 28, 2020

Possible bug in CSolver::SetRotatingFrame_GCL #1005

Closed

jblueh mentioned this pull request Mar 1, 2021

Hybrid Parallel AD (Part 1/?) #1214

Merged

5 tasks

		for (iVar = 0; iVar < nVar; iVar++)
		Solution[iVar] = 0.5* (Solution_i[iVar] + Solution_j[iVar]);

		SU2_OMP_FOR_DYN(OMP_MIN_SIZE)
		for (iVertex = 0; iVertex < geometry->nVertex[val_marker]; iVertex++) {

Conversation

pcarruscag commented May 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Related Work

PR Checklist

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!

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!

pcarruscag May 13, 2020 • 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!

Uh oh!

Uh oh!

pcarruscag commented May 27, 2020

Uh oh!

talbring left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pcarruscag commented May 9, 2020 •

edited

Loading

pcarruscag May 13, 2020 •

edited

Loading