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

Sliding mesh for incompressible flows + Cleaning of transfer routines #638

Merged
merged 21 commits into from
Feb 4, 2019

Conversation

rsanfer
Copy link
Contributor

@rsanfer rsanfer commented Jan 29, 2019

Proposed Changes

The current PR serves two purposes. First, it extends the sliding interface capabilities for multi-zone fluid-fluid interfaces to incompressible flows. Until now, these features were only available for compressible flows. For example, take the following case of a viscous flow enclosed by diathermal walls and discretized using two different zones with a permeable, non-matching interface:

testcase
discretization

With the current SU2 implementation, the solution diverges due to the lack of interface boundary conditions. After only 10 iterations, the contours of velocity are:

former

With this PR, the problem can be successfully solved with a multizone solution process. The contours of velocity and temperature are now:

velocity
temperature

and also for unsteady problems

unsteady

For the latter case I haven’t yet rotated the inner circle, because there are some missing features for dynamic meshes in the incompressible solver. The previous test case for steady-state flows has been added. One for unsteady flows will be enabled once the new output is in place, as the current output structure makes the regression scripts fail.

The previous, however, opened a can of worms, as there were some physics based calls in the parent transfer structure (e.g. GetnSlidingStates(), SetSlidingState() and SetnSlidingStates()) that made the incompressible CHT cases run into problems.

So I’ve also taken the chance to move some initialization and some calls to the specific physics-based transfer structures, making use of the polymorphism of the transfer code. This way the parent class remains opaque to the physics. At the same time, I cleaned up some long-time unused routines, made the Broadcast_Data the default for zone interfaces, and removed the MATCHING_MESH option as it is just a particular case of Nearest Neighbour.

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 the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags).
  • My contribution is commented and consistent with SU2 style.
  • I have added a test case that demonstrates my contribution, if necessary.

@LaSerpe
Copy link
Contributor

LaSerpe commented Jan 30, 2019

Hi Ruben,
thanks for this contributions.
The implementation seems to be ok at first glance, and also the cleaning up is very nice.
It should pass without any problem, btw I'll have a more accurate look at it tomorrow!

cheers,
Giulio

}
}
break;
if (transfer_types[donorZone][targetZone] == SLIDING_INTERFACE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps we could put a switch statement in place of else ifs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I mentioned below to @pcarruscag, I'll have a look into this and maybe create a small class for the transfer types to avoid so many switch/if statements. I aim to remove this from driver_structure and move it all to driver_direct_multizone.cpp soon, I'll try to include it there.

@LaSerpe
Copy link
Contributor

LaSerpe commented Jan 31, 2019

Thanks Ruben, everything looks good to me.
It is a nice clean up and the implementation of the added capabilities is aligned to what we already have, so I don't see any issue with merging this in.
I just had a minor comment but it is just about a coding style preferences.
We can merge this in as soon as the regression tests pass.

Giulio

Copy link
Member

@economon economon left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding to the inc solver, test cases, and the simplification to transfer.

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.

Looks great and thank you for the clean up, I just have a couple comments:

if ( config_container[donorZone]->GetMatchingMesh() ) {
if (rank == MASTER_NODE)
cout << "between matching meshes. " << endl;
geometry_container[donorZone][INST_0][MESH_0]->MatchZone(config_container[donorZone], geometry_container[targetZone][INST_0][MESH_0], config_container[targetZone], donorZone, nZone);
Copy link
Member

Choose a reason for hiding this comment

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

You can probably delete the MatchZone method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I forgot to do so. I'll do it in a minute.

@@ -638,44 +638,44 @@ bool CMultizoneDriver::Transfer_Data(unsigned short donorZone, unsigned short ta
/*--- Select the transfer method according to the magnitudes being transferred ---*/

if (transfer_types[donorZone][targetZone] == SLIDING_INTERFACE) {
transfer_container[donorZone][targetZone]->Broadcast_InterfaceData_Interpolate(solver_container[donorZone][INST_0][MESH_0][FLOW_SOL],solver_container[targetZone][INST_0][MESH_0][FLOW_SOL],
geometry_container[donorZone][INST_0][MESH_0],geometry_container[targetZone][INST_0][MESH_0],
Copy link
Member

Choose a reason for hiding this comment

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

In the interest of keeping the drivers independent of physics, would it make sense to make "transfer_types" a matrix of a little class (instead of enum) that stores also the donor/target solvers? You could then avoid all these if's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree this could be done better. I'll look into it soon and put a new PR request, probably with some other changes towards generalisation on the driver that are also needed.

@rsanfer
Copy link
Contributor Author

rsanfer commented Feb 4, 2019

Tests are passing, so if there are no further comments I'll merge this one in. I'll further tackle driver generalisation in PRs to come.

@talbring talbring merged commit 110280b into develop Feb 4, 2019
@rsanfer rsanfer deleted the feature_sliding_incompressible branch February 5, 2019 07:16
CatarinaGarbacz pushed a commit that referenced this pull request Aug 26, 2019
Sliding mesh for incompressible flows + Cleaning of transfer routines
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.

5 participants