Fixed values for turbulence quantities in upstream half-plane #1236

maxaehle · 2021-03-16T08:28:58Z

Proposed Changes

On a user-specified upstream half-plane, fix the values of turbulence quantities to their far-field values, instead of integrating the turbulence model equations.

In the far-field flow, the production terms of SST for K and omega are zero, while destruction still takes place. Thus the turbulence quantities typically decay on their way from the far-field boundary to the airfoil, until they reach the lowerlimit. See for instance this turbulence kinetic energy plot on the NACA0012 mesh:

This has been presented in [1] together with two solution approaches:

Sustaining terms, which are additional source terms compensating the destruction terms in the far-field flow. This approach has been already implemented with the KIND_TURB_MODEL= SST_SUST option.
Floor values, which is equivalent to set the lowerlimit to the far-field values in the upstream region of the airfoil.
This PR adds floor values, but in the form of fixed values: The turbulence quantities are just set to the far-field values in the upstream region. This has been implemented in analogy to the strong boundary conditions.

The correction can be activated like this:

TURB_FIXED_VALUES= YES
TURB_FIXED_VALUES_DOMAIN= -1.0

The second parameter specifies the half-plane on which fixed values are applied. This half-plane consists of those points for which the dot product of their coordinate vector with the normalized far-field velocity vector is less than TURB_FIXED_VALUES_DOMAIN.

This is the turbulence kinetic energy with AoA=0° and 10°:

The SA model does not suffer from a decaying turbulence variable, but the proposed changes are also functional there.

[1] Philippe R. Spalart and Christopher L. Rumsey, Effective Inflow Conditions for Turbulence Models in Aerodynamic Calculations, https://arc.aiaa.org/doi/pdf/10.2514/1.29373

Related Work

?

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.

[X ] 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.

pcarruscag · 2021-03-16T10:21:27Z

Common/include/CConfig.hpp

+   * \details This half-plane is given by the condition that the dot product between the
+   * coordinate vector and the normalized far-field velocity vector is less than what this
+   * function returns.


Elegant 👍 Hopefully not too much :)

Common/src/CConfig.cpp

SU2_CFD/include/solvers/CTurbSolver.hpp

SU2_CFD/src/solvers/CTurbSolver.cpp

SU2_CFD/src/solvers/CTurbSSTSolver.cpp

SU2_CFD/src/integration/CIntegration.cpp

maxaehle · 2021-03-16T13:57:37Z

Do you think a test case for this is necessary?

pcarruscag · 2021-03-16T14:14:54Z

Thank you for the changes.
I would say a test case is always welcome. You can simply modify an existing one, this feature is orthogonal to everything else, and then please add the new options to the config_template (with maybe the nice explanation you have in CConfig).

pcarruscag · 2021-03-16T14:18:35Z

SU2_CFD/src/solvers/CTurbSSTSolver.cpp

