Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corrections to SA implementation #2352

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Corrections to SA implementation #2352

wants to merge 6 commits into from

Conversation

rois1995
Copy link
Contributor

@rois1995 rois1995 commented Sep 6, 2024

Proposed Changes

The CRot value was set to 1.0 for the HLPW5 but the standard value is 2.0 as suggested in TMR. Plus, there may be some inconsistencies with the negative branch of SA when used with the R variant. $\hat{S}$ should be used in the computation of the production term when $\hat{\nu}>0$. When $\hat{\nu}<0$, instead, the vorticity $\Omega$ should be used together with the $c_{t3}$ constant. Citing TMR:

"This model is the same as the "standard" version (SA), except that the (SA-R) production term becomes:

$c_{b1}(1-f_{t2})[\hat{S}+C_{rot}min(0,S-\Omega)]\hat{\nu}$

...

However, a negative production term is not acceptable for the negative branch ($\hat{\nu} < 0$) of the (SA-neg) model. If coding (SA-neg-R), a possible C0 continuous production term in the negative (SA-neg-R) branch is the following:

$c_{b1}(1-c_{t3})[\hat{\Omega}+C_{rot}min(0,S-\Omega)]\hat{\nu}$

"

I don't understand if the second production term has to be used everytime SA-neg is considered or only at the points where $\hat{\nu}<0$.

If it is already taken into account, then I'll modify the PR.

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.

  • 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
Copy link
Contributor Author

I think no more changes are needed. All of the test cases remain the same since probably the Rotation correction wasn't used.

The only major corrections are the following:

  • CRot now is set to 2.0 as in TMR
  • The computation of $\hat{S}$ is performed before the the Rotation correction is applied, not after.

@bigfooted
Copy link
Contributor

No regression tests are affected, so we have no test that uses this model? Can you add a test for this as well?

@pcarruscag pcarruscag changed the title Corrections to SA implementation [WIP] Corrections to SA implementation Sep 14, 2024
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.

3 participants