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

[WIP] Feature sst2003m #1557

Closed
wants to merge 6 commits into from
Closed

[WIP] Feature sst2003m #1557

wants to merge 6 commits into from

Conversation

bigfooted
Copy link
Contributor

@bigfooted bigfooted commented Mar 6, 2022

Proposed Changes

Give a brief overview of your contribution here in a few sentences.
edit:

Related Work

Resolve any issues (bug fix or feature request), note any related PRs, or mention interactions with the work of others, if any.
#1551
#1364

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

@bigfooted
Copy link
Contributor Author

bigfooted commented Mar 6, 2022

I looked at the transport equations and implemented the SST2003m model (as far as I can tell)
I'm not entirely sure about what happens in CTurbSSTSolver::Postprocessing
what's the difference between:
flownodes->
nodes->
geometry->nodes

I also have to check the implementation of the boundary conditions. When the implementation has been checked, I'll tackle the regression tests

Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start 👍
Of course this means we'll have to go to SU2 v8 😬
@jayantmukho anything we need to be aware here to avoid damaging UQ? Do we need to remove the tke term also from the perturbed Reynolds stress tensor?

SU2_CFD/src/solvers/CTurbSSTSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CTurbSSTSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/variables/CTurbSSTVariable.cpp Outdated Show resolved Hide resolved
SU2_CFD/include/numerics/turbulent/turb_sources.hpp Outdated Show resolved Hide resolved
SU2_CFD/include/numerics/turbulent/turb_sources.hpp Outdated Show resolved Hide resolved
bigfooted and others added 5 commits March 6, 2022 12:53
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@TobiKattmann
Copy link
Contributor

make the SST model exactly the SST2003m model according to the NASA website

Does it make sense to change the existing SST model rather than just adding the 2003m-version alongside of the 1994-version? Maybe rename to SST-1994 and SST-2003m, that makes it transparent. One could even keep SST while adding a warning that SST will be deprecated in the future and that SST-1994 is the equivalent successor.

The upside of doing it like that is that it wouldn't interfere that much with peoples cases (only little cfg change necessary to produce the same results) and it would be much less work for you to change all the regression tests.

And although the 2003-version is supposed to be an update to the 1994 version, I would bet there are people who prefer the 1994 version by choice. After all, when the Ancients first implemented SU2 they chose 1994 despite the 2003 version being around for 10 years already.

I would be ok with switching to 2003m completely but I dont see really see the benefit as it means more work for everybody (right?).

@bigfooted
Copy link
Contributor Author

Hi Tobi, Well the thing is that currently we do not have sst1994 nor sst2003. We can implement both version and make one of them the default but in any case it means that the regression tests have to be recalibrated. And if you want to update 'everything' then we also need to redo the V&V cases and make the plots again - preferably with something in place that will auto-create these plots when something changes in the future.

@pcarruscag
Copy link
Member

We should not roll out a new version of SST without having substantial coverage. I think with this PR we can:

  • Validate SST-2003m, flatplate and maybe channel bump, NACA0012.
  • Assess the changes required in the code, they are fairly subtle so we could consider having the 2 version, SST and 2003m as a "variant" specified like @suargi suggests for SA.
  • Assess the "damage" in the regressions, how many restarts need re-doing etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants