Skip to content

Conversation

@rois1995
Copy link
Contributor

@rois1995 rois1995 commented Oct 22, 2023

Proposed Changes

Hi everyone,

I have been working on the implementation of the DDES formulations for the SST model, following the work in "Development of DDES and IDDES Formulations for the k-ω Shear Stress Transport Model" (DOI:10.1007/s10494-011-9378-4). The implementation is easy, whereas the validation may take some time. I am currently working on the backward facing step and I will work on the flatplate too. The choice of the validation test cases is key since the computational cost is quite large, thus if you have any suggestions that may speed up this part then feel free to write them down.

Implemented versions:

  • SST-DDES
  • SST-IDDES
  • SST-SIDDES

I am also working on the Scale Adaptive Simulations (SAS) implementations for SST, following the work in "ON URANS SOLUTIONS WITH LES-LIKE BEHAVIOUR", Travis et al., and "[Evaluation of scale-adaptive simulation for transonic cavity flows", Babu et al., (https://www.inderscienceonline.com/doi/abs/10.1504/IJESMS.2016.075510). The last one, especially, is quite tricky due to the computation of the velocity laplacian. I tried computing it in Paraview as the divergence of the gradient field and it seems quite similar, but it is the first time that I've touched that section of the code, thus some work might still be needed.

Implemented versions:

  • SST-SAS_Simple (better naming is necessary), from the article of Travis et al.
  • SST-SAS_Complicated (better naming is necessary), it is related to the article from Babu et al.

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 --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • 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.

@rois1995 rois1995 changed the title [WIP] Implementation of DDES formulations for SST [WIP] Implementation of DDES and SAS formulations for SST Oct 23, 2023
Comment on lines 993 to 994
SAS_SIMPLE, /*!< \brief Menter k-w SST model with Scale Adaptive Simulations modifications. */
SAS_COMPLICATED, /*!< \brief Menter k-w SST model with Scale Adaptive Simulations modifications. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these the names in the literature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, these are just placeholders. There are no names in literature, that is why I named them according to the complexity of the model

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something after the authors since the models come from different papers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, why not

Comment on lines 1017 to 1020
SST_OPTIONS sasModel = SST_OPTIONS::SAS_SIMPLE; /*!< \brief Enum SST base model. */
bool sust = false; /*!< \brief Bool for SST model with sustaining terms. */
bool uq = false; /*!< \brief Bool for using uncertainty quantification. */
bool sas = false; /*!< \brief Bool for using Scale Adaptive Simulations. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably the type is enough and "none" indicates no sas.

Suggested change
SST_OPTIONS sasModel = SST_OPTIONS::SAS_SIMPLE; /*!< \brief Enum SST base model. */
bool sust = false; /*!< \brief Bool for SST model with sustaining terms. */
bool uq = false; /*!< \brief Bool for using uncertainty quantification. */
bool sas = false; /*!< \brief Bool for using Scale Adaptive Simulations. */
SST_OPTIONS sasModel = SST_OPTIONS::NONE; /*!< \brief Enum SST base model. */
bool sust = false; /*!< \brief Bool for SST model with sustaining terms. */
bool uq = false; /*!< \brief Bool for using uncertainty quantification. */

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll modify it.

Comment on lines 6160 to 6162
case SST_DDES: cout << "Delayed Detached Eddy Simulation (DDES) with Shear-layer Adapted SGS" << endl; break;
case SST_IDDES: cout << "Delayed Detached Eddy Simulation (DDES) with Shear-layer Adapted SGS" << endl; break;
case SST_SIDDES: cout << "Delayed Detached Eddy Simulation (DDES) with Shear-layer Adapted SGS" << endl; break;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description is slightly different right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I planned to modify the details on the final implementation. I'll do it now


su2double lengthScale_i, lengthScale_j;
su2double FTrans; /*!< \brief SAS function */
su2double VelLapl_X, VelLapl_Y, VelLapl_Z;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a matrix type for the laplacian in CVariable and here you can have a pointer instead.
Put these variables close to other turbulence variables please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the main problem with my implementation is that the norm of the velocity laplacian stated in the paper is $||U''||=\sqrt{\sum_i \left( \frac{\partial^2 U_i}{\partial x_j \partial x_j}\right)^2}$ and I am not sure that this is what I am computing. Do I need to store a matrix for each velocity component, compute it as gradient of the gradient and then sum the traces of each matrix?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like the norm of the Laplacian of the velocity components, the way you are computing it is not very accurate but maybe it is good enough. You could do it the finite volume way by reusing the numerics classes we use for scalar viscous fluxes.

Comment on lines 869 to 871
su2double VelLaplMag = VelLapl_X*VelLapl_X + VelLapl_Y*VelLapl_Y;
if (nDim == 3) VelLaplMag += VelLapl_Z*VelLapl_Z;
const su2double L_vK_1 = KolmConst * StrainMag_i / sqrt(VelLaplMag);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the velocity laplacian only used for the source term? Or will you have changes to the convection or diffusion fluxes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is just for the source term

Comment on lines 78 to 96
/*!
* \brief Get the value of the turbulence kinetic energy.
* \return the value of the turbulence kinetic energy.
*/
inline su2double GetSSTVariables_k(unsigned long iPoint) const { return k(iPoint); }

/*!
* \brief Get the value of the turbulence frequency Omega.
* \return the value of the turbulence frequency Omega.
*/
inline su2double GetSSTVariables_omega(unsigned long iPoint) const { return Omega(iPoint); }

/*!
* \brief Set the value of the SST variables computed with SA solution.
* \param[in] val_k
* \param[in] val_Omega
*/
void SetSSTVariables(unsigned long iPoint, su2double val_k, su2double val_Omega) {
k(iPoint) = val_k;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes related to the main theme? or were just debugging? If they can be separated to another PR let's do that please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are for computing the grid size for scale resolving simulations from an SA solution. Probably it has to be revised a little bit, since I found out that the formulation for the TKE is different if the QCR mod is used, thus I think I'll remove them for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to undo this change to the codi version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll do it

}
const su2double mut = Node_Flow->GetEddyViscosity(iPoint);
const su2double mu = Node_Flow->GetLaminarViscosity(iPoint);
const su2double LESIQ = 1.0/(1.0+0.05*pow((mut+mu)/mu, 0.53));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the constant here also used somewhere else? It would be good to avoid repeating hard-coded constants

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are used just here

Comment on lines 1682 to 1703
inline virtual su2double GetVelLapl_X(unsigned long iPoint) const { return 0.0; }
/*!
* \brief Get the value of the value of FTrans.
*/
inline virtual su2double GetVelLapl_Y(unsigned long iPoint) const { return 0.0; }
/*!
* \brief Get the value of the value of FTrans.
*/
inline virtual su2double GetVelLapl_Z(unsigned long iPoint) const { return 0.0; }

/*!
* \brief Set the value of the value of FTrans.
*/
inline virtual void AddVelLapl(unsigned long iPoint, su2double val_VelLapl_X, su2double val_VelLapl_Y) {}
/*!
* \brief Set the value of the value of FTrans.
*/
inline virtual void AddVelLapl_Z(unsigned long iPoint, su2double val_VelLapl_Z) {}
/*!
* \brief Set the value of the value of FTrans.
*/
inline virtual void SetVelLapl(unsigned long iPoint, su2double val_VelLapl_X, su2double val_VelLapl_Y) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a pointer for velocity and then you only need one of each function instead of handling xyz separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll try using the matrix type

- Changed names of SAS models
- Added descriptions of functions
- Clean up of output functions
- Removed SA functions or SST variables
/*--- If iPoint is boundary it only takes contributions from other boundary points. ---*/
if (boundary_i && !boundary_j) continue;

/*--- Add solution differences, with correction for compressible flows which use the enthalpy. ---*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if comments need updates please

@bigfooted
Copy link
Contributor

Hi @rois1995 any progress? Would be very interesting to add this to develop, looking forward to using it actually.

@rois1995
Copy link
Contributor Author

rois1995 commented Mar 7, 2024

Hi @rois1995 any progress? Would be very interesting to add this to develop, looking forward to using it actually.

Hi @bigfooted, unfortunately I do not have enough time/computational power to perform a proper validation. I only tried something with a NACA0021 at 17 deg and 60 deg and they were promising, at least for the DDES implementation. The SAS ones are not that good. I'll upload some post-processing as soon as I have the time to do so.

@Linnnnnn23
Copy link

I find your work really interesting. I've been studying the internal flow field in compressors and have had good results using SU2's SA_EDDES for calculating the cantilevered stator with a tip clearance. If you need help with code verification, I'd be glad to assist.

@rois1995
Copy link
Contributor Author

I find your work really interesting. I've been studying the internal flow field in compressors and have had good results using SU2's SA_EDDES for calculating the cantilevered stator with a tip clearance. If you need help with code verification, I'd be glad to assist.

Hi @Linnnnnn23, every help on the validation/verification is gladly accepted! Let me know if you need anything by my side.

@Linnnnnn23
Copy link

I find your work really interesting. I've been studying the internal flow field in compressors and have had good results using SU2's SA_EDDES for calculating the cantilevered stator with a tip clearance. If you need help with code verification, I'd be glad to assist.

Hi @Linnnnnn23, every help on the validation/verification is gladly accepted! Let me know if you need anything by my side.
Thank you for your response. Firstly, I would like to know what Verification and Validation (V&V) work has been conducted on the SST-based DDES (Delayed Detached Eddy Simulation) model to date. Secondly, we can provide a compressor cascade validation, with an inlet Mach number of 0.4, a Reynolds number of approximately 500,000, and a spanwise height of about 20% of the chord length, ensuring that the vortices resolved by DDES can develop in three dimensions. Thirdly, as I am a rookie to GitHub, I have not yet found out how to download your pull request code. For further communication, you can contact me via email at [email protected]

@BerkeCan97
Copy link
Contributor

Hi @rois1995 , I'd like to help with validation/verification if I can. Let me know if you are interested. You can contact me at [email protected].

@BerkeCan97
Copy link
Contributor

BerkeCan97 commented Oct 30, 2024

Hi @rois1995 any progress? Would be very interesting to add this to develop, looking forward to using it actually.

Hi @bigfooted, unfortunately I do not have enough time/computational power to perform a proper validation. I only tried something with a NACA0021 at 17 deg and 60 deg and they were promising, at least for the DDES implementation. The SAS ones are not that good. I'll upload some post-processing as soon as I have the time to do so.

Hi @rois1995, I believe there is an error or a typo in the Babu's paper in the Q_SAS source term and in turn in your implementation, which may be the reason for the poor performance in SAS results.

image

This term changes units depending on the output of the max function, which doesn't make any sense. I believe the terms in the max function should be divided by omega^2 and k^2 instead of omega and k, respectively. Which is exactly how it is done in https://resolver.tudelft.nl/uuid:5d23e2a6-5675-450d-bf3d-1dd40d736cae

I will try some of the benchmark cases in these papers when I have the time. Let me know what you think.

[edit: fixed the link to the tudelft repository]

@rois1995
Copy link
Contributor Author

rois1995 commented Apr 9, 2025

It seems that the problem is encountered on skew elements on periodic boundaries. I have tried running the SA-EDDES simulation for 30 timesteps with only one inner iteration (the problem remains when running for one temporal iteration with 30 inner iterations).

Periodic boundary
periodic

Center plane
center

The same problem remains with SA-URANS simulation.

Periodic boundary SA-URANS
periodic

The problem does not appear if Symmetry BCs are used instead of Periodic BCs.
periodic

EDIT:

I have tried with the SA-EDDES model and the problem still persists even with symmetry BCs, although it appears a little bit later.

@rois1995 rois1995 force-pushed the feature_SST_IDDES branch from 625ecb0 to 8f3efae Compare June 10, 2025 17:52
@pcarruscag
Copy link
Member

Is this one ready for review @rois1995 or still testing?

@rois1995
Copy link
Contributor Author

rois1995 commented Jul 7, 2025

Hi @pcarruscag, I am not planning on running other tests for now, I think that the results are good enough. For me, it is ready for review. Should I remove the debug variables/outputs before reviewing?

@pcarruscag
Copy link
Member

Yep please remove debug stuff

Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice additions 👍

Comment on lines 998 to 999
SAS_TRAVIS, /*!< \brief Menter k-w SST model with Scale Adaptive Simulations modifications. */
SAS_BABU, /*!< \brief Menter k-w SST model with Scale Adaptive Simulations modifications. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting

const bool sst_sas_travis = IsPresent(SST_OPTIONS::SAS_TRAVIS);
const bool sst_sas_babu = IsPresent(SST_OPTIONS::SAS_BABU);
if (sst_sas_travis && sst_sas_babu) {
SU2_MPI::Error("Two versions (Simple and Complicated) selected for SAS under SST_OPTIONS. Please choose only one.", CURRENT_FUNCTION);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update simple and complicated?

Comment on lines 1493 to 1497
SA_EDDES = 4, /*!< \brief Kind of Hybrid RANS/LES (SA - Delayed DES (DDES) with Shear Layer Adapted SGS: Enhanced DDES). */
SST_DDES = 5, /*!< \brief Kind of Hybrid RANS/LES (SST - Delayed DES (DDES): DDES). */
SST_IDDES = 6, /*!< \brief Kind of Hybrid RANS/LES (SST - Delayed DES (DDES): Improved DDES). */
SST_SIDDES = 7, /*!< \brief Kind of Hybrid RANS/LES (SST - Delayed DES (DDES): Simplified Improved DDES). */
SST_EDDES = 8, /*!< \brief Kind of Hybrid RANS/LES (SST - Delayed DES (DDES): Enhanced (SLA) DDES). */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting

Comment on lines 140 to 145
su2double FTrans; /*!< \brief SAS function */
su2double Q_SAS_1; /*!< \brief SAS function */
su2double Q_SAS_2; /*!< \brief SAS function */
su2double L; /*!< \brief SAS function */
su2double L_vK_1; /*!< \brief SAS function */
su2double L_vK_2; /*!< \brief SAS function */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these were here for debugging, please use local variables in the function where these quantities are used.


su2double dk = beta_star * Density_i * ScalarVar_i[1] * ScalarVar_i[0] * (1.0 + zetaFMt);
if (config->GetKind_HybridRANSLES() != NO_HYBRIDRANSLES)
dk = Density_i * sqrt(ScalarVar_i[0]*ScalarVar_i[0]*ScalarVar_i[0]) / lengthScale_i;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dk = Density_i * sqrt(ScalarVar_i[0]*ScalarVar_i[0]*ScalarVar_i[0]) / lengthScale_i;
dk = Density_i * sqrt(pow(ScalarVar_i[0], 3)) / lengthScale_i;

const su2double weight = halfOnVol;

const auto area = geometry->edges->GetNormal(iEdge);
AD::SetPreaccIn(area, nDim);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not doing anything because you are not starting/ending the preaccumulation.

Comment on lines 1190 to 1192
switch(kind_hybridRANSLES){
case SST_DDES: {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some links to the references you used for each formulation? (here or somewhere you think is more appropriate)

Comment on lines 72 to 85
Omega = sqrt(Vorticity[0]*Vorticity[0] + Vorticity[1]*Vorticity[1]+ Vorticity[2]*Vorticity[2]);

StrainDotVort[0] = Strain[0][0]*Vorticity[0]+Strain[0][1]*Vorticity[1]+Strain[0][2]*Vorticity[2];
StrainDotVort[1] = Strain[1][0]*Vorticity[0]+Strain[1][1]*Vorticity[1]+Strain[1][2]*Vorticity[2];
StrainDotVort[2] = Strain[2][0]*Vorticity[0]+Strain[2][1]*Vorticity[1]+Strain[2][2]*Vorticity[2];

numVecVort[0] = StrainDotVort[1]*Vorticity[2] - StrainDotVort[2]*Vorticity[1];
numVecVort[1] = StrainDotVort[2]*Vorticity[0] - StrainDotVort[0]*Vorticity[2];
numVecVort[2] = StrainDotVort[0]*Vorticity[1] - StrainDotVort[1]*Vorticity[0];

numerator = sqrt(6.0) * sqrt(numVecVort[0]*numVecVort[0] + numVecVort[1]*numVecVort[1] + numVecVort[2]*numVecVort[2]);
trace0 = 3.0*(pow(Strain[0][0],2.0) + pow(Strain[1][1],2.0) + pow(Strain[2][2],2.0));
trace1 = pow(Strain[0][0] + Strain[1][1] + Strain[2][2],2.0);
denominator = pow(Omega, 2.0) * sqrt(trace0-trace1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Geometry toolbox where possible please

Comment on lines 60 to 69
Strain[0][0] = PrimGrad_Flow[1][0];
Strain[1][0] = 0.5*(PrimGrad_Flow[2][0] + PrimGrad_Flow[1][1]);
Strain[0][1] = 0.5*(PrimGrad_Flow[1][1] + PrimGrad_Flow[2][0]);
Strain[1][1] = PrimGrad_Flow[2][1];
if (nDim == 3){
Strain[0][2] = 0.5*(PrimGrad_Flow[3][0] + PrimGrad_Flow[1][2]);
Strain[1][2] = 0.5*(PrimGrad_Flow[3][1] + PrimGrad_Flow[2][2]);
Strain[2][0] = 0.5*(PrimGrad_Flow[1][2] + PrimGrad_Flow[3][0]);
Strain[2][1] = 0.5*(PrimGrad_Flow[2][2] + PrimGrad_Flow[3][1]);
Strain[2][2] = PrimGrad_Flow[3][2];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have a helper function for this somewhere? Maybe in the viscous fluxes?

Copy link
Contributor Author

@rois1995 rois1995 Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that there is a ComputeMeanRateOfStrainMatrix in CNumerics, I will use that.

Comment on lines 53 to 60
AD::SetPreaccIn(PrimGrad_Flow, nDim+1, nDim);
AD::SetPreaccIn(Vorticity, 3);
/*--- Eddy viscosity ---*/
AD::SetPreaccIn(muT(iPoint));
/*--- Laminar viscosity --- */
AD::SetPreaccIn(LaminarViscosity);

Strain[0][0] = PrimGrad_Flow[1][0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to abstract the index for velocity somehow, otherwise this is not compatible with the NEMO solver.
Maybe change PrimGrad_Flow to VelGrad and whoever call this function is responsible by passing the correct part of the gradients.

@pcarruscag
Copy link
Member

Can you add a regression test?
Something of V&V quality (which you have already I think) would be awesome even if you don't have time to do a full write up on it.

@rois1995
Copy link
Contributor Author

I will definitively add a V&V case, although I do not have time at the moment. I will also review the suggestions as soon as possible.

Comment on lines 1099 to 1227
switch(kind_hybridRANSLES){
/*--- Every model is taken from "Development of DDES and IDDES Formulations for the k-ω Shear Stress Transport Model"
Mikhail S. Gritskevich et al. (DOI:10.1007/s10494-011-9378-4)
---*/
case SST_DDES: {

const su2double r_d = (eddyVisc + lamVisc) / max((KolmConst2*wallDist2 * sqrt(0.5 * (StrainMag*StrainMag + VortMag*VortMag))), 1e-10);
const su2double C_d1 = 20.0;
const su2double C_d2 = 3.0;

const su2double f_d = 1 - tanh(pow(C_d1 * r_d, C_d2));

const su2double l_LES = C_DES * h_max;

DES_lengthScale = l_RANS - f_d * max(0.0, l_RANS - l_LES);

break;
}
case SST_IDDES: {

// Constants
const su2double C_w = 0.15;
const su2double C_dt1 = 20.0;
const su2double C_dt2 = 3.0;
const su2double C_l = 5.0;
const su2double C_t = 1.87;

const su2double alpha = 0.25 - sqrt(wallDist2) / h_max;
const su2double f_b = min(2.0 * exp(-9.0 * alpha*alpha), 1.0);
const su2double r_dt = eddyVisc / max((KolmConst2*wallDist2 * sqrt(0.5 * (StrainMag*StrainMag + VortMag*VortMag))), 1e-10);
const su2double f_dt = 1 - tanh(pow(C_dt1 * r_dt, C_dt2));
const su2double ftilda_d = max(1.0 - f_dt, f_b);

const su2double r_dl = lamVisc / max((KolmConst2*wallDist2 * sqrt(0.5 * (StrainMag*StrainMag + VortMag*VortMag))), 1e-10);
const su2double f_l = tanh(pow(C_l*C_l*r_dl, 10.0));
const su2double f_t = tanh(pow(C_t*C_t*r_dt, 3.0));
const su2double f_e2 = 1.0 - max(f_t, f_l);
const su2double f_e1 = alpha >= 0.0 ? 2.0 * exp(-11.09*alpha*alpha) : 2.0 * exp(-9.0*alpha*alpha);
const su2double f_e = f_e2 * max((f_e1 - 1.0), 0.0);


const su2double Delta = min(C_w * max(sqrt(wallDist2), h_max), h_max);
const su2double l_LES = C_DES * Delta;

DES_lengthScale = ftilda_d *(1.0+f_e)*l_RANS + (1.0 - ftilda_d) * l_LES;

break;
}
case SST_SIDDES: {

// Constants
const su2double C_w = 0.15;
const su2double C_dt1 = 20.0;
const su2double C_dt2 = 3.0;

const su2double alpha = 0.25 - sqrt(wallDist2) / h_max;
const su2double f_b = min(2.0 * exp(-9.0 * alpha*alpha), 1.0);
const su2double r_dt = eddyVisc / max((KolmConst2*wallDist2 * sqrt(0.5 * (StrainMag*StrainMag + VortMag*VortMag))), 1e-10);
const su2double f_dt = 1 - tanh(pow(C_dt1 * r_dt, C_dt2));
const su2double ftilda_d = max(1.0 - f_dt, f_b);

const su2double Delta = min(C_w * max(sqrt(wallDist2), h_max), h_max);
const su2double l_LES = C_DES * Delta;

DES_lengthScale = ftilda_d*l_RANS + (1.0 - ftilda_d) * l_LES;

break;
}
case SST_EDDES: {

// Improved DDES version with the Shear-Layer-Adapted augmentation
// found in Detached Eddy Simulation: Recent Development and Application to Compressor Tip Leakage Flow, Xiao He, Fanzhou Zhao, Mehdi Vahdati
// originally from Application of SST-Based SLA-DDES Formulation to Turbomachinery Flows, Guoping Xia, Zifei Yin and Gorazd Medic
// I could be naming it either as SST_EDDES to follow the same notation as for the SA model or as SST_SLA_DDES to follow the paper notation

const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;

const auto nNeigh = geometry->nodes->GetnPoint(iPoint);

su2double vortexTiltingMeasure = nodes->GetVortex_Tilting(iPoint);

su2double deltaOmega = -1.0;
su2double vorticityDir[MAXNDIM] = {};

for (auto iDim = 0; iDim < MAXNDIM; iDim++){
vorticityDir[iDim] = Vorticity[iDim]/VortMag;
}

for (const auto jPoint : geometry->nodes->GetPoints(iPoint)){
const auto coord_j = geometry->nodes->GetCoord(jPoint);

for (const auto kPoint : geometry->nodes->GetPoints(iPoint)){
const auto coord_k = geometry->nodes->GetCoord(kPoint);

su2double delta[MAXNDIM] = {};
for (auto iDim = 0u; iDim < MAXNDIM; iDim++){
// TODO: Should I divide by 2 as I am interested in the dual volume (the edge is split at midpoint)?
delta[iDim] = (coord_j[iDim] - coord_k[iDim])/2.0;
}
su2double l_n_minus_m[MAXNDIM];
GeometryToolbox::CrossProduct(delta, vorticityDir, l_n_minus_m);
deltaOmega = max(deltaOmega, GeometryToolbox::Norm(3, l_n_minus_m));
}

// Add to VTM(iPoint) to perform the average
vortexTiltingMeasure += nodes->GetVortex_Tilting(jPoint);
}
deltaOmega /= sqrt(3.0);
vortexTiltingMeasure /= (nNeigh+1);

const su2double f_kh = max(f_min,
min(f_max,
f_min + ((f_max - f_min)/(a2 - a1)) * (vortexTiltingMeasure - a1)));

const su2double r_d = (eddyVisc + lamVisc) / max((KolmConst2*wallDist2 * sqrt(0.5 * (StrainMag*StrainMag + VortMag*VortMag))), 1e-10);
const su2double C_d1 = 20.0;
const su2double C_d2 = 3.0;

const su2double f_d = 1 - tanh(pow(C_d1 * r_d, C_d2));

su2double delta = deltaOmega * f_kh;
if (f_d < 0.99){
delta = h_max;
}

const su2double l_LES = C_DES * delta;
DES_lengthScale = l_RANS - f_d * max(0.0, l_RANS - l_LES);
}
}

Check notice

Code scanning / CodeQL

Long switch case Note

Switch has at least one case that is too long:
SST_EDDES (59 lines)
.
Switch has at least one case that is too long:
SST_EDDES_UNSTR (59 lines)
.

Copilot Autofix

AI 6 days ago

To fix the long case, we should extract the entirety of the logic inside the case SST_EDDES: block (lines 1169–1228) into a dedicated private helper method, e.g., CalcLengthScaleSST_EDDES(...). All the variables that this logic needs—locals from the function, members, and parameters from the loop—must be passed as parameters if they aren't accessible as class members. In this case, nearly all dependent variables are either available in the class or in function scope from the loop, or can be passed explicitly (typical parameters: indices, node pointers, relevant variables).

We'll insert the helper as a private method in the class' source file, just before or after SetDES_LengthScale, and replace the original content of case SST_EDDES: with a single call to this function. No changes to declarations in header files are required if marked as private/protected for internal use (some projects prefer explicitly listing such helpers in the header; however, only the implementation file is available, so we’ll keep all changes local).


Suggested changeset 1
SU2_CFD/src/solvers/CTurbSSTSolver.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/SU2_CFD/src/solvers/CTurbSSTSolver.cpp b/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
--- a/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
+++ b/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
@@ -1167,64 +1167,12 @@
         break;
       }
       case SST_EDDES: {
-
-        // Improved DDES version with the Shear-Layer-Adapted augmentation 
-        // found in Detached Eddy Simulation: Recent Development and Application to Compressor Tip Leakage Flow, Xiao He, Fanzhou Zhao, Mehdi Vahdati
-        // originally from Application of SST-Based SLA-DDES Formulation to Turbomachinery Flows, Guoping Xia, Zifei Yin and Gorazd Medic
-        // I could be naming it either as SST_EDDES to follow the same notation as for the SA model or as SST_SLA_DDES to follow the paper notation
-
-        const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;
-
-        const auto nNeigh = geometry->nodes->GetnPoint(iPoint);
-
-        su2double vortexTiltingMeasure = nodes->GetVortex_Tilting(iPoint);
-
-        const su2double omega = GeometryToolbox::Norm(3, Vorticity);
-
-        su2double ratioOmega[MAXNDIM] = {};
-
-        for (auto iDim = 0u; iDim < MAXNDIM; iDim++){
-          ratioOmega[iDim] = Vorticity[iDim]/omega;
-        }
-
-        const su2double deltaDDES = geometry->nodes->GetMaxLength(iPoint);
-
-        su2double ln_max = 0.0;
-        for (const auto jPoint : geometry->nodes->GetPoints(iPoint)) {
-          const auto coord_j = geometry->nodes->GetCoord(jPoint);
-          su2double delta[MAXNDIM] = {};
-          for (auto iDim = 0u; iDim < nDim; iDim++){
-            delta[iDim] = fabs(coord_j[iDim] - coord_i[iDim]);
-          }
-          su2double ln[3];
-          ln[0] = delta[1]*ratioOmega[2] - delta[2]*ratioOmega[1];
-          ln[1] = delta[2]*ratioOmega[0] - delta[0]*ratioOmega[2];
-          ln[2] = delta[0]*ratioOmega[1] - delta[1]*ratioOmega[0];
-          const su2double aux_ln = sqrt(ln[0]*ln[0] + ln[1]*ln[1] + ln[2]*ln[2]);
-          ln_max = max(ln_max, aux_ln);
-          vortexTiltingMeasure += nodes->GetVortex_Tilting(jPoint);
-        }
-        vortexTiltingMeasure /= (nNeigh + 1);
-
-        
-
-        const su2double f_kh = max(f_min,
-                                   min(f_max,
-                                       f_min + ((f_max - f_min)/(a2 - a1)) * (vortexTiltingMeasure - a1)));
-
-        const su2double r_d = (eddyVisc + lamVisc) / max((KolmConst2*wallDist2 * sqrt(0.5 * (StrainMag*StrainMag + VortMag*VortMag))), 1e-10);
-        const su2double C_d1 = 20.0;
-        const su2double C_d2 = 3.0;
-
-        const su2double f_d = 1 - tanh(pow(C_d1 * r_d, C_d2));
-
-        su2double delta = (ln_max/sqrt(3.0)) * f_kh;
-        if (f_d < 0.99){
-          delta = h_max;
-        }
-
-        const su2double l_LES = C_DES * delta;
-        DES_lengthScale = l_RANS - f_d * max(0.0, l_RANS - l_LES);
+        DES_lengthScale = CalcLengthScaleSST_EDDES(
+          geometry, nodes, iPoint, coord_i, Vorticity, nDim,
+          eddyVisc, lamVisc, KolmConst2, wallDist2, StrainMag, VortMag,
+          h_max, C_DES, l_RANS
+        );
+        break;
       }
       case SST_EDDES_UNSTR: {
 
@@ -1294,6 +1242,70 @@
   END_SU2_OMP_FOR
 }
 
+// Helper function to encapsulate the SST_EDDES case logic from SetDES_LengthScale
+su2double CTurbSSTSolver::CalcLengthScaleSST_EDDES(
+  CGeometry* geometry, CTurbSSTVariable* nodes, unsigned long iPoint,
+  const su2double* coord_i, const su2double* Vorticity, unsigned short nDim,
+  su2double eddyVisc, su2double lamVisc, su2double KolmConst2, su2double wallDist2,
+  su2double StrainMag, su2double VortMag,
+  su2double h_max, su2double C_DES, su2double l_RANS
+){
+  // Improved DDES version with the Shear-Layer-Adapted augmentation 
+  // found in Detached Eddy Simulation: Recent Development and Application to Compressor Tip Leakage Flow, Xiao He, Fanzhou Zhao, Mehdi Vahdati
+  // originally from Application of SST-Based SLA-DDES Formulation to Turbomachinery Flows, Guoping Xia, Zifei Yin and Gorazd Medic
+  // I could be naming it either as SST_EDDES to follow the same notation as for the SA model or as SST_SLA_DDES to follow the paper notation
+
+  const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;
+
+  const auto nNeigh = geometry->nodes->GetnPoint(iPoint);
+
+  su2double vortexTiltingMeasure = nodes->GetVortex_Tilting(iPoint);
+
+  const su2double omega = GeometryToolbox::Norm(3, Vorticity);
+
+  su2double ratioOmega[MAXNDIM] = {};
+  for (auto iDim = 0u; iDim < MAXNDIM; iDim++){
+    ratioOmega[iDim] = Vorticity[iDim]/omega;
+  }
+
+  // const su2double deltaDDES = geometry->nodes->GetMaxLength(iPoint); // Not used
+
+  su2double ln_max = 0.0;
+  for (const auto jPoint : geometry->nodes->GetPoints(iPoint)) {
+    const auto coord_j = geometry->nodes->GetCoord(jPoint);
+    su2double delta[MAXNDIM] = {};
+    for (auto iDim = 0u; iDim < nDim; iDim++){
+      delta[iDim] = fabs(coord_j[iDim] - coord_i[iDim]);
+    }
+    su2double ln[3];
+    ln[0] = delta[1]*ratioOmega[2] - delta[2]*ratioOmega[1];
+    ln[1] = delta[2]*ratioOmega[0] - delta[0]*ratioOmega[2];
+    ln[2] = delta[0]*ratioOmega[1] - delta[1]*ratioOmega[0];
+    const su2double aux_ln = sqrt(ln[0]*ln[0] + ln[1]*ln[1] + ln[2]*ln[2]);
+    ln_max = max(ln_max, aux_ln);
+    vortexTiltingMeasure += nodes->GetVortex_Tilting(jPoint);
+  }
+  vortexTiltingMeasure /= (nNeigh + 1);
+
+  const su2double f_kh = max(f_min,
+                             min(f_max,
+                                 f_min + ((f_max - f_min)/(a2 - a1)) * (vortexTiltingMeasure - a1)));
+
+  const su2double r_d = (eddyVisc + lamVisc) / max((KolmConst2*wallDist2 * sqrt(0.5 * (StrainMag*StrainMag + VortMag*VortMag))), 1e-10);
+  const su2double C_d1 = 20.0;
+  const su2double C_d2 = 3.0;
+
+  const su2double f_d = 1 - tanh(pow(C_d1 * r_d, C_d2));
+
+  su2double delta = (ln_max/sqrt(3.0)) * f_kh;
+  if (f_d < 0.99){
+    delta = h_max;
+  }
+
+  const su2double l_LES = C_DES * delta;
+  return l_RANS - f_d * max(0.0, l_RANS - l_LES);
+}
+
 void CTurbSSTSolver::ComputeUnderRelaxationFactor(const CConfig *config) {
 
   const su2double allowableRatio = config->GetMaxUpdateFractionSST();
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
@rois1995
Copy link
Contributor Author

rois1995 commented Oct 3, 2025

Should we allow DDES to be run on 2D meshes? The DDES flatplate testcase has a 2D mesh, but it does not really make any sense to run 2D cases with DDES.

@pcarruscag
Copy link
Member

If someone is determined enough they'll make a 2.5D mesh and still run it 😄
Maybe you can just print a big warning but allow it to run? It could be useful for regression tests.
Is there anything in the implementation that only works for 3D?

@rois1995
Copy link
Contributor Author

rois1995 commented Oct 6, 2025

I am pretty sure that the EDDES models have something that does not work in 2D, like the vortex-tilting measure (depends on the scalar product of the gradient of the velocity and the vorticity, thus in 2D it should be zero).


const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;

const auto nNeigh = geometry->nodes->GetnPoint(iPoint);

Check notice

Code scanning / CodeQL

Declaration hides variable Note

Variable nNeigh hides another variable of the same name (on
line 1080
).

Copilot Autofix

AI 6 days ago

To fix the problem, we should rename one of the nNeigh declarations to prevent variable shadowing. Since the outer nNeigh (line 1080) is defined at the start of the large loop and is used more broadly, it is best to rename the inner declaration (line 1178, inside the SST_EDDES case clause) to a distinct name such as nNeigh_EDDES.

Steps:

  • Locate the inner declaration at line 1178: const auto nNeigh = geometry->nodes->GetnPoint(iPoint);
  • Change it to, e.g., const auto nNeigh_EDDES = geometry->nodes->GetnPoint(iPoint);
  • Update any usage of nNeigh within this case SST_EDDES block to nNeigh_EDDES.
  • No new methods, imports, or definitions are required, just a rename for clarity.

Suggested changeset 1
SU2_CFD/src/solvers/CTurbSSTSolver.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/SU2_CFD/src/solvers/CTurbSSTSolver.cpp b/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
--- a/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
+++ b/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
@@ -1175,7 +1175,7 @@
 
         const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;
 
-        const auto nNeigh = geometry->nodes->GetnPoint(iPoint);
+        const auto nNeigh_EDDES = geometry->nodes->GetnPoint(iPoint);
 
         su2double vortexTiltingMeasure = nodes->GetVortex_Tilting(iPoint);
 
EOF
@@ -1175,7 +1175,7 @@

const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;

const auto nNeigh = geometry->nodes->GetnPoint(iPoint);
const auto nNeigh_EDDES = geometry->nodes->GetnPoint(iPoint);

su2double vortexTiltingMeasure = nodes->GetVortex_Tilting(iPoint);

Copilot is powered by AI and may make mistakes. Always verify output.

const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;

const auto nNeigh = geometry->nodes->GetnPoint(iPoint);

Check notice

Code scanning / CodeQL

Declaration hides variable Note

Variable nNeigh hides another variable of the same name (on
line 1080
).

Copilot Autofix

AI 6 days ago

To fix the variable shadowing, the best approach is to rename the inner variable declared at line 1238 within the case SST_EDDES_UNSTR: block to a distinct and descriptive name, e.g., nNeigh_case or something more reflective of its purpose within the block. This avoids hiding the nNeigh variable declared at line 1080 in the surrounding loop. Only the definition at line 1238, and any subsequent usages of it within the same case block, should be renamed. No functionality should be changed—only variable naming within the shown code region. No changes to imports or class definitions are needed.


Suggested changeset 1
SU2_CFD/src/solvers/CTurbSSTSolver.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/SU2_CFD/src/solvers/CTurbSSTSolver.cpp b/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
--- a/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
+++ b/SU2_CFD/src/solvers/CTurbSSTSolver.cpp
@@ -1235,7 +1235,7 @@
 
         const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;
 
-        const auto nNeigh = geometry->nodes->GetnPoint(iPoint);
+        const auto nNeigh_case = geometry->nodes->GetnPoint(iPoint);
 
         su2double vortexTiltingMeasure = nodes->GetVortex_Tilting(iPoint);
 
EOF
@@ -1235,7 +1235,7 @@

const su2double f_max = 1.0, f_min = 0.1, a1 = 0.15, a2 = 0.3;

const auto nNeigh = geometry->nodes->GetnPoint(iPoint);
const auto nNeigh_case = geometry->nodes->GetnPoint(iPoint);

su2double vortexTiltingMeasure = nodes->GetVortex_Tilting(iPoint);

Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants