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 essential bc with meshvar #255

Closed

Conversation

gthyagi
Copy link
Contributor

@gthyagi gthyagi commented Oct 21, 2024

Here is the fix for using mesh variable in essential boundary condition. #221

@gthyagi gthyagi requested review from lmoresi and julesghub October 21, 2024 05:03
@lmoresi
Copy link
Member

lmoresi commented Oct 21, 2024

Glad to see you've figured this out - Is this tested in parallel ?

@gthyagi
Copy link
Contributor Author

gthyagi commented Oct 21, 2024

Yes, I tested it in parallel on mac, and everything looks fine. I still need to run tests on NCI with multiple nodes.

@gthyagi
Copy link
Contributor Author

gthyagi commented Oct 21, 2024

Please hold off on merging this PR for now. I’m currently running some tests on the annulus with an internal boundary and checking if the code handles pressure boundary conditions without breaking.

@lmoresi
Copy link
Member

lmoresi commented Oct 21, 2024

I am not sure that we really can do pressure boundary conditions ? Are you talking about essential boundary conditions on the constraint equation ? Don't the other assumptions we make rule out using any discontinuous functions for the pressure term if we want to have boundary values imposed this way ?

Have you checked with @knepley whether the field split machinery is set up to work this way ?

@gthyagi
Copy link
Contributor Author

gthyagi commented Oct 22, 2024

In the spherical benchmarks, applying essential boundary conditions on pressure appears to eliminate the null space in the pressure solution. Since I am still testing these benchmarks, I thought it would be useful to keep the pressure boundary conditions for now, even though we don't fully understand their effects during the solve.

I made a small change at the solution separation stage to return all indices in the index set for the pressure field, which allows us to separate the velocity solution by taking the complement of that index set. The current implementation works correctly, whether or not we apply pressure boundary conditions, so things should work as expected.

@knepley
Copy link
Collaborator

knepley commented Oct 22, 2024

@gthyagi An alternative for removing the null space due to the pressure is to solve in a multigrid iteration, and on the coarse grid use SVD. This can often work because the smoothers and prolongation to not reintroduce components in the nullspace. You can set the coarse solve with -mg_coarse_pc_type svd. Let me know if this does not work.

@gthyagi
Copy link
Contributor Author

gthyagi commented Jan 17, 2025

this is same as #278, which is already merged.

@gthyagi gthyagi closed this Jan 17, 2025
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.

3 participants