Fix for turbulent viscosity calculation in SST Solver

[jayantmukho]

Proposed Changes

In a previous PR the SST model implementation in the CTurbSSTSolver was corrected to use the vorticity magnitude to calculate the turbulent viscosity (mu_T). But this change was not copied in the relevant numerics class where mu_T is used to calculate production terms. This is probably going to break all the SST related regression tests, so I will ensure convergence of those cases and update regression values.

Related Work

A relevant discussion to have here is whether to update the SST turbulence model according to a 2003 paper by Menter (author of the SST model): Ten years of industrial experience with the SST turbulence model

It involves tiny changes to coefficients, production limits, and uses strain magnitude instead of the vorticity magnitude to calculate mu_T. The last point means reversing the change that this PR is making. But currently the model is just inconsistent and needs to be corrected.

The changes are supposed to make the model more stable, which has been a cause for concern with the SST model before. Not sure if people have thoughts about altering the original turbulence model.

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.

[bmunguia]

@jayantmukho LGTM. I'm also in favor of moving to SST-2003 if it means a more robust SST solver.

This might also be a good time to continue to look into other issues with SST, e.g. some TKE terms mentioned in #797.

Another possible discrepancy I noticed is that in CTurbSSTVariable we have

CDkw(iPoint) = 0.0;
for (unsigned long iDim = 0; iDim < nDim; iDim++)
    CDkw(iPoint) += Gradient(iPoint,0,iDim)*Gradient(iPoint,1,iDim);
CDkw(iPoint) *= 2.0*val_density*sigma_om2/Solution(iPoint,1);
CDkw(iPoint) = max(CDkw(iPoint), pow(10.0, -20.0));
...
arg1 = min(arg2, 4.0*val_density*sigma_om2*Solution(iPoint,0) / (CDkw(iPoint)*val_dist*val_dist+EPS*EPS));
F1(iPoint) = tanh(pow(arg1, 4.0));

which is consistent with TMR, but when computing the cross diffusion residual we have

val_residual[1] += (1.0 - F1_i)*CDkw_i*Volume;

However, here TMR (as well as both SST-1993 and SST-2003) has

So to be consistent with TMR, would it make more sense to have e.g.

CDkw(iPoint) = 0.0;
for (unsigned long iDim = 0; iDim < nDim; iDim++)
    CDkw(iPoint) += Gradient(iPoint,0,iDim)*Gradient(iPoint,1,iDim);
CDkw(iPoint) *= 2.0*val_density*sigma_om2/Solution(iPoint,1);
...
arg1 = min(arg2, 4.0*val_density*sigma_om2*Solution(iPoint,0) / (max(CDkw(iPoint),pow(10.0,-10.0))*val_dist*val_dist+EPS*EPS));
F1(iPoint) = tanh(pow(arg1, 4.0));

so we don't clip the cross diffusion residual? Not sure how much of a difference this makes (or other discrepenacies like the TKE issue).

[economon]

Looks to be a good catch, @jayantmukho, thanks.

I am not sure how often that max is active, but given that a number of the SST tests are failing, it is not negligible. Have you looked at any comparisons with the prior version? Might be worth rerunning the V&V cases linked to the main project page to see (or at least the flat plate).

As for adding new SST variants, I am all for it! However, I would suggest that we keep the original "Standard" SST model and add the new versions alongside it, like we have done with SA. It is not too many lines of code that need to change, but we should find a clean way to do it (without duplicating code).

[clarkpede]

I just checked the math, and it looks good to me. With regards to the cross-diffusion terms: Without testing it myself, I'm not sure about what is the "correct" way to clip these terms.

Have you checked the difference in quantities of interest, such as skin friction coefficient, pressure coefficient, lift, and drag, on fully converged regression test cases? I would specifically be interested in the RAE2822 case.

Re-doing the review.

[pcarruscag]

Just a couple of non scientific comments.

