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 shared wall-inlet corner node for compressible #2266

Open
wants to merge 43 commits into
base: develop
Choose a base branch
from

Conversation

bigfooted
Copy link
Contributor

@bigfooted bigfooted commented Apr 16, 2024

Proposed Changes

Give a brief overview of your contribution here in a few sentences.
Nodes shared by an inlet and a wall show nonphysical values in the corner node for energy, temperature, pressure, density. This fixes the issue

Related Work

Resolve any issues (bug fix or feature request), note any related PRs, or mention interactions with the work of others, if any.

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 --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • 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.

@bigfooted
Copy link
Contributor Author

@Cristopher-Morales Can you check if this fixes the issue for you? This works for the testcase that you gave me (2D compressible channel flow).
There is still a (compared to develop) very small local change in pressure but I think this is because at the inlet, the radial velocity is not zero and close to the wall the radial velocity component is not negligible, this is I think why the pressure locally is still not completely homogeneous.

@bigfooted
Copy link
Contributor Author

bigfooted commented Apr 16, 2024

Below is a zoom of the 2D flow in a rectangular channel. The flow is from left to right. The top side is a wall, the left side is the inlet. We are looking at the corner node shared by the inlet and the wall.
Pictures showing the Pressure for old (develop) and new implementation. The local pressure changes reduce by more than one order of magnitude:
2024-04-16_23h37_19
2024-04-16_23h34_15

Old and new energy near the corner:
2024-04-16_23h35_55
2024-04-16_23h33_50

Near the corner, the y-momentum (radial velocity) is not zero:
2024-04-16_23h34_41

@bigfooted bigfooted self-assigned this Apr 16, 2024
@bigfooted
Copy link
Contributor Author

the user defined function case: a flat plate. top is with the fix, bottom (mirrored): the old develop.
udf

@bigfooted
Copy link
Contributor Author

@Cristopher-Morales can you check if bounded scalar works for you for this setup with compressible flow? I activated it and it seems to work fine for the small testcase that you sent. It is much better than scalar upwind.

@bigfooted
Copy link
Contributor Author

Poiseuille flow, top is corrected, bottom is develop. There is still a small discrepancy in the energy but this might be due to the strong velocity components close to the wall.
image

@Cristopher-Morales
Copy link
Contributor

@Cristopher-Morales can you check if bounded scalar works for you for this setup with compressible flow? I activated it and it seems to work fine for the small testcase that you sent. It is much better than scalar upwind.

species_transport
Hi, yes it works much better, specially if we refine the mesh towards the walls.

@bigfooted bigfooted changed the title [WIP] Fix shared wall-inlet corner node for compressible Fix shared wall-inlet corner node for compressible Apr 24, 2024
@bigfooted
Copy link
Contributor Author

@EvertBunschoten can you check specifically the bounded scalar method? not sure if I missed something, see the comments that I left in the code.

@GiuSirianni
Copy link

GiuSirianni commented May 13, 2024

I'm sorry to interject on this issue with what is probably a useless comment, but I wanted to ask if this issue affects you also in 1st order simulations?

In my experience MUSCL reconstruction at boundaries in SU2 is "incorrect" as it doesn't take into account the boundary condition applied and just assumed v_i = v_j, as far as I have understood from scouring the code. This usually is a small first order approximation locally, but with complex EoS or unlucky combinations of BCs and numerical settings it can cause unphysical solutions and crashes.

In a project of mine I have had some success by either disabling MUSCL outright at boundaries (very conservative and brutal solution), or disable it only at specific kinds of corners, which requires more memory unfortunately as one needs to somehow save if the node is a corner, etc.

The "real solution" would be to either save the boundary solution in ghost nodes and use it for the gradient computation, or fix gradients in post-process with a small performance penalty.

@EvertBunschoten
Copy link
Member

It looks like the edge mass fluxes are not stored when running compressible RANS simulations, preventing any species convection when using BOUNDED_SCALAR. I'm looking into why this happens. Below is the species solution after 10 iterations for the passive_transport_validation test case for the RANS solver and using BOUNDED_SCALAR. The species values specified at the inlet do not progress downstream using the current settings.

bounded_scalar_compressible

@bigfooted
Copy link
Contributor Author

Hi, @GiuSirianni, The problem I am addressing here is even more basic and occurs independent on the MUSCL scheme. When a corner is shared between an inlet and a wall, the inlet has to take into account that the node is shared by a wall (it has to know about no-slip for instance).
But it is good to know that MUSCL has issues at boundaries as well, this should definitely be fixed. If you can spare a couple of minutes, can you create an Issue here on github for MUSCL not being high order/robust on boundaries and share your experience?

@bigfooted
Copy link
Contributor Author

@EvertBunschoten since bounded scalar was never implemented for compressible flow, I think we should create a separate PR for it.
If the symmetry PR is merged, I will update the residuals for this PR as well.

Common/src/CConfig.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CEulerSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CEulerSolver.cpp Show resolved Hide resolved
SU2_CFD/src/solvers/CEulerSolver.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/solvers/CSpeciesSolver.cpp Outdated Show resolved Hide resolved
bigfooted and others added 2 commits August 22, 2024 16:57
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted
Copy link
Contributor Author

@pcarruscag Finally had some time to look into this again. Behavior is still as described above. If you have time, can you have another look?

Comment on lines +7150 to +7152
case INLET_TYPE::MASS_FLOW: {

/*--- Impose the wall velocity from the interior wall node. ---*/
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why the treatment differs for total conditions and mass flow.

/*--- Match the pressure, density and energy at the wall. ---*/

Pressure = nodes->GetPressure(iPoint);
Density = Pressure / (Gas_Constant * Temperature);
Copy link
Member

Choose a reason for hiding this comment

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

This temperature comes from the normal stagnation treatment?
Total conditions already respects the velocity of the interior node, what is this changing?
And, can we instead change the PTotal and TTotal that are specified and let the normal treatment do the rest?

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