k-omega SST 2D axisymmetric source terms #1195

FlorianDm · 2021-02-10T12:19:41Z

This implements the k-omega SST source terms for 2D axisymmetric problems in a new member function of the existing source term class (it would never be used independently of it so I did not create a new class).

The extra source terms are the difference between the equations in 2D cartesian and 3D cylindrical coordinates under an assumption of axisymmetry without swirl. They are almost the same as those for Wilcox's k-omega shown in this image (taken from http://eprints.gla.ac.uk/184394/1/184394.pdf).

Only the constants have been swapped for the blended ones and the changed definition of the eddy viscosity is taken into account (see menter1994.pdf). Cross diffusion is the same as in 2D when assuming there is no azimuth gradient.

Note that these source terms have the opposite sign (RHS) of the flow equations source terms (LHS) since the block is subtracted and not added.

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.

Looking into creating an axisymmetric RANS test/regression case

…e never needed alone)

SU2_CFD/src/numerics/turbulent/turb_sources.cpp

pcarruscag · 2021-02-10T20:40:29Z

SU2_CFD/src/numerics/turbulent/turb_sources.cpp

+
+void CSourcePieceWise_TurbSST::ResidualAxisymmetric(su2double alfa_blended, su2double zeta){
+


I would define this inline (in the hpp) and let the compiler have some fun trying to re-use some of the stuff that exists in the body of ComputeResidual.
Probably not massive performance gains but they are completely free in this case.

Otherwise everything looks very nice. Are you going to add the regression in this PR?

Okay why not... I make it inline

I was going to add the case in another PR but as you prefer. I reckon there is not much risk in merging this in already since the current state of affairs is no more validated or tested for performance and strictly wrong as it is incomplete. From a little testing I can see only a small effect on the results but convergence seems to be improved

Got it, I think it is always best to add it together with the code, while the reference results are fresh in your mind.
If you add it later you might find yourself having to debug again because of some change somewhere else.

since the current state of affairs is no more validated or tested for performance and strictly wrong as it is incomplete.

It's cases like these (small changes) where unit tests are handy: https://su2code.github.io/docs_v7/Writing-Unit-Tests/

Not sure about this but I don't really see the need for unit tests as this bit is structurally very straightforward code and its behaviour is tested in regression now anyway

Regression tests detect change rather than correctness.
It is possibly that while fixing a bug a new one is introduced, regression values change, people check a few of them (it is boring work so no one tests all of them, especially if the residuals change very little), people update the regressions...
It has happened before, it will happen again.
With unit tests it is harder for that to happen, depending on the size of the "unit"...

First sentence was wrong, text was edited.

Alright I can see how that could happen but can't come up with a good unit test for this part. Keeping this in mind for the future though

I agree with @pcarruscag. I brought up unit tests because "small changes that aren't complete" is a perfect use case for unit tests. Unit tests can be added to verify correctness before code is ready for a complete regression test.

SU2_CFD/include/numerics/turbulent/turb_sources.hpp

pcarruscag · 2021-02-11T15:26:19Z

SU2_CFD/include/numerics/turbulent/turb_sources.hpp

+      /*--- Add contribution to the jacobian for implicit time integration---*/
+      Jacobian_i[0][0] += yinv*Volume*(sigma_k_i/zeta*TurbVar_Grad_i[0][1]-V_i[2]);
+      Jacobian_i[0][1] -= yinv*Volume*sigma_k_i*TurbVar_i[0]*TurbVar_Grad_i[0][1]/(zeta*zeta);
+      Jacobian_i[1][0] += yinv*Volume*sigma_k_i/zeta*TurbVar_Grad_i[1][1];
+      Jacobian_i[1][1] -= yinv*Volume*(sigma_k_i*TurbVar_i[0]*TurbVar_Grad_i[1][1]/(zeta*zeta)+V_i[2]);


All the terms added to the normal SST Jacobian are negative to avoid imbalancing the diagonal dominance of the matrix.
Do you have a feel for how large the axi symmetric contributions might be compared with the normal ones? (I guess larger near the axis).

They may have the same magnitude as the normal ones and may be negative or positive depending on the sign of the y velocity and the gradients. Yes the further from the axis the more the problem becomes just normal 2D. I will try different versions with nothing more added to the jacobian or only convection or only diffusion (or setting anything positive to zero perhaps but that seems no good idea).

The "clamping things with min/max" to have the sign you want is very common.
We have a config option for "accurate flux Jacobians" that we use for other schemes, you can also base the decision to clamp or not on that option to make things more flexible.

pcarruscag

Aside those choices with the Jacobian all LGTM 👍

FlorianDm · 2021-02-16T15:04:20Z

About the jacobian I could not find a situation where it made a difference (I tried a lot) so I commented it out.

The residual itself does matter however. I added a test case which uses the same mesh as the NICFD nozzle tutorial. It is an axisymmetric transonic air nozzle without a shock. Pretty much any change to the axisymmetric flow or turbulence residuals would break it.

i created a pull request on the test cases repo develop branch su2code/TestCases#59 but then I just went ahead and merged it into develop since there is no review policy

SU2_CFD/include/numerics/turbulent/turb_sources.hpp

TestCases/axisymmetric_rans/air_nozzle/air_nozzle.cfg

pcarruscag · 2021-02-16T15:29:48Z

Thank you for adding a regression.
Why do you need both a restart_flow.dat and a solution_flow.dat file in Testcases? We should try to keep that repo small.

FlorianDm added 11 commits February 4, 2021 19:01

new function inside existing sst source term class (no new class sinc…

bd6f644

…e never needed alone)

implement function to compute residual

be5ed7a

fix typo k2 not k1

f1f6ce0

Merge branch 'develop' into feature_axi_turbsst

cc2efbe

fix error

1e6707b

add production

fe13b23

Merge branch 'develop' into feature_axi_turbsst

efd20bb

Merge branch 'develop' into feature_axi_turbsst

f939300

first attempt complete

81c6331

add diffusion terms to jacobian

c78cfce

small change

4c5c718

pr-triage bot added the PR: unreviewed label Feb 10, 2021

WallyMaier reviewed Feb 10, 2021

View reviewed changes

SU2_CFD/src/numerics/turbulent/turb_sources.cpp Outdated Show resolved Hide resolved

pcarruscag reviewed Feb 10, 2021

View reviewed changes

SU2_CFD/src/numerics/turbulent/turb_sources.cpp Outdated Show resolved Hide resolved

pcarruscag reviewed Feb 10, 2021

View reviewed changes

SU2_CFD/src/numerics/turbulent/turb_sources.cpp Outdated Show resolved Hide resolved

pcarruscag reviewed Feb 10, 2021

View reviewed changes

FlorianDm and others added 3 commits February 11, 2021 14:54

Corrections based on PR comments

f3637d8

cosmetics

c59e7a6

Merge branch 'develop' into feature_axi_turbsst

6883961

pcarruscag reviewed Feb 11, 2021

View reviewed changes

SU2_CFD/include/numerics/turbulent/turb_sources.hpp Outdated Show resolved Hide resolved

pcarruscag reviewed Feb 11, 2021

View reviewed changes

SU2_CFD/include/numerics/turbulent/turb_sources.hpp Outdated Show resolved Hide resolved

pcarruscag added the changelog:fix label Feb 11, 2021

pcarruscag reviewed Feb 11, 2021

View reviewed changes

FlorianDm added 3 commits February 11, 2021 16:36

remove else

459362a

add AD::SetPreaccIn

63e89f1

Merge branch 'develop' into feature_axi_turbsst

ea0f3a6

FlorianDm marked this pull request as draft February 11, 2021 16:23

pr-triage bot added PR: draft and removed PR: unreviewed labels Feb 11, 2021

pcarruscag approved these changes Feb 11, 2021

View reviewed changes

FlorianDm added 6 commits February 15, 2021 11:24

minor changes and correction

4c1fdcf

comment out jacobian to conserve diagonal dominance

dd1b1b3

spaces and comments

4767adb

error

8a31347

Merge branch 'develop' into feature_axi_turbsst

c2a25dc

add test case and regression list entry

0025253

FlorianDm mentioned this pull request Feb 16, 2021

axisymmetric RANS: add mesh and restart file file su2code/TestCases#59

Merged

pcarruscag reviewed Feb 16, 2021

View reviewed changes

SU2_CFD/include/numerics/turbulent/turb_sources.hpp Outdated Show resolved Hide resolved

pcarruscag reviewed Feb 16, 2021

View reviewed changes

TestCases/axisymmetric_rans/air_nozzle/air_nozzle.cfg Outdated Show resolved Hide resolved

remove jacobian contribution and test case config file options

2b17327

FlorianDm marked this pull request as ready for review February 16, 2021 15:54

pr-triage bot added PR: unreviewed and removed PR: draft labels Feb 16, 2021

Merge branch 'develop' into feature_axi_turbsst

a4c376a

FlorianDm merged commit b490f3d into develop Feb 17, 2021

FlorianDm deleted the feature_axi_turbsst branch February 17, 2021 08:51

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

TobiKattmann mentioned this pull request Feb 18, 2021

fix typo Linael -> Linear #1201

Merged

5 tasks

pr-triage bot added PR: merged and removed PR: unreviewed labels Feb 18, 2021

FlorianDm mentioned this pull request Nov 13, 2021

fix mistake in axisymmetric turbulent source production term #1438

Merged


		void CSourcePieceWise_TurbSST::ResidualAxisymmetric(su2double alfa_blended, su2double zeta){

k-omega SST 2D axisymmetric source terms #1195

k-omega SST 2D axisymmetric source terms #1195

Uh oh!

Conversation

FlorianDm commented Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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 Feb 16, 2021 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clarkpede Feb 18, 2021 • 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!

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

Choose a reason for hiding this comment

Uh oh!

FlorianDm commented Feb 16, 2021

Uh oh!

Uh oh!

Uh oh!

pcarruscag commented Feb 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

FlorianDm commented Feb 10, 2021 •

edited

Loading

pcarruscag Feb 16, 2021 •

edited

Loading

clarkpede Feb 18, 2021 •

edited

Loading