Fix periodic boundary condition for nodes on axis of rotation #840
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pcarruscag
reviewed
Dec 13, 2019
Comment on lines
7979
to
7983
| bool pointonAxis = false; | ||
|
|
||
| bool chksamePoint = false; | ||
|
|
||
| su2double dist_to_Axis = 0.0; |
There was a problem hiding this comment.
Hi @koodlyakshay,
Please follow the most common naming convention (pointOnAxis, distToAxis).
Is this going to match points to themselves if they are on the axis?
There was a problem hiding this comment.
Hi @pcarruscag,
Fixed the naming scheme.
Also, yes the idea is to map the CV around a node on the axis on one periodic face with the corresponding CV around the same node on the other periodic face.
talbring
approved these changes
Jan 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed Changes
This is a very small fix for periodic boundary conditions. In the MatchPeriodic routine, there is a check to make sure we never map to the same point when finding periodic pairs. However, if a node lies on the axis of rotation it has to map on to itself. The checks in the MatchPeriodic routine does not allow for this possibility and this node would end up getting mapped to the nearest neighbor which will lead to wrong results.
This PR introduces a small modification to the MatchPeriodic routine that will allow nodes on the axis to map onto themselves.
Related Work
As a test, consider the dummy geometry below. The domain is periodic about the Z-axis. I added a wall BC in the middle of the domain and the flow is supposed to separate at that point. This case is not very realistic but the main idea was to have periodic boundaries with points on the axis of rotation. The domain and the boundary conditions are similar to what we typically have when solving turbine blades. I wasn't sure if I can share the geometry, so I made this dummy test case. This case diverges with the develop branch. The image is from an intermediate result using the fix.
A quick way to verify if the nodes are matched properly is to check the velocity on the nodes that lie on the axis of rotation. Due to the periodicity constraint, the velocities must be aligned with the axis (here the Z-axis) which can be seen here as well.
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.