• The release after this kind of change should get more than a minor version increase, so if the changes are localized it may be worth waiting and bundling them with other significant ones (e.g. removing some legacy features).
• If you guys add more SST variants let's please do it in a smarter way than case SST: case SST_2003: ... as we are still seeing bug fixes from the last time we changed that kind of enum... I'm happy to help implement something like Cleanup bit flags #770.

[jayantmukho]

Sorry took a while to get around to it but here are the results from the validation cases that were requested. Turns out, this change is minimal in its effect to the overall simulation (at least for the two cases I have run)

Flatplate

For the flatplate case, there was no difference in the C_D calculation so the lines mesh convergence were on top of each other. But while I was doing that, I was playing around with the solver parameters and there was an interesting difference between simulations run with WEIGHTED_LEAST_SQUARES and those run with GREEN_GAUSS. I have included them here:

This shows interesting parallels between WEIGHTED_LEAST_SQUARES results and CFL3D results vs GREEN_GAUSS and FUN3D. Also interesting the SU2 with both gradient methods converges to a slightly different C_f value than CFL3D and FUN3D. Not really connected with this PR but interesting results that can be uploaded to the VandV section of the website.

RAE 2822

Luckily I have been working on the RAE2822 as a validation case so I already had a family of grids that worked for this case. Again seems like the change didn't make much of a difference. dev_SST refers to the current SST implementation in develop. SST refers to the results from this PR

Consequently, the C_P plots are also identical

[clarkpede]

Ok. Unless there's significant differences in any of the other regression test cases, then this PR looks good to me.

[economon]

Hmm.. you are right that the Cf results look a little different than the ones posted on the V&V page (for both SA and SST). Is there anything else different in the configuration for those cases from the set up for the cases on the V&V page? For the flat plate, I may have used unweighted LSQ for the posted V&V results.

[jayantmukho]

@economon, from the config files in the V&V folder, it seems like you ran without adaptive CFL and without the unweighted LSQ.

The main difference between the V&V runs and these ones is that I don't use any slope limiter. In the V&V cases the SLOPE_LIMITER_TURB= VENKATAKRISHNAN option was used.

There are some other differences:

1. I use adaptive CFL
2. I use REF_DIMENSIONALIZATION= FREESTREAM_PRESS_EQ_ONE
3. Linear solver settings are slightly different. For the V&V runs the following are used:

% Minimum error of the linear solver for implicit formulations
LINEAR_SOLVER_ERROR= 1E-15
%
% Max number of iterations of the linear solver for the implicit formulation
LINEAR_SOLVER_ITER= 25

For my runs, I used the settings you had suggested in #790 :

% Minimum error of the linear solver for implicit formulations
LINEAR_SOLVER_ERROR= 1E-10
%
% Max number of iterations of the linear solver for the implicit formulation
LINEAR_SOLVER_ITER= 10

4. For the SST runs, I did not specify these options that you used in the V&V case:

% Free-stream turbulence intensity for the SST model
< FREESTREAM_TURBULENCEINTENSITY = 0.00038729
< %
< % Free-strem turbulence to laminar viscosity ratio for the SST model
< FREESTREAM_TURB2LAMVISCRATIO   = 0.009

I have attached the config files I used. I am still working on confirming all the regression tests, everything seems to be reasonable thus far.

flatplate_configs.zip

[jayantmukho]

Updated all the regression test values. Ran all of them to convergence and the difference in the integrated quantities (lift, drag, sens_press, entropy generation etc) were < 1%.

While running the regression tests, I saw that some of them weren't up to date with the best practices from the recent PRs. I know they are for Regressions and not necessarily as example configs, but I think it'd be good to have the configs reflect best practices so that people can look to them for reference.

If people are on board with that, I can update some of the rans configs to include things like ILU, adaptive CFL, the new convergence monitoring systems etc. I will make sure that the tests still cover all the relevant config options. I will also make sure that if you were to run the test individually, it would run to convergence.

[pcarruscag]

I think coverage should take priority over best practices.

[talbring]

Looks ok to me!

[talbring]

Remember to change that