-
Notifications
You must be signed in to change notification settings - Fork 845
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
Correct gradients at boundaries #2285
Comments
Thanks for creating an issue for this. For no-slip walls, we know that velocity gradients tangential to the wall are zero, this could be taken into account, or in general as you said we should take into account the gradient information from the type of boundary condition. |
Hi @GiuSirianni, considering your experience with this, it would be great if you can contribute to this improvement. We have weekly developer meetings, do you have time to talk about this perhaps next week Wednesday? I think it is an important issue that needs to be addressed. |
Sure, I can join! I'm not sure if/when I will be able to actively contribute to a fix (I'm not even sure what it would be) |
To add some data I'll put a test where you can see the issue: This is an inviscid Euler simulation using the Span-Wagner EoS (CoolProp) of a non-ideal MDM nozzle, see https://su2code.github.io/tutorials/NICFD_nozzle using Using The boundary conditions are:
I tried both with and without a slope limiter as there are no discontinuities, but it makes no difference on the artifacts:
Complete test case ZIPInlet pressure zoomNotation:
Proof of concept code modification for "2nd order (BC 1st order)":To show that the error lies in MUSCL/gradients at boundaries I added these two lines of code in the upwind gradient computation Residuals:Mesh: |
Have you tried reconstructing from the interior point but not the one on the boundary? |
@pcarruscag I'm not sure if I misunderstood, you mean setting the limiter to zero on the boundary point right? Disable MUSCL on all points involved with a boundaryDisable MUSCL only on boundary points |
Thanks for testing. Both gradient methods use all the neighboring points. Green Gauss has special treatment for boundary surfaces and it must be particularly less accurate than least squares at these points. |
I agree that the "proposed" solution is no solution at all, it is just to prove the source of (a) problem. I do not agree that it is just masking the issue, it is removing it and just introducing another. Unless there is something I'm missing on the treatment of corners, which is very likely. I think the real solution in a perfect world would be to have ghost nodes with the boundary state saved and updated appropriately, as it would also fix symmetry boundaries, etc. I understand this is borderline non-implementable. |
PR #2194 Seems to fix this specific issue because a symmetry plane and an Euler wall are used here. We can think about similar improvements for viscous walls etc. |
That's great! @bigfooted can I ask if you see any issue in allowing the velocity index |
The current SU2 implementation checks if the array of variables contains the velocity and gives the index to the gradient calculator. You can give the gradient calculator the starting index and ending index as well.. So suppose you have an array of M variables, you could split it into 2 arrays, each containing a velocity vector, so array [0,..,N] and array [N+1,..,M], and you call greengauss twice. |
Good to know, thank you! |
@GiuSirianni @pcarruscag I think the correction of the symmetry plane and Euler wall has (mostly?) fixed this issue. But is there anything left in this discussion that we can use to improve quality and/or convergence? Can this issue be closed? |
No, switching to first order is not a proper solution so I agree on avoiding doing this. The only possible thing I would add is to take into account the inlet and outlet boundary conditions in the computation of gradients at boundary points, but this could be a separate issue altogether. |
There are other boundary-variable combinations where the gradients are known and can be enforced. |
Is your feature request related to a problem?
At any boundary that is not a Neumann-zero boundary ($\nabla \vec{V} \cdot \hat{n} = 0$ ) the gradients computed are incorrect, as they are all computed assuming that at boundaries $\vec{q}_i = \vec{q}_j$ .
For example, for a Riemann boundary condition we have a certain imposed external state$\vec{q}_e$ but this is not considered in the computation of gradients.
In my experience, this issue is exacerbated when using MUSCL reconstruction at corners shared by walls and Riemann boundaries, particularly in NICFD cases, as it can cause divergence of the solver and/or unphysical solutions.
Describe the solution you'd like
The solution is not easy as ideally it would require storing all "external" states for the computation of gradients. One may need to choose if swapping the order of the computation of boundary residuals and of gradients to avoid double computations, but it may be inefficient for MPI.
Describe alternatives you've considered
One could also disable MUSCL at corner/boundary nodes which would require saving which points are corners. This is a 1st order approximation locally, but my opinion is that the current assumption that$\vec{q}_i = \vec{q}_j$ is also lower order.
Another possibility is correcting boundaries after they have been computed by summation/subtraction, before the upwind residual computations.
Additional context
The text was updated successfully, but these errors were encountered: