[WIP] SA turb model consistent implementation

[suargi]

Implementation of the original SA and Negative SA turbulence models as well as their no-ft2 versions

Proposed Changes

Implementation of the original version of Spalart-Allmaras turbulence model (https://www.researchgate.net/publication/236888804_A_One-Equation_Turbulence_Model_for_Aerodynamic_Flows).
Implementation of the Spalart-Allmaras turbulence model without the ft2 term.
Implementation of the original version of Negative Spalart-Allmaras turbulence model (https://www.iccfd.org/iccfd7/assets/pdf/papers/ICCFD7-1902_paper.pdf)
Implementation of the Negative Spalart-Allmaras turbulence model without the ft2 term neither modified vorticity.

On the config file, choose:
SA: Original SA model
SA_NOFT2: SA model without ft2 term (the same model found in the develop branch)
SA_NEG: Original negative SA model
SA_NEG_NOFT2: Negative SA model (the same model found in the develop branch)

They are not the same as in the develop branch, because I corrected a minor bug.

Updated the template config file to be in accordance with the source code: turbulent models and OUTPUT_FILES tags

Related Work

This PR resolves issues #992 and #1051.

Regression testing

Tested on tutorials:

• Turbulent Flat Plate
• RAE2822
• Turbulent ONERA M6

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.
• 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.

[pcarruscag]

Thank you for taking the lead on this @suargi , this is important work for all the reasons @economon listed in #992.
My requests are towards making sure this is integrated in the code in the safest, most sustainable way possible.

[pcarruscag]

This way of handling the new options is a no go on my part, if we do it like this we will be chasing SA bugs for months, exactly like it happened when the solver options changed. Even with SST_SUST which was just one new option I fixed a bug last month or so.

I trust you to have changed all places in the code, but other people will not be aware (because very few people follow the pull requests) and unfortunately there are 10's of branches that do not track develop closely.

So, as a bare minimum this "is SA", "is SA-neg" logic needs to be encapsulated in a single method of CConfig, and replaced everywhere.
If you want to take it further we should create a small lightweight class that condenses all SA info, including coefficients.

Actually even less work, and probably more appropriate, create a boolean option called SA_USE_FT2= YES/NO, and avoid changing all the enum's and turbulence options.

[suargi]

I totally agree with that. I would suggest to follow the comment from @clarkpede on issue #992 so we are able to include more than one variation.
Maybe on the config file we can include a tag for all the SA variations. Something like:

SA_VARIATIONS: NO_FT2, COMP, MOD_VORT, etc

which are basically Boolean to identify whether we want to use no ft2, comp, modified vorticity, etc.

Then, on turb_sources to have only one class, the original SA model. Therein, include all possible variations.

[pcarruscag]

Yes that sounds reasonable for the options and there is precedent with the QCR modification of the stress tensor.

However, having classes that look like Swiss army knives by means of if's and switch's is a bad software practice IMO.
There are "modern" and very efficient ways to specialize such small aspects, I'm using (and have described) some in #1022, we can discuss them in the next developers meeting if you want.

[suargi]

I will take a look to that!

[pcarruscag]

👍 Do we have a "MESH" option?

[suargi]

https://github.com/su2code/SU2/blob/develop/Common/include/option_structure.hpp line 1631

[pcarruscag]

Indeed but what matters are the strings from 1642 to 1659, the enums are for internal use.

[suargi]

Okay, but "MESH" is indeed an output option, see https://github.com/su2code/SU2/blob/develop/SU2_CFD/src/output/COutput.cpp line 388. Maybe the pairing in option_structure is missing?

[pcarruscag]

It is only used internally in SU2_DEF I think, it does not need to be selected by users.

[suargi]

Good. So shall be removed from the config_template.

[koodlyakshay]

A small comment.
It looks like the BC transition model is only valid with the no-ft2 version (https://turbmodels.larc.nasa.gov/sa-bc_1eqn.html).

[suargi]

Thanks for pointing it out.
"the ft2 term present in (SA) may cause an undesirable delay in transition to turbulence in the RANS region, and should be avoided".
I will take it into account.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still a relevant issue please comment on it to restart the discussion. Thank you for your contributions.

[suargi]

Currently each variation/correction model has its unique name identification in SU2. In order to simultaneously combine multiple correction models to a given turbulence model I suggest the following:

• The config file option KIND_TURB_MODEL should specify only which turbulence model to use, i.e. SA or SST.
• Introduce an additional config file field TURB_MODEL_CORRECTIONS to indicate all variations/corrections to be considered.

Here an example,
% Specify turbulent model (NONE, SA, SST)
KIND_TURB_MODEL= SA
% Turbulence model corrections (NONE, SA-EDW, SA-NOFT2, SA-COMP, SA-NEG, SA-QCR2000, SST-SUST)
TURB_MODEL_CORRECTIONS= SA-EDW, SA-NEG

What do you think?