+    if(config->GetTurb_Fixed_Values()){
+      const su2double* velocity_inf = config->GetVelocity_FreeStreamND();
+      su2double sp =
+        GeometryToolbox::DotProduct(nDim, geometry->nodes->GetCoord(iPoint), velocity_inf)


Would it be appropriate to take the coordinate relative to the reference point provided in the config?
I guess in practice one would set the plane offset very large, but you never know what folks will do with your code (for better or worse).

Hmmmm you now have to check for zero velocity in two places as well...

Right ... I think I will revert the changes in CTurbSSTSolver::Source_Residual, so that the source residual is computed even if the residual is overwritten later.

How is a reference point specified in the config?

Probably the REF_ORIGIN_MOMENT_X, _Y, _Z. Yes giving the shift relative to these can be a sensible choice, let's discuss in the meeting.

pcarruscag

LGTM but present it in the meeting tomorrow before you merge.

modification of the serial testcase turb_naca0012_sst

…lues

maxaehle · 2021-03-17T17:49:13Z

After looking for testcases to modify, I think it is better to have the modified testcase in addition to, and not besides, the original one. Other users might want to build their cfg file on top of the original testcase cfg file, and could be surprised by the additional feature.

maxaehle · 2021-03-17T18:39:30Z

Now the PR is ready to be merged from my perspective.

pcarruscag · 2021-03-17T19:09:52Z

Alright 👍

TobiKattmann

Thanks @maxaehle for this addition 💐 lgtm with good style overall 👍

TobiKattmann · 2021-03-17T21:43:26Z

Common/src/CConfig.cpp

@@ -4364,6 +4370,10 @@ void CConfig::SetPostprocessing(unsigned short val_software, unsigned short val_
    SU2_MPI::Error("The LM transition model is under maintenance.", CURRENT_FUNCTION);
  }

+  if(Turb_Fixed_Values && Turb_Fixed_Values_MaxScalarProd==numeric_limits<su2double>::lowest()){


fyi: there is an OptionIsSet("TURB_FIXED_VALUES_DOMAIN") method that expresses directly what you want here.

TobiKattmann · 2021-03-17T21:45:53Z

SU2_CFD/include/solvers/CSolver.hpp

+   * \details Overridden in CTurbSolver to impose fixed values to turbulence quantities
+   * in a specified upstream half-plane.


👍 for this explanation in CSolver. If one really cannot avoid the virtual then it is a nice touch to specify where it is actally used

TobiKattmann · 2021-03-17T22:05:10Z

TestCases/rans/naca0012/turb_NACA0012_sst_fixedvalues.cfg

+%
+% Numerical method for spatial gradients (GREEN_GAUSS, WEIGHTED_LEAST_SQUARES)
+NUM_METHOD_GRAD= WEIGHTED_LEAST_SQUARES
+NUM_METHOD_GRAD_RECON= LEAST_SQUARES


Wait, I recall there were some accuracy problems with LSQ gradient. Introduced in/ and see the discussion in #790 . Was it fixed in the meantime or were the issues not that bad anyway @pcarruscag ?

TobiKattmann · 2021-03-17T22:16:36Z

TestCases/rans/naca0012/turb_NACA0012_sst_fixedvalues.cfg

+% SU2 configuration file                                                       %
+% Case description: upstream turbulence quantities fixed to far-field value    %
+% Date: Mar 17th, 2021                                                         %
+% File Version 7.1.1 "Blackbird"                                               %
+%                                                                              %


I think it is a nice touch to put your name (or some identifier) here so it is directly clear who did this :) If it is intentional for privacy reasons then sure, but as you seem to use your actual name in github i guess thats not the issue here

I modified the turb_NACA0012_sst_sust.cfg file by @economon, so I was unsure which author name(s) and institution(s) to use. Would it be appropriate to write

% Author: Max Aehle % Institution: TU Kaiserslautern

TobiKattmann · 2021-03-17T22:20:22Z

TestCases/rans/naca0012/turb_NACA0012_sst_fixedvalues.cfg

+% -------------------------- MULTIGRID PARAMETERS -----------------------------%
+%
+% Multi-Grid Levels (0 = no multi-grid)
+MGLEVEL= 0
+%
+% Multigrid pre-smoothing level
+MG_PRE_SMOOTH= ( 1, 1, 1, 1, 1, 1 )
+%
+% Multigrid post-smoothing level
+MG_POST_SMOOTH= ( 0, 0, 0, 0, 0, 0 )
+%
+% Jacobi implicit smoothing of the correction
+MG_CORRECTION_SMOOTH= ( 0, 0, 0, 0, 0, 0 )
+%
+% Damping factor for the residual restriction
+MG_DAMP_RESTRICTION= 0.75
+%
+% Damping factor for the correction prolongation
+MG_DAMP_PROLONGATION= 0.75
+


I am a big fan of keeping the cfg's to its minimum. All these options (and quite a few more) can be deleted which makes potential change of cfg option much simpler down the road. Additionally if someone picks up this cfg it is a lot less overwhelming. The copy paste of complete cfg's and change of a single option is not really helpful imo. But then again here it is prob consistent with the existing Testcase which is prob in that repo (yep it is with the sst_sust)... so maybe it is the better choice here

Yes, this is a trade-off between conciseness and demonstrating what exactly is the feature... I would like to leave it as it is

If one wants to concise and demonstrate what the feature is one should remove superfluous/unused options from one's config ... like those above. The gained space can be used to point out what the options do and maybe give an example what to expect from a certain change in order to increaser user intuition. It is always nicer to add stuff but that leads to PR's like #1129 because already dead options where copy-pasted around in new Testcases.
I am ok with leaving this one as is but for future PR's all those unused (if applicable) MG_ _ADJ_ and other should just be deleted.

TobiKattmann · 2021-03-17T22:25:33Z

TestCases/serial_regression.py

+    turb_naca0012_sst_fixedvalues.cfg_dir   = "rans/naca0012"
+    turb_naca0012_sst_fixedvalues.cfg_file  = "turb_NACA0012_sst_fixedvalues.cfg"
+    turb_naca0012_sst_fixedvalues.test_iter = 10
+    turb_naca0012_sst_fixedvalues.test_vals = [-9.562435, -1.566603, 1.022029, 0.040549]


just a note: One can add as many screen outputs as one wants here. I think it is a good thing to add some (even arbitrary) screen outputs in order to slightly increase the coverage. It might save the e.g. LINSOL_RESIDUAL screen output in that solver one day.

You mean to add LINSOL_RESIDUAL to the line

SCREEN_OUTPUT= (INNER_ITER, RMS_DENSITY, RMS_TKE, RMS_DISSIPATION, LIFT, DRAG)

I'd then just update the new testcase, or is it better to cover it in all of the three testcases of this family (pure SST, SST_SUST, and fixedvalues)?

It is a more a general thing (for future PR's) because in this case you could also just add the 5th line which would be RMS_DENSITY which would already help increasing test coverage (and maybe even INNER_ITER as 6th one). You can add as many 'test.vals' as you like and should maybe use that to safeguard some things which might otherwise go untested. So if you had

SCREEN_OUTPUT= (INNER_ITER, RMS_DENSITY, RMS_TKE, RMS_DISSIPATION, LIFT, DRAG, LINSOL_RESIDUAL)

you could

turb_naca0012_sst_fixedvalues.test_vals = [9, -3.456789, -9.562435, -1.566603, 1.022029, 0.040549, 0.012345]

with the first 2 and the last value made up.

You need to leave inner iter out of the test_vals, but include it in the config.
+1 vote to all your comments.

TobiKattmann · 2021-03-17T22:37:35Z

config_template.cfg

+% Shift of the fixed values half-space, in space units in the direction of far-field velocity
+TURB_FIXED_VALUES_DOMAIN= -1.0


I didnt understand this explanation in the config_template and with your PR explanation and 1 min on sheet of paper it was easy ...
The second parameter specifies the half-plane on which fixed values are applied. This half-plane consists of those points for which the dot product of their coordinate vector with the normalized far-field velocity vector is less than TURB_FIXED_VALUES_DOMAIN.
Imo it is ok to be verbose in the cfg_template
(Now that I got it, this short sentence makes sense as well... 🙈 i think the space units 🚀 👨‍🚀 threw me off)

Yeah maybe "spatial units" or "length units" are better terms

maxaehle · 2021-03-19T12:37:07Z

@TobiKattmann @pcarruscag: I tried to follow all of your proposals in the new branch fix_turb_fixed_values. I removed the multigrid stuff from the NACA 0012 SST_SUST and fixed_values cfg, but not of the pure SST one. And I added verification of LINSOL_RESIDUAL and RMS_DENSITY to all the three tests. We can open a new PR or merge this in an existing one.

Would you like to also remove the adjoint stuff from the cfgs? And right now the fixed_values test does not start from a restart solution, could that be a problem?

maxaehle · 2021-03-19T12:39:05Z

We could also write a script that detects cfg options whose removal does not break the regression ;-)

pcarruscag · 2021-03-22T12:49:46Z

Ah! Cool, sure open a new PR @maxaehle.
What adjoint stuff? file names and so on?
I think removing irrelevant options would be make the tests clearer, there are lots with Roe and JST options specified and vice versa which probably confuses new users.
I'm not so sure about removing all defaults thought... On one hand it would serve as regression for the default values set by CConfig, on the other it hides the tuning parameters of some methods... but then again those are more or less documented now.
🤷

Continuation of #1236

maxaehle added 4 commits March 15, 2021 19:37

Added floor value treatment for turbulence quantities

dc22431

Replace "floor values" by "fixed values"

7aa7a4f

CConfig check that fixed values domain is set

3b84e35

Fix comments

f725029

pr-triage bot added the PR: unreviewed label Mar 16, 2021

pcarruscag reviewed Mar 16, 2021

View reviewed changes

maxaehle added 2 commits March 16, 2021 14:20

Corrections requested by @pcarruscag

441a30e

Give Impose_Fixed_Values a more generic flavor

5654648

pcarruscag reviewed Mar 16, 2021

View reviewed changes

pcarruscag approved these changes Mar 16, 2021

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Mar 16, 2021

pcarruscag added the changelog:feature label Mar 16, 2021

maxaehle added 2 commits March 17, 2021 07:56

do not check for fixed values in SST source residual

a7d9cc6

Added serial testcase turb_naca0012_sst_fixedvalues

be91c93

modification of the serial testcase turb_naca0012_sst

pr-triage bot added PR: unreviewed and removed PR: reviewed-approved labels Mar 17, 2021

maxaehle added 4 commits March 17, 2021 17:55

Update config_template.cfg

296dbf7

Moved new turb_naca0012_sst_fixedvalues testcase

2e64c67

Merge branch 'feature_turb_fixed_values_2' into feature_turb_fixed_va…

6e618b3

…lues

Merge branch 'develop' into feature_turb_fixed_values

cd9326b

maxaehle merged commit b3b5a2f into develop Mar 17, 2021

maxaehle deleted the feature_turb_fixed_values branch March 17, 2021 19:19

pr-triage bot added PR: merged and removed PR: unreviewed labels Mar 17, 2021

TobiKattmann reviewed Mar 17, 2021

View reviewed changes

pr-triage bot added PR: unreviewed and removed PR: merged labels Mar 17, 2021

maxaehle added a commit that referenced this pull request Mar 19, 2021

Changes proposed by @TobiKattmann in #1236

528ffb6

maxaehle mentioned this pull request Mar 22, 2021

Continuation of #1236 #1240

Merged

4 tasks

maxaehle added a commit that referenced this pull request Mar 31, 2021

Merge pull request #1240 from su2code/fix_turb_fixed_values

533a2c4

Continuation of #1236

maxaehle mentioned this pull request Nov 17, 2021

Explicit Euler in CScalarSolver #1435

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed values for turbulence quantities in upstream half-plane #1236

Fixed values for turbulence quantities in upstream half-plane #1236

maxaehle commented Mar 16, 2021 •

edited

Loading

pcarruscag Mar 16, 2021

maxaehle commented Mar 16, 2021

pcarruscag commented Mar 16, 2021

pcarruscag Mar 16, 2021

pcarruscag Mar 16, 2021

maxaehle Mar 17, 2021

maxaehle Mar 17, 2021

maxaehle Mar 17, 2021

pcarruscag left a comment

maxaehle commented Mar 17, 2021

maxaehle commented Mar 17, 2021

pcarruscag commented Mar 17, 2021

TobiKattmann left a comment

TobiKattmann Mar 17, 2021

TobiKattmann Mar 17, 2021

TobiKattmann Mar 17, 2021

TobiKattmann Mar 17, 2021

maxaehle Mar 18, 2021

TobiKattmann Mar 17, 2021

maxaehle Mar 18, 2021

TobiKattmann Mar 18, 2021

TobiKattmann Mar 17, 2021

maxaehle Mar 18, 2021

TobiKattmann Mar 18, 2021

pcarruscag Mar 18, 2021

TobiKattmann Mar 17, 2021

maxaehle Mar 18, 2021

maxaehle commented Mar 19, 2021

maxaehle commented Mar 19, 2021

pcarruscag commented Mar 22, 2021

		* \details Overridden in CTurbSolver to impose fixed values to turbulence quantities
		* in a specified upstream half-plane.

		% Shift of the fixed values half-space, in space units in the direction of far-field velocity
		TURB_FIXED_VALUES_DOMAIN= -1.0

Fixed values for turbulence quantities in upstream half-plane #1236

Fixed values for turbulence quantities in upstream half-plane #1236

Conversation

maxaehle commented Mar 16, 2021 • edited Loading

Proposed Changes

Related Work

PR Checklist

Choose a reason for hiding this comment

maxaehle commented Mar 16, 2021

pcarruscag commented Mar 16, 2021

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

pcarruscag left a comment

Choose a reason for hiding this comment

maxaehle commented Mar 17, 2021

maxaehle commented Mar 17, 2021

pcarruscag commented Mar 17, 2021

TobiKattmann left a 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

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

maxaehle commented Mar 19, 2021

maxaehle commented Mar 19, 2021

pcarruscag commented Mar 22, 2021

maxaehle commented Mar 16, 2021 •

edited

Loading