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 slidingmesh for SA #1344

Merged
merged 16 commits into from
Aug 7, 2021
Merged

Conversation

maxaehle
Copy link
Contributor

@maxaehle maxaehle commented Aug 3, 2021

Proposed Changes

CSlidingInterface::GetDonor_Variable needs to access the Primitive of flow variables and the Solution of turbulent variables. Which one is the case, is currently decided by checking donor_solution->GetnPrimVar() == 2 (true->turbulent, false->flow). If the SA turbulence model is used, the non-overidden "virtual placeholder" CVariable::GetPrimitive(unsigned long,unsigned long) const returning zero is incorrectly called.

We propose to fix this by additionally checking for donor_solution->GetnPrimVar() == 1.

Related Work

@maxaehle, @mcmorelli, @BeckettZhou, @EduardoMolina

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.

@maxaehle maxaehle changed the title Fix slidingmesh reconstructboundary pr1 Fix slidingmesh for SA Aug 3, 2021
@maxaehle
Copy link
Contributor Author

maxaehle commented Aug 4, 2021

I followed @pcarruscag 's request in commit 12974ab.
BTW it's very confusing that the SA/SST turbulent solvers have nPrimVar==1,2 but GetPrimitive is not overridden and thus returns zero. So another fix to the bug considered in this PR would be to override GetPrimitive on the turbulent solvers such that it forwards to GetSolution?

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.

Thanks Max, I missed the solution v primitive part of the problem.

SU2_CFD/src/interfaces/cfd/CSlidingInterface.cpp Outdated Show resolved Hide resolved
@pcarruscag
Copy link
Member

I don't think forwarding GetPrimitive to GetSolution by default is a good option, a "Slinding state" layer could be appropriate but that does not need to be fixed right now.

@maxaehle
Copy link
Contributor Author

maxaehle commented Aug 4, 2021

By "Sliding State layer", do you mean a virtual CVariable::GetSlidingMeshDonorVariable that returns 0 by default, but forwards to GetSolution on the flow and GetPrimitive on the turb variables?

@pcarruscag
Copy link
Member

I would add that layer to the solver so that the interpolation does not have to bypass it by going straight to the nodes.

@pcarruscag pcarruscag merged commit df2469f into develop Aug 7, 2021
@pcarruscag pcarruscag deleted the fix_slidingmesh_reconstructboundary_pr1 branch August 7, 2021 14:16
@maxaehle
Copy link
Contributor Author

maxaehle commented Aug 9, 2021

Ok thanks Pedro!

@TobiKattmann
Copy link
Contributor

Thanks @maxaehle 💐

Not sure how this will interact with scalar solvers (species transport) to come ... but in case someone there wants to work with sliding interfaces then I guess one has to test that a priori

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.

5 participants