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

Fix CFL Turbulence reduction option for adaptive cfl #2295

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

Conversation

emaberman
Copy link

Proposed Changes

Extend the Turbulence CFL reduction option, such that it can be used for adaptive CFL too.

Turbulence cfl reduction was previously implemented only for a constant cfl, here the treatment is extended also for adaptive cfl.

Related Work

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

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.

Extend the Turbulence CFL reduction option, such that it will be available
when using the adaptive CFL
SU2_CFD/src/solvers/CSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CSolver.cpp Outdated Show resolved Hide resolved
@pcarruscag
Copy link
Member

Thanks for the contribution, add yourself to AUTHORS.md please.

@bigfooted
Copy link
Contributor

Please check and update the regression tests before merging.

@emaberman
Copy link
Author

I am having trouble with local regression testing. When running regression tests locally on my computer following instructions in the documentation many tests fail, including ones that seem to pass when attempting to upload. I seem to be missing something..
What am I doing wrong?

@pcarruscag
Copy link
Member

The test values are sensitive to compiler versions and options.
Just run the failing cases locally and if the results looks reasonable update the results based on the github tests.

@bigfooted
Copy link
Contributor

There is one case that has different residuals, the Jones turbocharger case. The other case is a know problem that is fixed in another PR.
So you should check if this case uses adaptive CFL and CFL_REDUCTION_TURB, because that is when the residuals could be different. Please also check if it still converges, because it looks like it 's not. So maybe it needs another CFL number?

@emaberman
Copy link
Author

The jones_turbocharger_restart case was set to use adaptive CFL coupled with CFL turbulence Reduction thus changes in the results are expected , I ran the case a few thousand steps and the case seems to converge. Therefore I am allowing myself to change test results leaving the configuration setting as is.

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.

None yet

3 participants