-
Notifications
You must be signed in to change notification settings - Fork 849
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] Fix cornernode normal #2392
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the full fix.
First issue, at vertices shared by two markers we compute two normals for the same point, potentially resulting in different "normal neighbors". Then the result we get depends on whether markers are merged or not, their order, etc.
Then the distance we should use is the wall distance and not just the straight line between points.
So if you want to do something that doesn't break all cases you have the solution I mentioned in the issue
smallest non-zero wall distance of a neighbor of the wall point.
OK, so the 'normal angle' does not matter anymore in that case, we only have edge length (to an interior node) as requirement? |
I am very interested in a solution to this issue, as this particular bug has caused convergence issues for a long time (especially with the SST Models). Here is a comparison of 8.1.0 (with the corner of issues circled in red) and the fix_cornernode_normal version just created. It seems to be causing all of the wall distances to be miscalculated now. |
We only have wall distance as a requirement, not distance between nodes. |
OK, I changed it to wall distance. But still, the corner node has zero shear stress, so y+ = 0 in that case. But I do not think that this is problematic. |
Good point, in such cases we can probably use cbrt of the volume at that point to avoid a 0. |
I can introduce FindClosest_Neighbor, and use that for the walls in TurbSSTSolver. We can then phase out FindNearest_Neighbor over time. |
Just do what you need to do in the SST wall boundary condition for now |
Y plus value of zero just means that the skin friction is zero there, right? which is expected since it is a stagnation point. Thus I don't understand where a division by zero should be occurring. Moreover, why are we computing the distance again to search for the closest one? Shouldn't we use the wall distance already computed? |
@pcarruscag do you think this is the way to go? If so, I can clean it up and fix the regressions, that's still a whole bunch. |
I don't think so.
So, in the SST solver, and nowhere else:
|
But we still take interior neighbors only and discard wall points, we just do not do the second search?
|
Those will have wall distance = 0 |
if (Point_Normal==0) { | ||
cout << "no point found at i = " << iPoint << endl; | ||
su2double Area = 0.0; | ||
su2double TwoVol = 2.0* (geometry->nodes->GetVolume(iPoint) + geometry->nodes->GetPeriodicVolume(iPoint)); | ||
for (size_t iNeigh = 0; iNeigh < geometry->nodes->GetnPoint(iPoint); ++iNeigh) { | ||
size_t iEdge = geometry->nodes->GetEdge(iPoint, iNeigh); | ||
size_t jPoint = geometry->nodes->GetPoint(iPoint, iNeigh); | ||
const su2double* Normal = geometry->edges->GetNormal(iEdge); | ||
Area = GeometryToolbox::Norm(nDim, Normal); | ||
|
||
dist_min += TwoVol / Area; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pcarruscag I now did this. If I compare this length scale in the corner node to the ones next to it, it is much larger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure but have you tried what I suggested? Namely, wall distance (not Euclidean), and volume divided by the sum of vertex areas (vertices of viscous markers).
Vertices are on boundaries, edges are internal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm getting old and am a bit hard of hearing :-)
OK, so in the code above we only take the vertex areas, we check if jPoint is actually on the wall. using GetViscousBoundary
For the 'regular' routine we can do x.n/||n||
I have modified it, length scales seem to be fine now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool thanks for testing. I think it's simpler than that, I'll push something in the weekend, don't start updating tests yet.
Proposed Changes
Give a brief overview of your contribution here in a few sentences.
Quick fix to issue #2383
The normal node is the interior node that is most normal to the edge. If the node is on the wall, it is only considered as normal node if no previous normal nodes were detected (just for the case that there are no normal nodes). A normal node that is on the wall can be overwritten by any interior node.
PR Checklist
pre-commit run --all
to format old commits.